Fix the NaCl bitstream reader to report fatal errors.

include/llvm/Bitcode/NaCl/NaClBitstreamReader.h

Index: include/llvm/Bitcode/NaCl/NaClBitstreamReader.h
diff --git a/include/llvm/Bitcode/NaCl/NaClBitstreamReader.h b/include/llvm/Bitcode/NaCl/NaClBitstreamReader.h
index 97e216441939906f33f1c2d1f48585005bf51e8f..1d07b19dfd5f618a6a37d71ccc105b11853e178d 100644
--- a/include/llvm/Bitcode/NaCl/NaClBitstreamReader.h
+++ b/include/llvm/Bitcode/NaCl/NaClBitstreamReader.h
@@ -90,6 +90,11 @@ public:
     return InitialAddress;
   }
 
+  /// Returns the Bit as a Byte:BitInByte string. MinByteWidth is the
+  /// minimum number of characters to print out the Byte value (blank
+  /// fills).
+  static std::string getBitAddress(uint64_t Bit, unsigned MinByteWidth=1);
+
   //===--------------------------------------------------------------------===//
   // Block Manipulation
   //===--------------------------------------------------------------------===//
@@ -209,9 +214,30 @@ public:
 /// Unlike iterators, NaClBitstreamCursors are heavy-weight objects
 /// that should not be passed by value.
 class NaClBitstreamCursor {
+public:
+  /// This class handles errors in the bitstream reader. Redirects
+  /// fatal error messages to virtual method Fatal.
+  class ErrorHandler {
+    ErrorHandler(const ErrorHandler &) = delete;
+    ErrorHandler &operator=(const ErrorHandler &) = delete;
+  public:
+    explicit ErrorHandler(NaClBitstreamCursor &Cursor) : Cursor(Cursor) {}
+    LLVM_ATTRIBUTE_NORETURN
+    virtual void Fatal(const std::string &ErrorMessage) const;
+    virtual ~ErrorHandler() {}
+    uint64_t getCurrentBitNo() const {
+      return Cursor.GetCurrentBitNo();
+    }
+  private:
+    NaClBitstreamCursor &Cursor;
+  };
+
+private:
   friend class Deserializer;
   NaClBitstreamReader *BitStream;
   size_t NextChar;
+  // The current error handler for the bitstream reader.
+  std::unique_ptr<ErrorHandler> ErrHandler;
 
   /// CurWord/word_t - This is the current data we have pulled from the stream
   /// but have not returned to the client.  This is specifically and
@@ -248,11 +274,12 @@ class NaClBitstreamCursor {
 
 public:
 
-  NaClBitstreamCursor() {
+  NaClBitstreamCursor() : ErrHandler(new ErrorHandler(*this)) {
     init(nullptr);
   }
 
-  explicit NaClBitstreamCursor(NaClBitstreamReader &R) { init(&R); }
+  explicit NaClBitstreamCursor(NaClBitstreamReader &R)
+      : ErrHandler(new ErrorHandler(*this)) { init(&R); }
 
   void init(NaClBitstreamReader *R) {
     freeState();
@@ -268,6 +295,13 @@ public:
 
   void freeState();
 
+  // Replaces the current bitstream error handler with the new
+  // handler. Takes ownership of the new handler and deletes it when
+  // it is no longer needed.
+  void setErrorHandler(std::unique_ptr<ErrorHandler> &NewHandler) {
+    ErrHandler = std::move(NewHandler);
+  }
+
   bool isEndPos(size_t pos) {
     return BitStream->getBitcodeBytes().isObjectEnd(static_cast<uint64_t>(pos));
   }
@@ -297,6 +331,20 @@ public:
     return BitStream;
   }
 
+  /// Returns Bit as a Byte::BitInByte string. MinByteWidth is the
+  /// minimum number of characters to print out the Byte value (blank
+  /// fills).
+  std::string getBitAddress(uint64_t Bit, unsigned MinByteWidth=1) const {
+    return BitStream->getBitAddress(Bit, MinByteWidth);

[jvoung (off chromium), 2015/02/23 18:58:56]

Is it useful to have this wrapper or can the bitstream static function be called
directly? I can see the other "getCurrentBitAddress" being more useful since it
also calls GetCurrentBitNo()

[Karl, 2015/02/23 20:46:22]

Removed.

+  }
+
+  /// Returns the current bit address (string) of the bit cursor.
+  /// MinByteWidth is the minimum number of characters to print out
+  /// the Byte value (blank fills).
+  std::string getCurrentBitAddress(unsigned MinByteWidth=1) const {
+    return BitStream->getBitAddress(GetCurrentBitNo(), MinByteWidth);
+  }
+
   /// Flags that modify the behavior of advance().
   enum {
     /// AF_DontPopBlockAtEnd - If this flag is used, the advance() method does
@@ -355,7 +403,8 @@ public:
   void JumpToBit(uint64_t BitNo) {
     uintptr_t ByteNo = uintptr_t(BitNo/8) & ~(sizeof(word_t)-1);
     unsigned WordBitNo = unsigned(BitNo & (sizeof(word_t)*8-1));
-    assert(canSkipToPos(ByteNo) && "Invalid location");
+    if (!canSkipToPos(ByteNo))
+      reportInvalidJumpToBit(BitNo);
 
     // Move the cursor to the right word.
     NextChar = ByteNo;
@@ -553,6 +602,9 @@ private:
   //===--------------------------------------------------------------------===//
 
 private:
+  // Returns abbreviation encoding associated with Value.
+  NaClBitCodeAbbrevOp::Encoding getEncoding(uint64_t Value);
+
   void skipAbbreviatedField(const NaClBitCodeAbbrevOp &Op);
 
   // Reads the next Value using the abbreviation Op. Returns true only
@@ -571,12 +623,19 @@ private:
                               unsigned NumArrayElements,
                               SmallVectorImpl<uint64_t> &Vals);
 
+  // Reports that that abbreviation Index is not valid.
+  void reportInvalidAbbrevNumber(unsigned Index) const;
+
+  // Reports that jumping to Bit is not valid.
+  void reportInvalidJumpToBit(uint64_t Bit) const;
+
 public:
 
   /// getAbbrev - Return the abbreviation for the specified AbbrevId.
   const NaClBitCodeAbbrev *getAbbrev(unsigned AbbrevID) const {
     unsigned AbbrevNo = AbbrevID-naclbitc::FIRST_APPLICATION_ABBREV;
-    assert(AbbrevNo < CurAbbrevs.size() && "Invalid abbrev #!");
+    if (AbbrevNo >= CurAbbrevs.size())
+      reportInvalidAbbrevNumber(AbbrevID);
     return CurAbbrevs[AbbrevNo];
   }