[llvm] r364464 - BitStream reader: propagate errors

JF Bastien via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 26 12:50:13 PDT 2019


Author: jfb
Date: Wed Jun 26 12:50:12 2019
New Revision: 364464

URL: http://llvm.org/viewvc/llvm-project?rev=364464&view=rev
Log:
BitStream reader: propagate errors

The bitstream reader handles errors poorly. This has two effects:

 * Bugs in file handling (especially modules) manifest as an "unexpected end of
   file" crash
 * Users of clang as a library end up aborting because the code unconditionally
   calls `report_fatal_error`

The bitstream reader should be more resilient and return Expected / Error as
soon as an error is encountered, not way late like it does now. This patch
starts doing so and adopting the error handling where I think it makes sense.
There's plenty more to do: this patch propagates errors to be minimally useful,
and follow-ups will propagate them further and improve diagnostics.

https://bugs.llvm.org/show_bug.cgi?id=42311
<rdar://problem/33159405>

Differential Revision: https://reviews.llvm.org/D63518

Modified:
    llvm/trunk/include/llvm/Bitcode/BitstreamReader.h
    llvm/trunk/include/llvm/Support/Error.h
    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
    llvm/trunk/lib/Bitcode/Reader/BitstreamReader.cpp
    llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp
    llvm/trunk/test/Bitcode/invalid.test
    llvm/trunk/test/tools/llvm-lto/error.ll
    llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp
    llvm/trunk/unittests/Bitcode/BitstreamReaderTest.cpp

Modified: llvm/trunk/include/llvm/Bitcode/BitstreamReader.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Bitcode/BitstreamReader.h?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Bitcode/BitstreamReader.h (original)
+++ llvm/trunk/include/llvm/Bitcode/BitstreamReader.h Wed Jun 26 12:50:12 2019
@@ -97,7 +97,7 @@ private:
   unsigned BitsInCurWord = 0;
 
 public:
-  static const size_t MaxChunkSize = sizeof(word_t) * 8;
+  static const constexpr size_t MaxChunkSize = sizeof(word_t) * 8;
 
   SimpleBitstreamCursor() = default;
   explicit SimpleBitstreamCursor(ArrayRef<uint8_t> BitcodeBytes)
@@ -127,7 +127,7 @@ public:
   ArrayRef<uint8_t> getBitcodeBytes() const { return BitcodeBytes; }
 
   /// Reset the stream to the specified bit number.
-  void JumpToBit(uint64_t BitNo) {
+  Error JumpToBit(uint64_t BitNo) {
     size_t ByteNo = size_t(BitNo/8) & ~(sizeof(word_t)-1);
     unsigned WordBitNo = unsigned(BitNo & (sizeof(word_t)*8-1));
     assert(canSkipToPos(ByteNo) && "Invalid location");
@@ -137,8 +137,14 @@ public:
     BitsInCurWord = 0;
 
     // Skip over any bits that are already consumed.
-    if (WordBitNo)
-      Read(WordBitNo);
+    if (WordBitNo) {
+      if (Expected<word_t> Res = Read(WordBitNo))
+        return Error::success();
+      else
+        return Res.takeError();
+    }
+
+    return Error::success();
   }
 
   /// Get a pointer into the bitstream at the specified byte offset.
@@ -154,9 +160,11 @@ public:
     return getPointerToByte(BitNo / 8, NumBytes);
   }
 
-  void fillCurWord() {
+  Error fillCurWord() {
     if (NextChar >= BitcodeBytes.size())
-      report_fatal_error("Unexpected end of file");
+      return createStringError(std::errc::io_error,
+                               "Unexpected end of file reading %u of %u bytes",
+                               NextChar, BitcodeBytes.size());
 
     // Read the next word from the stream.
     const uint8_t *NextCharPtr = BitcodeBytes.data() + NextChar;
@@ -175,9 +183,10 @@ public:
     }
     NextChar += BytesRead;
     BitsInCurWord = BytesRead * 8;
+    return Error::success();
   }
 
-  word_t Read(unsigned NumBits) {
+  Expected<word_t> Read(unsigned NumBits) {
     static const unsigned BitsInWord = MaxChunkSize;
 
     assert(NumBits && NumBits <= BitsInWord &&
@@ -199,11 +208,14 @@ public:
     word_t R = BitsInCurWord ? CurWord : 0;
     unsigned BitsLeft = NumBits - BitsInCurWord;
 
-    fillCurWord();
+    if (Error fillResult = fillCurWord())
+      return std::move(fillResult);
 
     // If we run out of data, abort.
     if (BitsLeft > BitsInCurWord)
-      report_fatal_error("Unexpected end of file");
+      return createStringError(std::errc::io_error,
+                               "Unexpected end of file reading %u of %u bits",
+                               BitsInCurWord, BitsLeft);
 
     word_t R2 = CurWord & (~word_t(0) >> (BitsInWord - BitsLeft));
 
@@ -217,8 +229,12 @@ public:
     return R;
   }
 
-  uint32_t ReadVBR(unsigned NumBits) {
-    uint32_t Piece = Read(NumBits);
+  Expected<uint32_t> ReadVBR(unsigned NumBits) {
+    Expected<unsigned> MaybeRead = Read(NumBits);
+    if (!MaybeRead)
+      return MaybeRead;
+    uint32_t Piece = MaybeRead.get();
+
     if ((Piece & (1U << (NumBits-1))) == 0)
       return Piece;
 
@@ -231,14 +247,21 @@ public:
         return Result;
 
       NextBit += NumBits-1;
-      Piece = Read(NumBits);
+      MaybeRead = Read(NumBits);
+      if (!MaybeRead)
+        return MaybeRead;
+      Piece = MaybeRead.get();
     }
   }
 
   // Read a VBR that may have a value up to 64-bits in size. The chunk size of
   // the VBR must still be <= 32 bits though.
-  uint64_t ReadVBR64(unsigned NumBits) {
-    uint32_t Piece = Read(NumBits);
+  Expected<uint64_t> ReadVBR64(unsigned NumBits) {
+    Expected<unsigned> MaybeRead = Read(NumBits);
+    if (!MaybeRead)
+      return MaybeRead;
+    uint32_t Piece = MaybeRead.get();
+
     if ((Piece & (1U << (NumBits-1))) == 0)
       return uint64_t(Piece);
 
@@ -251,7 +274,10 @@ public:
         return Result;
 
       NextBit += NumBits-1;
-      Piece = Read(NumBits);
+      MaybeRead = Read(NumBits);
+      if (!MaybeRead)
+        return MaybeRead;
+      Piece = MaybeRead.get();
     }
   }
 
@@ -365,12 +391,16 @@ public:
   };
 
   /// Advance the current bitstream, returning the next entry in the stream.
-  BitstreamEntry advance(unsigned Flags = 0) {
+  Expected<BitstreamEntry> advance(unsigned Flags = 0) {
     while (true) {
       if (AtEndOfStream())
         return BitstreamEntry::getError();
 
-      unsigned Code = ReadCode();
+      Expected<unsigned> MaybeCode = ReadCode();
+      if (!MaybeCode)
+        return MaybeCode.takeError();
+      unsigned Code = MaybeCode.get();
+
       if (Code == bitc::END_BLOCK) {
         // Pop the end of the block unless Flags tells us not to.
         if (!(Flags & AF_DontPopBlockAtEnd) && ReadBlockEnd())
@@ -378,14 +408,19 @@ public:
         return BitstreamEntry::getEndBlock();
       }
 
-      if (Code == bitc::ENTER_SUBBLOCK)
-        return BitstreamEntry::getSubBlock(ReadSubBlockID());
+      if (Code == bitc::ENTER_SUBBLOCK) {
+        if (Expected<unsigned> MaybeSubBlock = ReadSubBlockID())
+          return BitstreamEntry::getSubBlock(MaybeSubBlock.get());
+        else
+          return MaybeSubBlock.takeError();
+      }
 
       if (Code == bitc::DEFINE_ABBREV &&
           !(Flags & AF_DontAutoprocessAbbrevs)) {
         // We read and accumulate abbrev's, the client can't do anything with
         // them anyway.
-        ReadAbbrevRecord();
+        if (Error Err = ReadAbbrevRecord())
+          return std::move(Err);
         continue;
       }
 
@@ -395,53 +430,66 @@ public:
 
   /// This is a convenience function for clients that don't expect any
   /// subblocks. This just skips over them automatically.
-  BitstreamEntry advanceSkippingSubblocks(unsigned Flags = 0) {
+  Expected<BitstreamEntry> advanceSkippingSubblocks(unsigned Flags = 0) {
     while (true) {
       // If we found a normal entry, return it.
-      BitstreamEntry Entry = advance(Flags);
+      Expected<BitstreamEntry> MaybeEntry = advance(Flags);
+      if (!MaybeEntry)
+        return MaybeEntry;
+      BitstreamEntry Entry = MaybeEntry.get();
+
       if (Entry.Kind != BitstreamEntry::SubBlock)
         return Entry;
 
       // If we found a sub-block, just skip over it and check the next entry.
-      if (SkipBlock())
-        return BitstreamEntry::getError();
+      if (Error Err = SkipBlock())
+        return std::move(Err);
     }
   }
 
-  unsigned ReadCode() {
-    return Read(CurCodeSize);
-  }
+  Expected<unsigned> ReadCode() { return Read(CurCodeSize); }
 
   // Block header:
   //    [ENTER_SUBBLOCK, blockid, newcodelen, <align4bytes>, blocklen]
 
   /// Having read the ENTER_SUBBLOCK code, read the BlockID for the block.
-  unsigned ReadSubBlockID() {
-    return ReadVBR(bitc::BlockIDWidth);
-  }
+  Expected<unsigned> ReadSubBlockID() { return ReadVBR(bitc::BlockIDWidth); }
 
   /// Having read the ENTER_SUBBLOCK abbrevid and a BlockID, skip over the body
-  /// of this block. If the block record is malformed, return true.
-  bool SkipBlock() {
-    // Read and ignore the codelen value.  Since we are skipping this block, we
-    // don't care what code widths are used inside of it.
-    ReadVBR(bitc::CodeLenWidth);
+  /// of this block.
+  Error SkipBlock() {
+    // Read and ignore the codelen value.
+    if (Expected<uint32_t> Res = ReadVBR(bitc::CodeLenWidth))
+      ; // Since we are skipping this block, we don't care what code widths are
+        // used inside of it.
+    else
+      return Res.takeError();
+
     SkipToFourByteBoundary();
-    size_t NumFourBytes = Read(bitc::BlockSizeWidth);
+    Expected<unsigned> MaybeNum = Read(bitc::BlockSizeWidth);
+    if (!MaybeNum)
+      return MaybeNum.takeError();
+    size_t NumFourBytes = MaybeNum.get();
 
     // Check that the block wasn't partially defined, and that the offset isn't
     // bogus.
-    size_t SkipTo = GetCurrentBitNo() + NumFourBytes*4*8;
-    if (AtEndOfStream() || !canSkipToPos(SkipTo/8))
-      return true;
+    size_t SkipTo = GetCurrentBitNo() + NumFourBytes * 4 * 8;
+    if (AtEndOfStream())
+      return createStringError(std::errc::illegal_byte_sequence,
+                               "can't skip block: already at end of stream");
+    if (!canSkipToPos(SkipTo / 8))
+      return createStringError(std::errc::illegal_byte_sequence,
+                               "can't skip to bit %zu from %" PRIu64, SkipTo,
+                               GetCurrentBitNo());
 
-    JumpToBit(SkipTo);
-    return false;
+    if (Error Res = JumpToBit(SkipTo))
+      return Res;
+
+    return Error::success();
   }
 
-  /// Having read the ENTER_SUBBLOCK abbrevid, enter the block, and return true
-  /// if the block has an error.
-  bool EnterSubBlock(unsigned BlockID, unsigned *NumWordsP = nullptr);
+  /// Having read the ENTER_SUBBLOCK abbrevid, and enter the block.
+  Error EnterSubBlock(unsigned BlockID, unsigned *NumWordsP = nullptr);
 
   bool ReadBlockEnd() {
     if (BlockScope.empty()) return true;
@@ -476,22 +524,23 @@ public:
   }
 
   /// Read the current record and discard it, returning the code for the record.
-  unsigned skipRecord(unsigned AbbrevID);
+  Expected<unsigned> skipRecord(unsigned AbbrevID);
 
-  unsigned readRecord(unsigned AbbrevID, SmallVectorImpl<uint64_t> &Vals,
-                      StringRef *Blob = nullptr);
+  Expected<unsigned> readRecord(unsigned AbbrevID,
+                                SmallVectorImpl<uint64_t> &Vals,
+                                StringRef *Blob = nullptr);
 
   //===--------------------------------------------------------------------===//
   // Abbrev Processing
   //===--------------------------------------------------------------------===//
-  void ReadAbbrevRecord();
+  Error ReadAbbrevRecord();
 
   /// Read and return a block info block from the bitstream. If an error was
   /// encountered, return None.
   ///
   /// \param ReadBlockInfoNames Whether to read block/record name information in
   /// the BlockInfo block. Only llvm-bcanalyzer uses this.
-  Optional<BitstreamBlockInfo>
+  Expected<Optional<BitstreamBlockInfo>>
   ReadBlockInfoBlock(bool ReadBlockInfoNames = false);
 
   /// Set the block info to be used by this BitstreamCursor to interpret

Modified: llvm/trunk/include/llvm/Support/Error.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Error.h?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Error.h (original)
+++ llvm/trunk/include/llvm/Support/Error.h Wed Jun 26 12:50:12 2019
@@ -1160,8 +1160,8 @@ private:
 
 /// Create formatted StringError object.
 template <typename... Ts>
-Error createStringError(std::error_code EC, char const *Fmt,
-                        const Ts &... Vals) {
+inline Error createStringError(std::error_code EC, char const *Fmt,
+                               const Ts &... Vals) {
   std::string Buffer;
   raw_string_ostream Stream(Buffer);
   Stream << format(Fmt, Vals...);
@@ -1170,6 +1170,12 @@ Error createStringError(std::error_code
 
 Error createStringError(std::error_code EC, char const *Msg);
 
+template <typename... Ts>
+inline Error createStringError(std::errc EC, char const *Fmt,
+                               const Ts &... Vals) {
+  return createStringError(std::make_error_code(EC), Fmt, Vals...);
+}
+
 /// This class wraps a filename and another Error.
 ///
 /// In some cases, an error needs to live along a 'source' name, in order to

Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Wed Jun 26 12:50:12 2019
@@ -105,18 +105,25 @@ static Error error(const Twine &Message)
       Message, make_error_code(BitcodeError::CorruptedBitcode));
 }
 
-/// Helper to read the header common to all bitcode files.
-static bool hasValidBitcodeHeader(BitstreamCursor &Stream) {
-  // Sniff for the signature.
-  if (!Stream.canSkipToPos(4) ||
-      Stream.Read(8) != 'B' ||
-      Stream.Read(8) != 'C' ||
-      Stream.Read(4) != 0x0 ||
-      Stream.Read(4) != 0xC ||
-      Stream.Read(4) != 0xE ||
-      Stream.Read(4) != 0xD)
-    return false;
-  return true;
+static Error hasInvalidBitcodeHeader(BitstreamCursor &Stream) {
+  if (!Stream.canSkipToPos(4))
+    return createStringError(std::errc::illegal_byte_sequence,
+                             "file too small to contain bitcode header");
+  for (unsigned C : {'B', 'C'})
+    if (Expected<SimpleBitstreamCursor::word_t> Res = Stream.Read(8)) {
+      if (Res.get() != C)
+        return createStringError(std::errc::illegal_byte_sequence,
+                                 "file doesn't start with bitcode header");
+    } else
+      return Res.takeError();
+  for (unsigned C : {0x0, 0xC, 0xE, 0xD})
+    if (Expected<SimpleBitstreamCursor::word_t> Res = Stream.Read(4)) {
+      if (Res.get() != C)
+        return createStringError(std::errc::illegal_byte_sequence,
+                                 "file doesn't start with bitcode header");
+    } else
+      return Res.takeError();
+  return Error::success();
 }
 
 static Expected<BitstreamCursor> initStream(MemoryBufferRef Buffer) {
@@ -133,8 +140,8 @@ static Expected<BitstreamCursor> initStr
       return error("Invalid bitcode wrapper header");
 
   BitstreamCursor Stream(ArrayRef<uint8_t>(BufPtr, BufEnd));
-  if (!hasValidBitcodeHeader(Stream))
-    return error("Invalid bitcode signature");
+  if (Error Err = hasInvalidBitcodeHeader(Stream))
+    return std::move(Err);
 
   return std::move(Stream);
 }
@@ -164,8 +171,8 @@ static void stripTBAA(Module *M) {
 /// Read the "IDENTIFICATION_BLOCK_ID" block, do some basic enforcement on the
 /// "epoch" encoded in the bitcode, and return the producer name if any.
 static Expected<std::string> readIdentificationBlock(BitstreamCursor &Stream) {
-  if (Stream.EnterSubBlock(bitc::IDENTIFICATION_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::IDENTIFICATION_BLOCK_ID))
+    return std::move(Err);
 
   // Read all the records.
   SmallVector<uint64_t, 64> Record;
@@ -173,7 +180,11 @@ static Expected<std::string> readIdentif
   std::string ProducerIdentification;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    BitstreamEntry Entry;
+    if (Expected<BitstreamEntry> Res = Stream.advance())
+      Entry = Res.get();
+    else
+      return Res.takeError();
 
     switch (Entry.Kind) {
     default:
@@ -188,8 +199,10 @@ static Expected<std::string> readIdentif
 
     // Read a record.
     Record.clear();
-    unsigned BitCode = Stream.readRecord(Entry.ID, Record);
-    switch (BitCode) {
+    Expected<unsigned> MaybeBitCode = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeBitCode)
+      return MaybeBitCode.takeError();
+    switch (unsigned BitCode = MaybeBitCode.get()) {
     default: // Default behavior: reject
       return error("Invalid value");
     case bitc::IDENTIFICATION_CODE_STRING: // IDENTIFICATION: [strchr x N]
@@ -214,7 +227,12 @@ static Expected<std::string> readIdentif
     if (Stream.AtEndOfStream())
       return "";
 
-    BitstreamEntry Entry = Stream.advance();
+    BitstreamEntry Entry;
+    if (Expected<BitstreamEntry> Res = Stream.advance())
+      Entry = std::move(Res.get());
+    else
+      return Res.takeError();
+
     switch (Entry.Kind) {
     case BitstreamEntry::EndBlock:
     case BitstreamEntry::Error:
@@ -225,25 +243,30 @@ static Expected<std::string> readIdentif
         return readIdentificationBlock(Stream);
 
       // Ignore other sub-blocks.
-      if (Stream.SkipBlock())
-        return error("Malformed block");
+      if (Error Err = Stream.SkipBlock())
+        return std::move(Err);
       continue;
     case BitstreamEntry::Record:
-      Stream.skipRecord(Entry.ID);
-      continue;
+      if (Expected<unsigned> Skipped = Stream.skipRecord(Entry.ID))
+        continue;
+      else
+        return Skipped.takeError();
     }
   }
 }
 
 static Expected<bool> hasObjCCategoryInModule(BitstreamCursor &Stream) {
-  if (Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
+    return std::move(Err);
 
   SmallVector<uint64_t, 64> Record;
   // Read all the records for this module.
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -257,7 +280,10 @@ static Expected<bool> hasObjCCategoryInM
     }
 
     // Read a record.
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default:
       break; // Default behavior, ignore unknown content.
     case bitc::MODULE_CODE_SECTIONNAME: { // SECTIONNAME: [strchr x N]
@@ -280,7 +306,11 @@ static Expected<bool> hasObjCCategory(Bi
   // We expect a number of well-defined blocks, though we don't necessarily
   // need to understand them all.
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    BitstreamEntry Entry;
+    if (Expected<BitstreamEntry> Res = Stream.advance())
+      Entry = std::move(Res.get());
+    else
+      return Res.takeError();
 
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
@@ -293,20 +323,22 @@ static Expected<bool> hasObjCCategory(Bi
         return hasObjCCategoryInModule(Stream);
 
       // Ignore other sub-blocks.
-      if (Stream.SkipBlock())
-        return error("Malformed block");
+      if (Error Err = Stream.SkipBlock())
+        return std::move(Err);
       continue;
 
     case BitstreamEntry::Record:
-      Stream.skipRecord(Entry.ID);
-      continue;
+      if (Expected<unsigned> Skipped = Stream.skipRecord(Entry.ID))
+        continue;
+      else
+        return Skipped.takeError();
     }
   }
 }
 
 static Expected<std::string> readModuleTriple(BitstreamCursor &Stream) {
-  if (Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
+    return std::move(Err);
 
   SmallVector<uint64_t, 64> Record;
 
@@ -314,7 +346,10 @@ static Expected<std::string> readModuleT
 
   // Read all the records for this module.
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -328,7 +363,10 @@ static Expected<std::string> readModuleT
     }
 
     // Read a record.
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default: break;  // Default behavior, ignore unknown content.
     case bitc::MODULE_CODE_TRIPLE: {  // TRIPLE: [strchr x N]
       std::string S;
@@ -347,7 +385,10 @@ static Expected<std::string> readTriple(
   // We expect a number of well-defined blocks, though we don't necessarily
   // need to understand them all.
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
@@ -360,13 +401,15 @@ static Expected<std::string> readTriple(
         return readModuleTriple(Stream);
 
       // Ignore other sub-blocks.
-      if (Stream.SkipBlock())
-        return error("Malformed block");
+      if (Error Err = Stream.SkipBlock())
+        return std::move(Err);
       continue;
 
     case BitstreamEntry::Record:
-      Stream.skipRecord(Entry.ID);
-      continue;
+      if (llvm::Expected<unsigned> Skipped = Stream.skipRecord(Entry.ID))
+        continue;
+      else
+        return Skipped.takeError();
     }
   }
 }
@@ -1253,8 +1296,8 @@ static void decodeLLVMAttributesForBitco
 }
 
 Error BitcodeReader::parseAttributeBlock() {
-  if (Stream.EnterSubBlock(bitc::PARAMATTR_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::PARAMATTR_BLOCK_ID))
+    return Err;
 
   if (!MAttributes.empty())
     return error("Invalid multiple blocks");
@@ -1265,7 +1308,10 @@ Error BitcodeReader::parseAttributeBlock
 
   // Read all the records.
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -1280,7 +1326,10 @@ Error BitcodeReader::parseAttributeBlock
 
     // Read a record.
     Record.clear();
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default:  // Default behavior: ignore.
       break;
     case bitc::PARAMATTR_CODE_ENTRY_OLD: // ENTRY: [paramidx0, attr0, ...]
@@ -1454,8 +1503,8 @@ Error BitcodeReader::parseAttrKind(uint6
 }
 
 Error BitcodeReader::parseAttributeGroupBlock() {
-  if (Stream.EnterSubBlock(bitc::PARAMATTR_GROUP_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::PARAMATTR_GROUP_BLOCK_ID))
+    return Err;
 
   if (!MAttributeGroups.empty())
     return error("Invalid multiple blocks");
@@ -1464,7 +1513,10 @@ Error BitcodeReader::parseAttributeGroup
 
   // Read all the records.
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -1479,7 +1531,10 @@ Error BitcodeReader::parseAttributeGroup
 
     // Read a record.
     Record.clear();
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default:  // Default behavior: ignore.
       break;
     case bitc::PARAMATTR_GRP_CODE_ENTRY: { // ENTRY: [grpid, idx, a0, a1, ...]
@@ -1555,8 +1610,8 @@ Error BitcodeReader::parseAttributeGroup
 }
 
 Error BitcodeReader::parseTypeTable() {
-  if (Stream.EnterSubBlock(bitc::TYPE_BLOCK_ID_NEW))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::TYPE_BLOCK_ID_NEW))
+    return Err;
 
   return parseTypeTableBody();
 }
@@ -1572,7 +1627,10 @@ Error BitcodeReader::parseTypeTableBody(
 
   // Read all the records for this type table.
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -1590,7 +1648,10 @@ Error BitcodeReader::parseTypeTableBody(
     // Read a record.
     Record.clear();
     Type *ResultTy = nullptr;
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default:
       return error("Invalid value");
     case bitc::TYPE_CODE_NUMENTRY: // TYPE_CODE_NUMENTRY: [numentries]
@@ -1800,8 +1861,8 @@ Error BitcodeReader::parseTypeTableBody(
 }
 
 Error BitcodeReader::parseOperandBundleTags() {
-  if (Stream.EnterSubBlock(bitc::OPERAND_BUNDLE_TAGS_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::OPERAND_BUNDLE_TAGS_BLOCK_ID))
+    return Err;
 
   if (!BundleTags.empty())
     return error("Invalid multiple blocks");
@@ -1809,7 +1870,10 @@ Error BitcodeReader::parseOperandBundleT
   SmallVector<uint64_t, 64> Record;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -1824,7 +1888,10 @@ Error BitcodeReader::parseOperandBundleT
 
     // Tags are implicitly mapped to integers by their order.
 
-    if (Stream.readRecord(Entry.ID, Record) != bitc::OPERAND_BUNDLE_TAG)
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    if (MaybeRecord.get() != bitc::OPERAND_BUNDLE_TAG)
       return error("Invalid record");
 
     // OPERAND_BUNDLE_TAG: [strchr x N]
@@ -1836,15 +1903,19 @@ Error BitcodeReader::parseOperandBundleT
 }
 
 Error BitcodeReader::parseSyncScopeNames() {
-  if (Stream.EnterSubBlock(bitc::SYNC_SCOPE_NAMES_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::SYNC_SCOPE_NAMES_BLOCK_ID))
+    return Err;
 
   if (!SSIDs.empty())
     return error("Invalid multiple synchronization scope names blocks");
 
   SmallVector<uint64_t, 64> Record;
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
     case BitstreamEntry::Error:
@@ -1861,7 +1932,10 @@ Error BitcodeReader::parseSyncScopeNames
     // Synchronization scope names are implicitly mapped to synchronization
     // scope IDs by their order.
 
-    if (Stream.readRecord(Entry.ID, Record) != bitc::SYNC_SCOPE_NAME)
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    if (MaybeRecord.get() != bitc::SYNC_SCOPE_NAME)
       return error("Invalid record");
 
     SmallString<16> SSN;
@@ -1902,22 +1976,18 @@ Expected<Value *> BitcodeReader::recordV
 
 /// Helper to note and return the current location, and jump to the given
 /// offset.
-static uint64_t jumpToValueSymbolTable(uint64_t Offset,
-                                       BitstreamCursor &Stream) {
+static Expected<uint64_t> jumpToValueSymbolTable(uint64_t Offset,
+                                                 BitstreamCursor &Stream) {
   // Save the current parsing location so we can jump back at the end
   // of the VST read.
   uint64_t CurrentBit = Stream.GetCurrentBitNo();
-  Stream.JumpToBit(Offset * 32);
-#ifndef NDEBUG
-  // Do some checking if we are in debug mode.
-  BitstreamEntry Entry = Stream.advance();
-  assert(Entry.Kind == BitstreamEntry::SubBlock);
-  assert(Entry.ID == bitc::VALUE_SYMTAB_BLOCK_ID);
-#else
-  // In NDEBUG mode ignore the output so we don't get an unused variable
-  // warning.
-  Stream.advance();
-#endif
+  if (Error JumpFailed = Stream.JumpToBit(Offset * 32))
+    return std::move(JumpFailed);
+  Expected<BitstreamEntry> MaybeEntry = Stream.advance();
+  if (!MaybeEntry)
+    return MaybeEntry.takeError();
+  assert(MaybeEntry.get().Kind == BitstreamEntry::SubBlock);
+  assert(MaybeEntry.get().ID == bitc::VALUE_SYMTAB_BLOCK_ID);
   return CurrentBit;
 }
 
@@ -1942,12 +2012,15 @@ Error BitcodeReader::parseGlobalValueSym
   unsigned FuncBitcodeOffsetDelta =
       Stream.getAbbrevIDWidth() + bitc::BlockIDWidth;
 
-  if (Stream.EnterSubBlock(bitc::VALUE_SYMTAB_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::VALUE_SYMTAB_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock:
@@ -1960,7 +2033,10 @@ Error BitcodeReader::parseGlobalValueSym
     }
 
     Record.clear();
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     case bitc::VST_CODE_FNENTRY: // [valueid, offset]
       setDeferredFunctionInfo(FuncBitcodeOffsetDelta,
                               cast<Function>(ValueList[Record[0]]), Record);
@@ -1977,12 +2053,16 @@ Error BitcodeReader::parseValueSymbolTab
   // VST (where we want to jump to the VST offset) and the function-level
   // VST (where we don't).
   if (Offset > 0) {
-    CurrentBit = jumpToValueSymbolTable(Offset, Stream);
+    Expected<uint64_t> MaybeCurrentBit = jumpToValueSymbolTable(Offset, Stream);
+    if (!MaybeCurrentBit)
+      return MaybeCurrentBit.takeError();
+    CurrentBit = MaybeCurrentBit.get();
     // If this module uses a string table, read this as a module-level VST.
     if (UseStrtab) {
       if (Error Err = parseGlobalValueSymbolTable())
         return Err;
-      Stream.JumpToBit(CurrentBit);
+      if (Error JumpFailed = Stream.JumpToBit(CurrentBit))
+        return JumpFailed;
       return Error::success();
     }
     // Otherwise, the VST will be in a similar format to a function-level VST,
@@ -2003,8 +2083,8 @@ Error BitcodeReader::parseValueSymbolTab
   unsigned FuncBitcodeOffsetDelta =
       Stream.getAbbrevIDWidth() + bitc::BlockIDWidth;
 
-  if (Stream.EnterSubBlock(bitc::VALUE_SYMTAB_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::VALUE_SYMTAB_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
 
@@ -2014,7 +2094,10 @@ Error BitcodeReader::parseValueSymbolTab
   SmallString<128> ValueName;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -2022,7 +2105,8 @@ Error BitcodeReader::parseValueSymbolTab
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       if (Offset > 0)
-        Stream.JumpToBit(CurrentBit);
+        if (Error JumpFailed = Stream.JumpToBit(CurrentBit))
+          return JumpFailed;
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
@@ -2031,7 +2115,10 @@ Error BitcodeReader::parseValueSymbolTab
 
     // Read a record.
     Record.clear();
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default:  // Default behavior: unknown type.
       break;
     case bitc::VST_CODE_ENTRY: {  // VST_CODE_ENTRY: [valueid, namechar x N]
@@ -2176,8 +2263,8 @@ static APInt readWideAPInt(ArrayRef<uint
 }
 
 Error BitcodeReader::parseConstants() {
-  if (Stream.EnterSubBlock(bitc::CONSTANTS_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::CONSTANTS_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
 
@@ -2186,7 +2273,10 @@ Error BitcodeReader::parseConstants() {
   unsigned NextCstNo = ValueList.size();
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -2209,8 +2299,10 @@ Error BitcodeReader::parseConstants() {
     Record.clear();
     Type *VoidType = Type::getVoidTy(Context);
     Value *V = nullptr;
-    unsigned BitCode = Stream.readRecord(Entry.ID, Record);
-    switch (BitCode) {
+    Expected<unsigned> MaybeBitCode = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeBitCode)
+      return MaybeBitCode.takeError();
+    switch (unsigned BitCode = MaybeBitCode.get()) {
     default:  // Default behavior: unknown constant
     case bitc::CST_CODE_UNDEF:     // UNDEF
       V = UndefValue::get(CurTy);
@@ -2669,14 +2761,17 @@ Error BitcodeReader::parseConstants() {
 }
 
 Error BitcodeReader::parseUseLists() {
-  if (Stream.EnterSubBlock(bitc::USELIST_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::USELIST_BLOCK_ID))
+    return Err;
 
   // Read all the records.
   SmallVector<uint64_t, 64> Record;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -2692,7 +2787,10 @@ Error BitcodeReader::parseUseLists() {
     // Read a use list record.
     Record.clear();
     bool IsBB = false;
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default:  // Default behavior: unknown type.
       break;
     case bitc::USELIST_CODE_BB:
@@ -2741,15 +2839,16 @@ Error BitcodeReader::rememberAndSkipMeta
   DeferredMetadataInfo.push_back(CurBit);
 
   // Skip over the block for now.
-  if (Stream.SkipBlock())
-    return error("Invalid record");
+  if (Error Err = Stream.SkipBlock())
+    return Err;
   return Error::success();
 }
 
 Error BitcodeReader::materializeMetadata() {
   for (uint64_t BitPos : DeferredMetadataInfo) {
     // Move the bit stream to the saved position.
-    Stream.JumpToBit(BitPos);
+    if (Error JumpFailed = Stream.JumpToBit(BitPos))
+      return JumpFailed;
     if (Error Err = MDLoader->parseModuleMetadata())
       return Err;
   }
@@ -2787,8 +2886,8 @@ Error BitcodeReader::rememberAndSkipFunc
   DeferredFunctionInfo[Fn] = CurBit;
 
   // Skip over the function block for now.
-  if (Stream.SkipBlock())
-    return error("Invalid record");
+  if (Error Err = Stream.SkipBlock())
+    return Err;
   return Error::success();
 }
 
@@ -2835,7 +2934,8 @@ Error BitcodeReader::globalCleanup() {
 /// or if we have an anonymous function being materialized, since anonymous
 /// functions do not have a name and are therefore not in the VST.
 Error BitcodeReader::rememberAndSkipFunctionBodies() {
-  Stream.JumpToBit(NextUnreadBit);
+  if (Error JumpFailed = Stream.JumpToBit(NextUnreadBit))
+    return JumpFailed;
 
   if (Stream.AtEndOfStream())
     return error("Could not find function in stream");
@@ -2850,7 +2950,11 @@ Error BitcodeReader::rememberAndSkipFunc
   SmallVector<uint64_t, 64> Record;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     default:
       return error("Expect SubBlock");
@@ -2869,7 +2973,12 @@ Error BitcodeReader::rememberAndSkipFunc
 }
 
 bool BitcodeReaderBase::readBlockInfo() {
-  Optional<BitstreamBlockInfo> NewBlockInfo = Stream.ReadBlockInfoBlock();
+  Expected<Optional<BitstreamBlockInfo>> MaybeNewBlockInfo =
+      Stream.ReadBlockInfoBlock();
+  if (!MaybeNewBlockInfo)
+    return true; // FIXME Handle the error.
+  Optional<BitstreamBlockInfo> NewBlockInfo =
+      std::move(MaybeNewBlockInfo.get());
   if (!NewBlockInfo)
     return true;
   BlockInfo = std::move(*NewBlockInfo);
@@ -3205,16 +3314,20 @@ Error BitcodeReader::parseGlobalIndirect
 
 Error BitcodeReader::parseModule(uint64_t ResumeBit,
                                  bool ShouldLazyLoadMetadata) {
-  if (ResumeBit)
-    Stream.JumpToBit(ResumeBit);
-  else if (Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
-    return error("Invalid record");
+  if (ResumeBit) {
+    if (Error JumpFailed = Stream.JumpToBit(ResumeBit))
+      return JumpFailed;
+  } else if (Error Err = Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
 
   // Read all the records for this module.
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
@@ -3225,8 +3338,8 @@ Error BitcodeReader::parseModule(uint64_
     case BitstreamEntry::SubBlock:
       switch (Entry.ID) {
       default:  // Skip unknown content.
-        if (Stream.SkipBlock())
-          return error("Invalid record");
+        if (Error Err = Stream.SkipBlock())
+          return Err;
         break;
       case bitc::BLOCKINFO_BLOCK_ID:
         if (readBlockInfo())
@@ -3259,8 +3372,8 @@ Error BitcodeReader::parseModule(uint64_
           // We must have had a VST forward declaration record, which caused
           // the parser to jump to and parse the VST earlier.
           assert(VSTOffset > 0);
-          if (Stream.SkipBlock())
-            return error("Invalid record");
+          if (Error Err = Stream.SkipBlock())
+            return Err;
         }
         break;
       case bitc::CONSTANTS_BLOCK_ID:
@@ -3312,8 +3425,8 @@ Error BitcodeReader::parseModule(uint64_
             // materializing functions. The ResumeBit points to the
             // start of the last function block recorded in the
             // DeferredFunctionInfo map. Skip it.
-            if (Stream.SkipBlock())
-              return error("Invalid record");
+            if (Error Err = Stream.SkipBlock())
+              return Err;
             continue;
           }
         }
@@ -3357,8 +3470,10 @@ Error BitcodeReader::parseModule(uint64_
     }
 
     // Read a record.
-    auto BitCode = Stream.readRecord(Entry.ID, Record);
-    switch (BitCode) {
+    Expected<unsigned> MaybeBitCode = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeBitCode)
+      return MaybeBitCode.takeError();
+    switch (unsigned BitCode = MaybeBitCode.get()) {
     default: break;  // Default behavior, ignore unknown content.
     case bitc::MODULE_CODE_VERSION: {
       Expected<unsigned> VersionOrErr = parseVersionRecord(Record);
@@ -3485,8 +3600,8 @@ void BitcodeReader::propagateByValTypes(
 
 /// Lazily parse the specified function body block.
 Error BitcodeReader::parseFunctionBody(Function *F) {
-  if (Stream.EnterSubBlock(bitc::FUNCTION_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::FUNCTION_BLOCK_ID))
+    return Err;
 
   // Unexpected unresolved metadata when parsing function.
   if (MDLoader->hasFwdRefs())
@@ -3520,7 +3635,10 @@ Error BitcodeReader::parseFunctionBody(F
   SmallVector<uint64_t, 64> Record;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
@@ -3531,8 +3649,8 @@ Error BitcodeReader::parseFunctionBody(F
     case BitstreamEntry::SubBlock:
       switch (Entry.ID) {
       default:  // Skip unknown content.
-        if (Stream.SkipBlock())
-          return error("Invalid record");
+        if (Error Err = Stream.SkipBlock())
+          return Err;
         break;
       case bitc::CONSTANTS_BLOCK_ID:
         if (Error Err = parseConstants())
@@ -3568,8 +3686,10 @@ Error BitcodeReader::parseFunctionBody(F
     // Read a record.
     Record.clear();
     Instruction *I = nullptr;
-    unsigned BitCode = Stream.readRecord(Entry.ID, Record);
-    switch (BitCode) {
+    Expected<unsigned> MaybeBitCode = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeBitCode)
+      return MaybeBitCode.takeError();
+    switch (unsigned BitCode = MaybeBitCode.get()) {
     default: // Default behavior: reject
       return error("Invalid value");
     case bitc::FUNC_CODE_DECLAREBLOCKS: {   // DECLAREBLOCKS: [nblocks]
@@ -4922,8 +5042,8 @@ Error BitcodeReader::materialize(GlobalV
     return Err;
 
   // Move the bit stream to the saved position of the deferred function body.
-  Stream.JumpToBit(DFII->second);
-
+  if (Error JumpFailed = Stream.JumpToBit(DFII->second))
+    return JumpFailed;
   if (Error Err = parseFunctionBody(F))
     return Err;
   F->setIsMaterializable(false);
@@ -5086,10 +5206,13 @@ Error ModuleSummaryIndexBitcodeReader::p
     return Error::success();
 
   assert(Offset > 0 && "Expected non-zero VST offset");
-  uint64_t CurrentBit = jumpToValueSymbolTable(Offset, Stream);
+  Expected<uint64_t> MaybeCurrentBit = jumpToValueSymbolTable(Offset, Stream);
+  if (!MaybeCurrentBit)
+    return MaybeCurrentBit.takeError();
+  uint64_t CurrentBit = MaybeCurrentBit.get();
 
-  if (Stream.EnterSubBlock(bitc::VALUE_SYMTAB_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::VALUE_SYMTAB_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
 
@@ -5097,7 +5220,10 @@ Error ModuleSummaryIndexBitcodeReader::p
   SmallString<128> ValueName;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -5105,7 +5231,8 @@ Error ModuleSummaryIndexBitcodeReader::p
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       // Done parsing VST, jump back to wherever we came from.
-      Stream.JumpToBit(CurrentBit);
+      if (Error JumpFailed = Stream.JumpToBit(CurrentBit))
+        return JumpFailed;
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
@@ -5114,7 +5241,10 @@ Error ModuleSummaryIndexBitcodeReader::p
 
     // Read a record.
     Record.clear();
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default: // Default behavior: ignore (e.g. VST_CODE_BBENTRY records).
       break;
     case bitc::VST_CODE_ENTRY: { // VST_CODE_ENTRY: [valueid, namechar x N]
@@ -5162,8 +5292,8 @@ Error ModuleSummaryIndexBitcodeReader::p
 // At the end of this routine the module Index is populated with a map
 // from global value id to GlobalValueSummary objects.
 Error ModuleSummaryIndexBitcodeReader::parseModule() {
-  if (Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
   DenseMap<unsigned, GlobalValue::LinkageTypes> ValueIdToLinkageMap;
@@ -5171,7 +5301,10 @@ Error ModuleSummaryIndexBitcodeReader::p
 
   // Read the index for this module.
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
@@ -5182,8 +5315,8 @@ Error ModuleSummaryIndexBitcodeReader::p
     case BitstreamEntry::SubBlock:
       switch (Entry.ID) {
       default: // Skip unknown content.
-        if (Stream.SkipBlock())
-          return error("Invalid record");
+        if (Error Err = Stream.SkipBlock())
+          return Err;
         break;
       case bitc::BLOCKINFO_BLOCK_ID:
         // Need to parse these to get abbrev ids (e.g. for VST)
@@ -5196,8 +5329,8 @@ Error ModuleSummaryIndexBitcodeReader::p
         assert(((SeenValueSymbolTable && VSTOffset > 0) ||
                 !SeenGlobalValSummary) &&
                "Expected early VST parse via VSTOffset record");
-        if (Stream.SkipBlock())
-          return error("Invalid record");
+        if (Error Err = Stream.SkipBlock())
+          return Err;
         break;
       case bitc::GLOBALVAL_SUMMARY_BLOCK_ID:
       case bitc::FULL_LTO_GLOBALVAL_SUMMARY_BLOCK_ID:
@@ -5228,8 +5361,10 @@ Error ModuleSummaryIndexBitcodeReader::p
 
     case BitstreamEntry::Record: {
         Record.clear();
-        auto BitCode = Stream.readRecord(Entry.ID, Record);
-        switch (BitCode) {
+        Expected<unsigned> MaybeBitCode = Stream.readRecord(Entry.ID, Record);
+        if (!MaybeBitCode)
+          return MaybeBitCode.takeError();
+        switch (unsigned BitCode = MaybeBitCode.get()) {
         default:
           break; // Default behavior, ignore unknown content.
         case bitc::MODULE_CODE_VERSION: {
@@ -5386,16 +5521,23 @@ static void setImmutableRefs(std::vector
 // Eagerly parse the entire summary block. This populates the GlobalValueSummary
 // objects in the index.
 Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
-  if (Stream.EnterSubBlock(ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(ID))
+    return Err;
   SmallVector<uint64_t, 64> Record;
 
   // Parse version
   {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
+
     if (Entry.Kind != BitstreamEntry::Record)
       return error("Invalid Summary Block: record for version expected");
-    if (Stream.readRecord(Entry.ID, Record) != bitc::FS_VERSION)
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    if (MaybeRecord.get() != bitc::FS_VERSION)
       return error("Invalid Summary Block: version expected");
   }
   const uint64_t Version = Record[0];
@@ -5420,7 +5562,10 @@ Error ModuleSummaryIndexBitcodeReader::p
       PendingTypeCheckedLoadConstVCalls;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -5441,8 +5586,10 @@ Error ModuleSummaryIndexBitcodeReader::p
     // in the combined index VST entries). The records also contain
     // information used for ThinLTO renaming and importing.
     Record.clear();
-    auto BitCode = Stream.readRecord(Entry.ID, Record);
-    switch (BitCode) {
+    Expected<unsigned> MaybeBitCode = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeBitCode)
+      return MaybeBitCode.takeError();
+    switch (unsigned BitCode = MaybeBitCode.get()) {
     default: // Default behavior: ignore.
       break;
     case bitc::FS_FLAGS: {  // [flags]
@@ -5765,8 +5912,8 @@ Error ModuleSummaryIndexBitcodeReader::p
 // Parse the  module string table block into the Index.
 // This populates the ModulePathStringTable map in the index.
 Error ModuleSummaryIndexBitcodeReader::parseModuleStringTable() {
-  if (Stream.EnterSubBlock(bitc::MODULE_STRTAB_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::MODULE_STRTAB_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
 
@@ -5774,7 +5921,10 @@ Error ModuleSummaryIndexBitcodeReader::p
   ModuleSummaryIndex::ModuleInfo *LastSeenModule = nullptr;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -5788,7 +5938,10 @@ Error ModuleSummaryIndexBitcodeReader::p
     }
 
     Record.clear();
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default: // Default behavior: ignore.
       break;
     case bitc::MST_CODE_ENTRY: {
@@ -5854,12 +6007,16 @@ const std::error_category &llvm::Bitcode
 
 static Expected<StringRef> readBlobInRecord(BitstreamCursor &Stream,
                                             unsigned Block, unsigned RecordID) {
-  if (Stream.EnterSubBlock(Block))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(Block))
+    return std::move(Err);
 
   StringRef Strtab;
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case BitstreamEntry::EndBlock:
       return Strtab;
@@ -5868,14 +6025,18 @@ static Expected<StringRef> readBlobInRec
       return error("Malformed block");
 
     case BitstreamEntry::SubBlock:
-      if (Stream.SkipBlock())
-        return error("Malformed block");
+      if (Error Err = Stream.SkipBlock())
+        return std::move(Err);
       break;
 
     case BitstreamEntry::Record:
       StringRef Blob;
       SmallVector<uint64_t, 1> Record;
-      if (Stream.readRecord(Entry.ID, Record, &Blob) == RecordID)
+      Expected<unsigned> MaybeRecord =
+          Stream.readRecord(Entry.ID, Record, &Blob);
+      if (!MaybeRecord)
+        return MaybeRecord.takeError();
+      if (MaybeRecord.get() == RecordID)
         Strtab = Blob;
       break;
     }
@@ -5911,7 +6072,11 @@ llvm::getBitcodeFileContents(MemoryBuffe
     if (BCBegin + 8 >= Stream.getBitcodeBytes().size())
       return F;
 
-    BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case BitstreamEntry::EndBlock:
     case BitstreamEntry::Error:
@@ -5921,10 +6086,16 @@ llvm::getBitcodeFileContents(MemoryBuffe
       uint64_t IdentificationBit = -1ull;
       if (Entry.ID == bitc::IDENTIFICATION_BLOCK_ID) {
         IdentificationBit = Stream.GetCurrentBitNo() - BCBegin * 8;
-        if (Stream.SkipBlock())
-          return error("Malformed block");
+        if (Error Err = Stream.SkipBlock())
+          return std::move(Err);
+
+        {
+          Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+          if (!MaybeEntry)
+            return MaybeEntry.takeError();
+          Entry = MaybeEntry.get();
+        }
 
-        Entry = Stream.advance();
         if (Entry.Kind != BitstreamEntry::SubBlock ||
             Entry.ID != bitc::MODULE_BLOCK_ID)
           return error("Malformed block");
@@ -5932,8 +6103,8 @@ llvm::getBitcodeFileContents(MemoryBuffe
 
       if (Entry.ID == bitc::MODULE_BLOCK_ID) {
         uint64_t ModuleBit = Stream.GetCurrentBitNo() - BCBegin * 8;
-        if (Stream.SkipBlock())
-          return error("Malformed block");
+        if (Error Err = Stream.SkipBlock())
+          return std::move(Err);
 
         F.Mods.push_back({Stream.getBitcodeBytes().slice(
                               BCBegin, Stream.getCurrentByteNo() - BCBegin),
@@ -5981,13 +6152,15 @@ llvm::getBitcodeFileContents(MemoryBuffe
         continue;
       }
 
-      if (Stream.SkipBlock())
-        return error("Malformed block");
+      if (Error Err = Stream.SkipBlock())
+        return std::move(Err);
       continue;
     }
     case BitstreamEntry::Record:
-      Stream.skipRecord(Entry.ID);
-      continue;
+      if (Expected<unsigned> StreamFailed = Stream.skipRecord(Entry.ID))
+        continue;
+      else
+        return StreamFailed.takeError();
     }
   }
 }
@@ -6007,7 +6180,8 @@ BitcodeModule::getModuleImpl(LLVMContext
 
   std::string ProducerIdentification;
   if (IdentificationBit != -1ull) {
-    Stream.JumpToBit(IdentificationBit);
+    if (Error JumpFailed = Stream.JumpToBit(IdentificationBit))
+      return std::move(JumpFailed);
     Expected<std::string> ProducerIdentificationOrErr =
         readIdentificationBlock(Stream);
     if (!ProducerIdentificationOrErr)
@@ -6016,7 +6190,8 @@ BitcodeModule::getModuleImpl(LLVMContext
     ProducerIdentification = *ProducerIdentificationOrErr;
   }
 
-  Stream.JumpToBit(ModuleBit);
+  if (Error JumpFailed = Stream.JumpToBit(ModuleBit))
+    return std::move(JumpFailed);
   auto *R = new BitcodeReader(std::move(Stream), Strtab, ProducerIdentification,
                               Context);
 
@@ -6054,7 +6229,8 @@ BitcodeModule::getLazyModule(LLVMContext
 Error BitcodeModule::readSummary(ModuleSummaryIndex &CombinedIndex,
                                  StringRef ModulePath, uint64_t ModuleId) {
   BitstreamCursor Stream(Buffer);
-  Stream.JumpToBit(ModuleBit);
+  if (Error JumpFailed = Stream.JumpToBit(ModuleBit))
+    return JumpFailed;
 
   ModuleSummaryIndexBitcodeReader R(std::move(Stream), Strtab, CombinedIndex,
                                     ModulePath, ModuleId);
@@ -6064,7 +6240,8 @@ Error BitcodeModule::readSummary(ModuleS
 // Parse the specified bitcode buffer, returning the function info index.
 Expected<std::unique_ptr<ModuleSummaryIndex>> BitcodeModule::getSummary() {
   BitstreamCursor Stream(Buffer);
-  Stream.JumpToBit(ModuleBit);
+  if (Error JumpFailed = Stream.JumpToBit(ModuleBit))
+    return std::move(JumpFailed);
 
   auto Index = llvm::make_unique<ModuleSummaryIndex>(/*HaveGVs=*/false);
   ModuleSummaryIndexBitcodeReader R(std::move(Stream), Strtab, *Index,
@@ -6078,12 +6255,15 @@ Expected<std::unique_ptr<ModuleSummaryIn
 
 static Expected<bool> getEnableSplitLTOUnitFlag(BitstreamCursor &Stream,
                                                 unsigned ID) {
-  if (Stream.EnterSubBlock(ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(ID))
+    return std::move(Err);
   SmallVector<uint64_t, 64> Record;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -6100,8 +6280,10 @@ static Expected<bool> getEnableSplitLTOU
 
     // Look for the FS_FLAGS record.
     Record.clear();
-    auto BitCode = Stream.readRecord(Entry.ID, Record);
-    switch (BitCode) {
+    Expected<unsigned> MaybeBitCode = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeBitCode)
+      return MaybeBitCode.takeError();
+    switch (MaybeBitCode.get()) {
     default: // Default behavior: ignore.
       break;
     case bitc::FS_FLAGS: { // [flags]
@@ -6119,13 +6301,17 @@ static Expected<bool> getEnableSplitLTOU
 // Check if the given bitcode buffer contains a global value summary block.
 Expected<BitcodeLTOInfo> BitcodeModule::getLTOInfo() {
   BitstreamCursor Stream(Buffer);
-  Stream.JumpToBit(ModuleBit);
+  if (Error JumpFailed = Stream.JumpToBit(ModuleBit))
+    return std::move(JumpFailed);
 
-  if (Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
+    return std::move(Err);
 
   while (true) {
-    BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
@@ -6154,13 +6340,15 @@ Expected<BitcodeLTOInfo> BitcodeModule::
       }
 
       // Ignore other sub-blocks.
-      if (Stream.SkipBlock())
-        return error("Malformed block");
+      if (Error Err = Stream.SkipBlock())
+        return std::move(Err);
       continue;
 
     case BitstreamEntry::Record:
-      Stream.skipRecord(Entry.ID);
-      continue;
+      if (Expected<unsigned> StreamFailed = Stream.skipRecord(Entry.ID))
+        continue;
+      else
+        return StreamFailed.takeError();
     }
   }
 }

Modified: llvm/trunk/lib/Bitcode/Reader/BitstreamReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitstreamReader.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/BitstreamReader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/BitstreamReader.cpp Wed Jun 26 12:50:12 2019
@@ -17,9 +17,8 @@ using namespace llvm;
 //  BitstreamCursor implementation
 //===----------------------------------------------------------------------===//
 
-/// EnterSubBlock - Having read the ENTER_SUBBLOCK abbrevid, enter
-/// the block, and return true if the block has an error.
-bool BitstreamCursor::EnterSubBlock(unsigned BlockID, unsigned *NumWordsP) {
+/// Having read the ENTER_SUBBLOCK abbrevid, enter the block.
+Error BitstreamCursor::EnterSubBlock(unsigned BlockID, unsigned *NumWordsP) {
   // Save the current block's state on BlockScope.
   BlockScope.push_back(Block(CurCodeSize));
   BlockScope.back().PrevAbbrevs.swap(CurAbbrevs);
@@ -34,21 +33,39 @@ bool BitstreamCursor::EnterSubBlock(unsi
   }
 
   // Get the codesize of this block.
-  CurCodeSize = ReadVBR(bitc::CodeLenWidth);
-  // We can't read more than MaxChunkSize at a time
+  Expected<uint32_t> MaybeVBR = ReadVBR(bitc::CodeLenWidth);
+  if (!MaybeVBR)
+    return MaybeVBR.takeError();
+  CurCodeSize = MaybeVBR.get();
+
   if (CurCodeSize > MaxChunkSize)
-    return true;
+    return llvm::createStringError(
+        std::errc::illegal_byte_sequence,
+        "can't read more than %zu at a time, trying to read %u", +MaxChunkSize,
+        CurCodeSize);
 
   SkipToFourByteBoundary();
-  unsigned NumWords = Read(bitc::BlockSizeWidth);
-  if (NumWordsP) *NumWordsP = NumWords;
+  Expected<word_t> MaybeNum = Read(bitc::BlockSizeWidth);
+  if (!MaybeNum)
+    return MaybeNum.takeError();
+  word_t NumWords = MaybeNum.get();
+  if (NumWordsP)
+    *NumWordsP = NumWords;
+
+  if (CurCodeSize == 0)
+    return llvm::createStringError(
+        std::errc::illegal_byte_sequence,
+        "can't enter sub-block: current code size is 0");
+  if (AtEndOfStream())
+    return llvm::createStringError(
+        std::errc::illegal_byte_sequence,
+        "can't enter sub block: already at end of stream");
 
-  // Validate that this block is sane.
-  return CurCodeSize == 0 || AtEndOfStream();
+  return Error::success();
 }
 
-static uint64_t readAbbreviatedField(BitstreamCursor &Cursor,
-                                     const BitCodeAbbrevOp &Op) {
+static Expected<uint64_t> readAbbreviatedField(BitstreamCursor &Cursor,
+                                               const BitCodeAbbrevOp &Op) {
   assert(!Op.isLiteral() && "Not to be used with literals!");
 
   // Decode the value as we are commanded.
@@ -63,13 +80,16 @@ static uint64_t readAbbreviatedField(Bit
     assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
     return Cursor.ReadVBR64((unsigned)Op.getEncodingData());
   case BitCodeAbbrevOp::Char6:
-    return BitCodeAbbrevOp::DecodeChar6(Cursor.Read(6));
+    if (Expected<unsigned> Res = Cursor.Read(6))
+      return BitCodeAbbrevOp::DecodeChar6(Res.get());
+    else
+      return Res.takeError();
   }
   llvm_unreachable("invalid abbreviation encoding");
 }
 
-static void skipAbbreviatedField(BitstreamCursor &Cursor,
-                                 const BitCodeAbbrevOp &Op) {
+static Error skipAbbreviatedField(BitstreamCursor &Cursor,
+                                  const BitCodeAbbrevOp &Op) {
   assert(!Op.isLiteral() && "Not to be used with literals!");
 
   // Decode the value as we are commanded.
@@ -79,26 +99,43 @@ static void skipAbbreviatedField(Bitstre
     llvm_unreachable("Should not reach here");
   case BitCodeAbbrevOp::Fixed:
     assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
-    Cursor.Read((unsigned)Op.getEncodingData());
-    break;
+    if (Expected<unsigned> Res = Cursor.Read((unsigned)Op.getEncodingData()))
+      break;
+    else
+      return Res.takeError();
   case BitCodeAbbrevOp::VBR:
     assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
-    Cursor.ReadVBR64((unsigned)Op.getEncodingData());
-    break;
+    if (Expected<uint64_t> Res =
+            Cursor.ReadVBR64((unsigned)Op.getEncodingData()))
+      break;
+    else
+      return Res.takeError();
   case BitCodeAbbrevOp::Char6:
-    Cursor.Read(6);
-    break;
+    if (Expected<unsigned> Res = Cursor.Read(6))
+      break;
+    else
+      return Res.takeError();
   }
+  return ErrorSuccess();
 }
 
 /// skipRecord - Read the current record and discard it.
-unsigned BitstreamCursor::skipRecord(unsigned AbbrevID) {
+Expected<unsigned> BitstreamCursor::skipRecord(unsigned AbbrevID) {
   // Skip unabbreviated records by reading past their entries.
   if (AbbrevID == bitc::UNABBREV_RECORD) {
-    unsigned Code = ReadVBR(6);
-    unsigned NumElts = ReadVBR(6);
+    Expected<uint32_t> MaybeCode = ReadVBR(6);
+    if (!MaybeCode)
+      return MaybeCode.takeError();
+    unsigned Code = MaybeCode.get();
+    Expected<uint32_t> MaybeVBR = ReadVBR(6);
+    if (!MaybeVBR)
+      return MaybeVBR.get();
+    unsigned NumElts = MaybeVBR.get();
     for (unsigned i = 0; i != NumElts; ++i)
-      (void)ReadVBR64(6);
+      if (Expected<uint64_t> Res = ReadVBR64(6))
+        ; // Skip!
+      else
+        return Res.takeError();
     return Code;
   }
 
@@ -110,8 +147,13 @@ unsigned BitstreamCursor::skipRecord(uns
   else {
     if (CodeOp.getEncoding() == BitCodeAbbrevOp::Array ||
         CodeOp.getEncoding() == BitCodeAbbrevOp::Blob)
-      report_fatal_error("Abbreviation starts with an Array or a Blob");
-    Code = readAbbreviatedField(*this, CodeOp);
+      return llvm::createStringError(
+          std::errc::illegal_byte_sequence,
+          "Abbreviation starts with an Array or a Blob");
+    Expected<uint64_t> MaybeCode = readAbbreviatedField(*this, CodeOp);
+    if (!MaybeCode)
+      return MaybeCode.takeError();
+    Code = MaybeCode.get();
   }
 
   for (unsigned i = 1, e = Abbv->getNumOperandInfos(); i < e; ++i) {
@@ -121,13 +163,17 @@ unsigned BitstreamCursor::skipRecord(uns
 
     if (Op.getEncoding() != BitCodeAbbrevOp::Array &&
         Op.getEncoding() != BitCodeAbbrevOp::Blob) {
-      skipAbbreviatedField(*this, Op);
+      if (Error Err = skipAbbreviatedField(*this, Op))
+        return std::move(Err);
       continue;
     }
 
     if (Op.getEncoding() == BitCodeAbbrevOp::Array) {
       // Array case.  Read the number of elements as a vbr6.
-      unsigned NumElts = ReadVBR(6);
+      Expected<uint32_t> MaybeNum = ReadVBR(6);
+      if (!MaybeNum)
+        return MaybeNum.takeError();
+      unsigned NumElts = MaybeNum.get();
 
       // Get the element encoding.
       assert(i+2 == e && "array op not second to last?");
@@ -140,15 +186,22 @@ unsigned BitstreamCursor::skipRecord(uns
         report_fatal_error("Array element type can't be an Array or a Blob");
       case BitCodeAbbrevOp::Fixed:
         assert((unsigned)EltEnc.getEncodingData() <= MaxChunkSize);
-        JumpToBit(GetCurrentBitNo() + NumElts * EltEnc.getEncodingData());
+        if (Error Err = JumpToBit(GetCurrentBitNo() +
+                                  NumElts * EltEnc.getEncodingData()))
+          return std::move(Err);
         break;
       case BitCodeAbbrevOp::VBR:
         assert((unsigned)EltEnc.getEncodingData() <= MaxChunkSize);
         for (; NumElts; --NumElts)
-          ReadVBR64((unsigned)EltEnc.getEncodingData());
+          if (Expected<uint64_t> Res =
+                  ReadVBR64((unsigned)EltEnc.getEncodingData()))
+            ; // Skip!
+          else
+            return Res.takeError();
         break;
       case BitCodeAbbrevOp::Char6:
-        JumpToBit(GetCurrentBitNo() + NumElts * 6);
+        if (Error Err = JumpToBit(GetCurrentBitNo() + NumElts * 6))
+          return std::move(Err);
         break;
       }
       continue;
@@ -156,7 +209,10 @@ unsigned BitstreamCursor::skipRecord(uns
 
     assert(Op.getEncoding() == BitCodeAbbrevOp::Blob);
     // Blob case.  Read the number of bytes as a vbr6.
-    unsigned NumElts = ReadVBR(6);
+    Expected<uint32_t> MaybeNum = ReadVBR(6);
+    if (!MaybeNum)
+      return MaybeNum.takeError();
+    unsigned NumElts = MaybeNum.get();
     SkipToFourByteBoundary();  // 32-bit alignment
 
     // Figure out where the end of this blob will be including tail padding.
@@ -170,19 +226,30 @@ unsigned BitstreamCursor::skipRecord(uns
     }
 
     // Skip over the blob.
-    JumpToBit(NewEnd);
+    if (Error Err = JumpToBit(NewEnd))
+      return std::move(Err);
   }
   return Code;
 }
 
-unsigned BitstreamCursor::readRecord(unsigned AbbrevID,
-                                     SmallVectorImpl<uint64_t> &Vals,
-                                     StringRef *Blob) {
+Expected<unsigned> BitstreamCursor::readRecord(unsigned AbbrevID,
+                                               SmallVectorImpl<uint64_t> &Vals,
+                                               StringRef *Blob) {
   if (AbbrevID == bitc::UNABBREV_RECORD) {
-    unsigned Code = ReadVBR(6);
-    unsigned NumElts = ReadVBR(6);
+    Expected<uint32_t> MaybeCode = ReadVBR(6);
+    if (!MaybeCode)
+      return MaybeCode.takeError();
+    uint32_t Code = MaybeCode.get();
+    Expected<uint32_t> MaybeNumElts = ReadVBR(6);
+    if (!MaybeNumElts)
+      return MaybeNumElts.takeError();
+    uint32_t NumElts = MaybeNumElts.get();
+
     for (unsigned i = 0; i != NumElts; ++i)
-      Vals.push_back(ReadVBR64(6));
+      if (Expected<uint64_t> MaybeVal = ReadVBR64(6))
+        Vals.push_back(MaybeVal.get());
+      else
+        return MaybeVal.takeError();
     return Code;
   }
 
@@ -198,7 +265,10 @@ unsigned BitstreamCursor::readRecord(uns
     if (CodeOp.getEncoding() == BitCodeAbbrevOp::Array ||
         CodeOp.getEncoding() == BitCodeAbbrevOp::Blob)
       report_fatal_error("Abbreviation starts with an Array or a Blob");
-    Code = readAbbreviatedField(*this, CodeOp);
+    if (Expected<uint64_t> MaybeCode = readAbbreviatedField(*this, CodeOp))
+      Code = MaybeCode.get();
+    else
+      return MaybeCode.takeError();
   }
 
   for (unsigned i = 1, e = Abbv->getNumOperandInfos(); i != e; ++i) {
@@ -210,13 +280,19 @@ unsigned BitstreamCursor::readRecord(uns
 
     if (Op.getEncoding() != BitCodeAbbrevOp::Array &&
         Op.getEncoding() != BitCodeAbbrevOp::Blob) {
-      Vals.push_back(readAbbreviatedField(*this, Op));
+      if (Expected<uint64_t> MaybeVal = readAbbreviatedField(*this, Op))
+        Vals.push_back(MaybeVal.get());
+      else
+        return MaybeVal.takeError();
       continue;
     }
 
     if (Op.getEncoding() == BitCodeAbbrevOp::Array) {
       // Array case.  Read the number of elements as a vbr6.
-      unsigned NumElts = ReadVBR(6);
+      Expected<uint32_t> MaybeNumElts = ReadVBR(6);
+      if (!MaybeNumElts)
+        return MaybeNumElts.takeError();
+      uint32_t NumElts = MaybeNumElts.get();
 
       // Get the element encoding.
       if (i + 2 != e)
@@ -232,22 +308,36 @@ unsigned BitstreamCursor::readRecord(uns
         report_fatal_error("Array element type can't be an Array or a Blob");
       case BitCodeAbbrevOp::Fixed:
         for (; NumElts; --NumElts)
-          Vals.push_back(Read((unsigned)EltEnc.getEncodingData()));
+          if (Expected<SimpleBitstreamCursor::word_t> MaybeVal =
+                  Read((unsigned)EltEnc.getEncodingData()))
+            Vals.push_back(MaybeVal.get());
+          else
+            return MaybeVal.takeError();
         break;
       case BitCodeAbbrevOp::VBR:
         for (; NumElts; --NumElts)
-          Vals.push_back(ReadVBR64((unsigned)EltEnc.getEncodingData()));
+          if (Expected<uint64_t> MaybeVal =
+                  ReadVBR64((unsigned)EltEnc.getEncodingData()))
+            Vals.push_back(MaybeVal.get());
+          else
+            return MaybeVal.takeError();
         break;
       case BitCodeAbbrevOp::Char6:
         for (; NumElts; --NumElts)
-          Vals.push_back(BitCodeAbbrevOp::DecodeChar6(Read(6)));
+          if (Expected<SimpleBitstreamCursor::word_t> MaybeVal = Read(6))
+            Vals.push_back(BitCodeAbbrevOp::DecodeChar6(MaybeVal.get()));
+          else
+            return MaybeVal.takeError();
       }
       continue;
     }
 
     assert(Op.getEncoding() == BitCodeAbbrevOp::Blob);
     // Blob case.  Read the number of bytes as a vbr6.
-    unsigned NumElts = ReadVBR(6);
+    Expected<uint32_t> MaybeNumElts = ReadVBR(6);
+    if (!MaybeNumElts)
+      return MaybeNumElts.takeError();
+    uint32_t NumElts = MaybeNumElts.get();
     SkipToFourByteBoundary();  // 32-bit alignment
 
     // Figure out where the end of this blob will be including tail padding.
@@ -265,7 +355,8 @@ unsigned BitstreamCursor::readRecord(uns
     // Otherwise, inform the streamer that we need these bytes in memory.  Skip
     // over tail padding first, in case jumping to NewEnd invalidates the Blob
     // pointer.
-    JumpToBit(NewEnd);
+    if (Error Err = JumpToBit(NewEnd))
+      return std::move(Err);
     const char *Ptr = (const char *)getPointerToBit(CurBitPos, NumElts);
 
     // If we can return a reference to the data, do so to avoid copying it.
@@ -281,19 +372,35 @@ unsigned BitstreamCursor::readRecord(uns
   return Code;
 }
 
-void BitstreamCursor::ReadAbbrevRecord() {
+Error BitstreamCursor::ReadAbbrevRecord() {
   auto Abbv = std::make_shared<BitCodeAbbrev>();
-  unsigned NumOpInfo = ReadVBR(5);
+  Expected<uint32_t> MaybeNumOpInfo = ReadVBR(5);
+  if (!MaybeNumOpInfo)
+    return MaybeNumOpInfo.takeError();
+  unsigned NumOpInfo = MaybeNumOpInfo.get();
   for (unsigned i = 0; i != NumOpInfo; ++i) {
-    bool IsLiteral = Read(1);
+    Expected<word_t> MaybeIsLiteral = Read(1);
+    if (!MaybeIsLiteral)
+      return MaybeIsLiteral.takeError();
+    bool IsLiteral = MaybeIsLiteral.get();
     if (IsLiteral) {
-      Abbv->Add(BitCodeAbbrevOp(ReadVBR64(8)));
+      Expected<uint64_t> MaybeOp = ReadVBR64(8);
+      if (!MaybeOp)
+        return MaybeOp.takeError();
+      Abbv->Add(BitCodeAbbrevOp(MaybeOp.get()));
       continue;
     }
 
-    BitCodeAbbrevOp::Encoding E = (BitCodeAbbrevOp::Encoding)Read(3);
+    Expected<word_t> MaybeEncoding = Read(3);
+    if (!MaybeEncoding)
+      return MaybeEncoding.takeError();
+    BitCodeAbbrevOp::Encoding E =
+        (BitCodeAbbrevOp::Encoding)MaybeEncoding.get();
     if (BitCodeAbbrevOp::hasEncodingData(E)) {
-      uint64_t Data = ReadVBR64(5);
+      Expected<uint64_t> MaybeData = ReadVBR64(5);
+      if (!MaybeData)
+        return MaybeData.takeError();
+      uint64_t Data = MaybeData.get();
 
       // As a special case, handle fixed(0) (i.e., a fixed field with zero bits)
       // and vbr(0) as a literal zero.  This is decoded the same way, and avoids
@@ -317,11 +424,14 @@ void BitstreamCursor::ReadAbbrevRecord()
   if (Abbv->getNumOperandInfos() == 0)
     report_fatal_error("Abbrev record with no operands");
   CurAbbrevs.push_back(std::move(Abbv));
+
+  return Error::success();
 }
 
-Optional<BitstreamBlockInfo>
+Expected<Optional<BitstreamBlockInfo>>
 BitstreamCursor::ReadBlockInfoBlock(bool ReadBlockInfoNames) {
-  if (EnterSubBlock(bitc::BLOCKINFO_BLOCK_ID)) return None;
+  if (llvm::Error Err = EnterSubBlock(bitc::BLOCKINFO_BLOCK_ID))
+    return std::move(Err);
 
   BitstreamBlockInfo NewBlockInfo;
 
@@ -330,7 +440,11 @@ BitstreamCursor::ReadBlockInfoBlock(bool
 
   // Read all the records for this module.
   while (true) {
-    BitstreamEntry Entry = advanceSkippingSubblocks(AF_DontAutoprocessAbbrevs);
+    Expected<BitstreamEntry> MaybeEntry =
+        advanceSkippingSubblocks(AF_DontAutoprocessAbbrevs);
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::SubBlock: // Handled for us already.
@@ -346,7 +460,8 @@ BitstreamCursor::ReadBlockInfoBlock(bool
     // Read abbrev records, associate them with CurBID.
     if (Entry.ID == bitc::DEFINE_ABBREV) {
       if (!CurBlockInfo) return None;
-      ReadAbbrevRecord();
+      if (Error Err = ReadAbbrevRecord())
+        return std::move(Err);
 
       // ReadAbbrevRecord installs the abbrev in CurAbbrevs.  Move it to the
       // appropriate BlockInfo.
@@ -357,22 +472,28 @@ BitstreamCursor::ReadBlockInfoBlock(bool
 
     // Read a record.
     Record.clear();
-    switch (readRecord(Entry.ID, Record)) {
-      default: break;  // Default behavior, ignore unknown content.
-      case bitc::BLOCKINFO_CODE_SETBID:
-        if (Record.size() < 1) return None;
-        CurBlockInfo = &NewBlockInfo.getOrCreateBlockInfo((unsigned)Record[0]);
-        break;
-      case bitc::BLOCKINFO_CODE_BLOCKNAME: {
-        if (!CurBlockInfo) return None;
-        if (!ReadBlockInfoNames)
-          break; // Ignore name.
-        std::string Name;
-        for (unsigned i = 0, e = Record.size(); i != e; ++i)
-          Name += (char)Record[i];
-        CurBlockInfo->Name = Name;
-        break;
-      }
+    Expected<unsigned> MaybeBlockInfo = readRecord(Entry.ID, Record);
+    if (!MaybeBlockInfo)
+      return MaybeBlockInfo.takeError();
+    switch (MaybeBlockInfo.get()) {
+    default:
+      break; // Default behavior, ignore unknown content.
+    case bitc::BLOCKINFO_CODE_SETBID:
+      if (Record.size() < 1)
+        return None;
+      CurBlockInfo = &NewBlockInfo.getOrCreateBlockInfo((unsigned)Record[0]);
+      break;
+    case bitc::BLOCKINFO_CODE_BLOCKNAME: {
+      if (!CurBlockInfo)
+        return None;
+      if (!ReadBlockInfoNames)
+        break; // Ignore name.
+      std::string Name;
+      for (unsigned i = 0, e = Record.size(); i != e; ++i)
+        Name += (char)Record[i];
+      CurBlockInfo->Name = Name;
+      break;
+    }
       case bitc::BLOCKINFO_CODE_SETRECORDNAME: {
         if (!CurBlockInfo) return None;
         if (!ReadBlockInfoNames)
@@ -384,6 +505,6 @@ BitstreamCursor::ReadBlockInfoBlock(bool
                                                            Name));
         break;
       }
-    }
+      }
   }
 }

Modified: llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp Wed Jun 26 12:50:12 2019
@@ -674,8 +674,12 @@ MetadataLoader::MetadataLoaderImpl::lazy
   SmallVector<uint64_t, 64> Record;
   // Get the abbrevs, and preload record positions to make them lazy-loadable.
   while (true) {
-    BitstreamEntry Entry = IndexCursor.advanceSkippingSubblocks(
+    Expected<BitstreamEntry> MaybeEntry = IndexCursor.advanceSkippingSubblocks(
         BitstreamCursor::AF_DontPopBlockAtEnd);
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
     case BitstreamEntry::Error:
@@ -687,14 +691,22 @@ MetadataLoader::MetadataLoaderImpl::lazy
       // The interesting case.
       ++NumMDRecordLoaded;
       uint64_t CurrentPos = IndexCursor.GetCurrentBitNo();
-      auto Code = IndexCursor.skipRecord(Entry.ID);
+      Expected<unsigned> MaybeCode = IndexCursor.skipRecord(Entry.ID);
+      if (!MaybeCode)
+        return MaybeCode.takeError();
+      unsigned Code = MaybeCode.get();
       switch (Code) {
       case bitc::METADATA_STRINGS: {
         // Rewind and parse the strings.
-        IndexCursor.JumpToBit(CurrentPos);
+        if (Error Err = IndexCursor.JumpToBit(CurrentPos))
+          return std::move(Err);
         StringRef Blob;
         Record.clear();
-        IndexCursor.readRecord(Entry.ID, Record, &Blob);
+        if (Expected<unsigned> MaybeRecord =
+                IndexCursor.readRecord(Entry.ID, Record, &Blob))
+          ;
+        else
+          return MaybeRecord.takeError();
         unsigned NumStrings = Record[0];
         MDStringRef.reserve(NumStrings);
         auto IndexNextMDString = [&](StringRef Str) {
@@ -707,26 +719,37 @@ MetadataLoader::MetadataLoaderImpl::lazy
       case bitc::METADATA_INDEX_OFFSET: {
         // This is the offset to the index, when we see this we skip all the
         // records and load only an index to these.
-        IndexCursor.JumpToBit(CurrentPos);
+        if (Error Err = IndexCursor.JumpToBit(CurrentPos))
+          return std::move(Err);
         Record.clear();
-        IndexCursor.readRecord(Entry.ID, Record);
+        if (Expected<unsigned> MaybeRecord =
+                IndexCursor.readRecord(Entry.ID, Record))
+          ;
+        else
+          return MaybeRecord.takeError();
         if (Record.size() != 2)
           return error("Invalid record");
         auto Offset = Record[0] + (Record[1] << 32);
         auto BeginPos = IndexCursor.GetCurrentBitNo();
-        IndexCursor.JumpToBit(BeginPos + Offset);
-        Entry = IndexCursor.advanceSkippingSubblocks(
-            BitstreamCursor::AF_DontPopBlockAtEnd);
+        if (Error Err = IndexCursor.JumpToBit(BeginPos + Offset))
+          return std::move(Err);
+        Expected<BitstreamEntry> MaybeEntry =
+            IndexCursor.advanceSkippingSubblocks(
+                BitstreamCursor::AF_DontPopBlockAtEnd);
+        if (!MaybeEntry)
+          return MaybeEntry.takeError();
+        Entry = MaybeEntry.get();
         assert(Entry.Kind == BitstreamEntry::Record &&
                "Corrupted bitcode: Expected `Record` when trying to find the "
                "Metadata index");
         Record.clear();
-        auto Code = IndexCursor.readRecord(Entry.ID, Record);
-        (void)Code;
-        assert(Code == bitc::METADATA_INDEX && "Corrupted bitcode: Expected "
-                                               "`METADATA_INDEX` when trying "
-                                               "to find the Metadata index");
-
+        if (Expected<unsigned> MaybeCode =
+                IndexCursor.readRecord(Entry.ID, Record))
+          assert(MaybeCode.get() == bitc::METADATA_INDEX &&
+                 "Corrupted bitcode: Expected `METADATA_INDEX` when trying to "
+                 "find the Metadata index");
+        else
+          return MaybeCode.takeError();
         // Delta unpack
         auto CurrentValue = BeginPos;
         GlobalMetadataBitPosIndex.reserve(Record.size());
@@ -742,21 +765,33 @@ MetadataLoader::MetadataLoaderImpl::lazy
         return error("Corrupted Metadata block");
       case bitc::METADATA_NAME: {
         // Named metadata need to be materialized now and aren't deferred.
-        IndexCursor.JumpToBit(CurrentPos);
+        if (Error Err = IndexCursor.JumpToBit(CurrentPos))
+          return std::move(Err);
         Record.clear();
-        unsigned Code = IndexCursor.readRecord(Entry.ID, Record);
-        assert(Code == bitc::METADATA_NAME);
+
+        unsigned Code;
+        if (Expected<unsigned> MaybeCode =
+                IndexCursor.readRecord(Entry.ID, Record)) {
+          Code = MaybeCode.get();
+          assert(Code == bitc::METADATA_NAME);
+        } else
+          return MaybeCode.takeError();
 
         // Read name of the named metadata.
         SmallString<8> Name(Record.begin(), Record.end());
-        Code = IndexCursor.ReadCode();
+        if (Expected<unsigned> MaybeCode = IndexCursor.ReadCode())
+          Code = MaybeCode.get();
+        else
+          return MaybeCode.takeError();
 
         // Named Metadata comes in two parts, we expect the name to be followed
         // by the node
         Record.clear();
-        unsigned NextBitCode = IndexCursor.readRecord(Code, Record);
-        assert(NextBitCode == bitc::METADATA_NAMED_NODE);
-        (void)NextBitCode;
+        if (Expected<unsigned> MaybeNextBitCode =
+                IndexCursor.readRecord(Code, Record))
+          assert(MaybeNextBitCode.get() == bitc::METADATA_NAMED_NODE);
+        else
+          return MaybeNextBitCode.takeError();
 
         // Read named metadata elements.
         unsigned Size = Record.size();
@@ -775,9 +810,14 @@ MetadataLoader::MetadataLoaderImpl::lazy
       case bitc::METADATA_GLOBAL_DECL_ATTACHMENT: {
         // FIXME: we need to do this early because we don't materialize global
         // value explicitly.
-        IndexCursor.JumpToBit(CurrentPos);
+        if (Error Err = IndexCursor.JumpToBit(CurrentPos))
+          return std::move(Err);
         Record.clear();
-        IndexCursor.readRecord(Entry.ID, Record);
+        if (Expected<unsigned> MaybeRecord =
+                IndexCursor.readRecord(Entry.ID, Record))
+          ;
+        else
+          return MaybeRecord.takeError();
         if (Record.size() % 2 == 0)
           return error("Invalid record");
         unsigned ValueID = Record[0];
@@ -845,8 +885,8 @@ Error MetadataLoader::MetadataLoaderImpl
   // skip the whole block in case we lazy-load.
   auto EntryPos = Stream.GetCurrentBitNo();
 
-  if (Stream.EnterSubBlock(bitc::METADATA_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::METADATA_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
   PlaceholderQueue Placeholders;
@@ -871,9 +911,14 @@ Error MetadataLoader::MetadataLoaderImpl
       // Return at the beginning of the block, since it is easy to skip it
       // entirely from there.
       Stream.ReadBlockEnd(); // Pop the abbrev block context.
-      Stream.JumpToBit(EntryPos);
-      if (Stream.SkipBlock())
-        return error("Invalid record");
+      if (Error Err = IndexCursor.JumpToBit(EntryPos))
+        return Err;
+      if (Error Err = Stream.SkipBlock()) {
+        // FIXME this drops the error on the floor, which
+        // ThinLTO/X86/debuginfo-cu-import.ll relies on.
+        consumeError(std::move(Err));
+        return Error::success();
+      }
       return Error::success();
     }
     // Couldn't load an index, fallback to loading all the block "old-style".
@@ -883,7 +928,10 @@ Error MetadataLoader::MetadataLoaderImpl
 
   // Read all the records.
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -902,10 +950,13 @@ Error MetadataLoader::MetadataLoaderImpl
     Record.clear();
     StringRef Blob;
     ++NumMDRecordLoaded;
-    unsigned Code = Stream.readRecord(Entry.ID, Record, &Blob);
-    if (Error Err =
-            parseOneMetadata(Record, Code, Placeholders, Blob, NextMetadataNo))
-      return Err;
+    if (Expected<unsigned> MaybeCode =
+            Stream.readRecord(Entry.ID, Record, &Blob)) {
+      if (Error Err = parseOneMetadata(Record, MaybeCode.get(), Placeholders,
+                                       Blob, NextMetadataNo))
+        return Err;
+    } else
+      return MaybeCode.takeError();
   }
 }
 
@@ -930,12 +981,25 @@ void MetadataLoader::MetadataLoaderImpl:
   }
   SmallVector<uint64_t, 64> Record;
   StringRef Blob;
-  IndexCursor.JumpToBit(GlobalMetadataBitPosIndex[ID - MDStringRef.size()]);
-  auto Entry = IndexCursor.advanceSkippingSubblocks();
+  if (Error Err = IndexCursor.JumpToBit(
+          GlobalMetadataBitPosIndex[ID - MDStringRef.size()]))
+    report_fatal_error("lazyLoadOneMetadata failed jumping: " +
+                       toString(std::move(Err)));
+  Expected<BitstreamEntry> MaybeEntry = IndexCursor.advanceSkippingSubblocks();
+  if (!MaybeEntry)
+    // FIXME this drops the error on the floor.
+    report_fatal_error("lazyLoadOneMetadata failed advanceSkippingSubblocks: " +
+                       toString(MaybeEntry.takeError()));
+  BitstreamEntry Entry = MaybeEntry.get();
   ++NumMDRecordLoaded;
-  unsigned Code = IndexCursor.readRecord(Entry.ID, Record, &Blob);
-  if (Error Err = parseOneMetadata(Record, Code, Placeholders, Blob, ID))
-    report_fatal_error("Can't lazyload MD");
+  if (Expected<unsigned> MaybeCode =
+          IndexCursor.readRecord(Entry.ID, Record, &Blob)) {
+    if (Error Err =
+            parseOneMetadata(Record, MaybeCode.get(), Placeholders, Blob, ID))
+      report_fatal_error("Can't lazyload MD, parseOneMetadata: " +
+                         toString(std::move(Err)));
+  } else
+    report_fatal_error("Can't lazyload MD: " + toString(MaybeCode.takeError()));
 }
 
 /// Ensure that all forward-references and placeholders are resolved.
@@ -1032,12 +1096,17 @@ Error MetadataLoader::MetadataLoaderImpl
     // Read name of the named metadata.
     SmallString<8> Name(Record.begin(), Record.end());
     Record.clear();
-    Code = Stream.ReadCode();
+    Expected<unsigned> MaybeCode = Stream.ReadCode();
+    if (!MaybeCode)
+      return MaybeCode.takeError();
+    Code = MaybeCode.get();
 
     ++NumMDRecordLoaded;
-    unsigned NextBitCode = Stream.readRecord(Code, Record);
-    if (NextBitCode != bitc::METADATA_NAMED_NODE)
-      return error("METADATA_NAME not followed by METADATA_NAMED_NODE");
+    if (Expected<unsigned> MaybeNextBitCode = Stream.readRecord(Code, Record)) {
+      if (MaybeNextBitCode.get() != bitc::METADATA_NAMED_NODE)
+        return error("METADATA_NAME not followed by METADATA_NAMED_NODE");
+    } else
+      return MaybeNextBitCode.takeError();
 
     // Read named metadata elements.
     unsigned Size = Record.size();
@@ -1863,7 +1932,10 @@ Error MetadataLoader::MetadataLoaderImpl
     if (R.AtEndOfStream())
       return error("Invalid record: metadata strings bad length");
 
-    unsigned Size = R.ReadVBR(6);
+    Expected<uint32_t> MaybeSize = R.ReadVBR(6);
+    if (!MaybeSize)
+      return MaybeSize.takeError();
+    uint32_t Size = MaybeSize.get();
     if (Strings.size() < Size)
       return error("Invalid record: metadata strings truncated chars");
 
@@ -1892,14 +1964,17 @@ Error MetadataLoader::MetadataLoaderImpl
 /// Parse metadata attachments.
 Error MetadataLoader::MetadataLoaderImpl::parseMetadataAttachment(
     Function &F, const SmallVectorImpl<Instruction *> &InstructionList) {
-  if (Stream.EnterSubBlock(bitc::METADATA_ATTACHMENT_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::METADATA_ATTACHMENT_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
   PlaceholderQueue Placeholders;
 
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -1916,7 +1991,10 @@ Error MetadataLoader::MetadataLoaderImpl
     // Read a metadata attachment record.
     Record.clear();
     ++NumMDRecordLoaded;
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecord)
+      return MaybeRecord.takeError();
+    switch (MaybeRecord.get()) {
     default: // Default behavior: ignore.
       break;
     case bitc::METADATA_ATTACHMENT: {
@@ -1990,14 +2068,17 @@ Error MetadataLoader::MetadataLoaderImpl
 
 /// Parse the metadata kinds out of the METADATA_KIND_BLOCK.
 Error MetadataLoader::MetadataLoaderImpl::parseMetadataKinds() {
-  if (Stream.EnterSubBlock(bitc::METADATA_KIND_BLOCK_ID))
-    return error("Invalid record");
+  if (Error Err = Stream.EnterSubBlock(bitc::METADATA_KIND_BLOCK_ID))
+    return Err;
 
   SmallVector<uint64_t, 64> Record;
 
   // Read all the records.
   while (true) {
-    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
@@ -2013,8 +2094,10 @@ Error MetadataLoader::MetadataLoaderImpl
     // Read a record.
     Record.clear();
     ++NumMDRecordLoaded;
-    unsigned Code = Stream.readRecord(Entry.ID, Record);
-    switch (Code) {
+    Expected<unsigned> MaybeCode = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeCode)
+      return MaybeCode.takeError();
+    switch (MaybeCode.get()) {
     default: // Default behavior: ignore.
       break;
     case bitc::METADATA_KIND: {

Modified: llvm/trunk/test/Bitcode/invalid.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/invalid.test?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/test/Bitcode/invalid.test (original)
+++ llvm/trunk/test/Bitcode/invalid.test Wed Jun 26 12:50:12 2019
@@ -29,13 +29,13 @@ RUN:   FileCheck --check-prefix=MISMATCH
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-invoke-non-function-explicit-type.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=NON-FUNCTION-EXPLICIT-INVOKE %s
 
-INVALID-EMPTY: Invalid bitcode signature
+INVALID-EMPTY: error: file too small to contain bitcode header
 INVALID-ENCODING: Invalid encoding
-BAD-ABBREV: Malformed block
-UNEXPECTED-EOF: Malformed block
-BAD-ABBREV-NUMBER: Malformed block
+BAD-ABBREV: error: can't skip to bit 25870861920 from 96
+UNEXPECTED-EOF: error: can't skip to bit 25870861920 from 96
+BAD-ABBREV-NUMBER: error: can't skip to bit 25870861920 from 96
 BAD-TYPE-TABLE-FORWARD-REF: Invalid TYPE table: Only named structs can be forward referenced
-BAD-BITWIDTH: Malformed block
+BAD-BITWIDTH: error: can't skip to bit 3616 from 96
 BAD-ALIGN: Invalid alignment value
 MISMATCHED-EXPLICIT-GEP: Explicit gep type does not match pointee type of pointer operand
 MISMATCHED-EXPLICIT-LOAD: Explicit load/store type does not match pointee type of pointer operand
@@ -154,7 +154,7 @@ EXTRACT-0-IDXS: EXTRACTVAL: Invalid inst
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-load-ptr-type.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=BAD-LOAD-PTR-TYPE %s
 
-BAD-LOAD-PTR-TYPE: Malformed block
+BAD-LOAD-PTR-TYPE: error: can't skip to bit 3616 from 96
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-inserted-value-type-mismatch.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=INSERT-TYPE-MISMATCH %s
@@ -164,7 +164,7 @@ INSERT-TYPE-MISMATCH: Inserted value typ
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-code-len-width.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-CODELENWIDTH %s
 
-INVALID-CODELENWIDTH: Malformed block
+INVALID-CODELENWIDTH: error: can't skip to bit 3616 from 96
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-function-argument-type.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-ARGUMENT-TYPE %s

Modified: llvm/trunk/test/tools/llvm-lto/error.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-lto/error.ll?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-lto/error.ll (original)
+++ llvm/trunk/test/tools/llvm-lto/error.ll Wed Jun 26 12:50:12 2019
@@ -8,4 +8,4 @@
 ; CHECK-LIBS: llvm-lto: {{.*}}/Inputs/empty.bc: Could not read LTO input file: The file was not recognized as a valid object file
 
 ; RUN: not llvm-lto --thinlto %S/Inputs/empty.bc 2>&1 | FileCheck %s --check-prefix=CHECK-THIN
-; CHECK-THIN: llvm-lto: error loading file '{{.*}}/Inputs/empty.bc': Invalid bitcode signature
+; CHECK-THIN: llvm-lto: error loading file '{{.*}}/Inputs/empty.bc': file too small to contain bitcode header

Modified: llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp (original)
+++ llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp Wed Jun 26 12:50:12 2019
@@ -449,15 +449,17 @@ struct PerBlockIDStats {
 
 static std::map<unsigned, PerBlockIDStats> BlockIDStats;
 
-
-
-/// ReportError - All bitcode analysis errors go through this function, making this a
-/// good place to breakpoint if debugging.
+/// All bitcode analysis errors go through this function, making this a good
+/// place to breakpoint if debugging.
 static bool ReportError(const Twine &Err) {
   WithColor::error() << Err << "\n";
   return true;
 }
 
+static bool ReportError(Error &&Err) {
+  return ReportError(toString(std::move(Err)));
+}
+
 static bool decodeMetadataStringsBlob(StringRef Indent,
                                       ArrayRef<uint64_t> Record,
                                       StringRef Blob) {
@@ -478,7 +480,10 @@ static bool decodeMetadataStringsBlob(St
     if (R.AtEndOfStream())
       return ReportError("bad length");
 
-    unsigned Size = R.ReadVBR(6);
+    Expected<uint32_t> MaybeSize = R.ReadVBR(6);
+    if (!MaybeSize)
+      return ReportError(MaybeSize.takeError());
+    uint32_t Size = MaybeSize.get();
     if (Strings.size() < Size)
       return ReportError("truncated chars");
 
@@ -518,19 +523,24 @@ static bool ParseBlock(BitstreamCursor &
   bool DumpRecords = Dump;
   if (BlockID == bitc::BLOCKINFO_BLOCK_ID) {
     if (Dump) outs() << Indent << "<BLOCKINFO_BLOCK/>\n";
-    Optional<BitstreamBlockInfo> NewBlockInfo =
+    Expected<Optional<BitstreamBlockInfo>> MaybeNewBlockInfo =
         Stream.ReadBlockInfoBlock(/*ReadBlockInfoNames=*/true);
+    if (!MaybeNewBlockInfo)
+      return ReportError(MaybeNewBlockInfo.takeError());
+    Optional<BitstreamBlockInfo> NewBlockInfo =
+        std::move(MaybeNewBlockInfo.get());
     if (!NewBlockInfo)
       return ReportError("Malformed BlockInfoBlock");
     BlockInfo = std::move(*NewBlockInfo);
-    Stream.JumpToBit(BlockBitStart);
+    if (Error Err = Stream.JumpToBit(BlockBitStart))
+      return ReportError(std::move(Err));
     // It's not really interesting to dump the contents of the blockinfo block.
     DumpRecords = false;
   }
 
   unsigned NumWords = 0;
-  if (Stream.EnterSubBlock(BlockID, &NumWords))
-    return ReportError("Malformed block record");
+  if (Error Err = Stream.EnterSubBlock(BlockID, &NumWords))
+    return ReportError(std::move(Err));
 
   // Keep it for later, when we see a MODULE_HASH record
   uint64_t BlockEntryPos = Stream.getCurrentByteNo();
@@ -562,9 +572,12 @@ static bool ParseBlock(BitstreamCursor &
 
     uint64_t RecordStartBit = Stream.GetCurrentBitNo();
 
-    BitstreamEntry Entry =
-      Stream.advance(BitstreamCursor::AF_DontAutoprocessAbbrevs);
-    
+    Expected<BitstreamEntry> MaybeEntry =
+        Stream.advance(BitstreamCursor::AF_DontAutoprocessAbbrevs);
+    if (!MaybeEntry)
+      return ReportError(MaybeEntry.takeError());
+    BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
       return ReportError("malformed bitcode file");
@@ -599,7 +612,8 @@ static bool ParseBlock(BitstreamCursor &
     }
 
     if (Entry.ID == bitc::DEFINE_ABBREV) {
-      Stream.ReadAbbrevRecord();
+      if (Error Err = Stream.ReadAbbrevRecord())
+        return ReportError(std::move(Err));
       ++BlockStats.NumAbbrevs;
       continue;
     }
@@ -610,7 +624,10 @@ static bool ParseBlock(BitstreamCursor &
 
     StringRef Blob;
     uint64_t CurrentRecordPos = Stream.GetCurrentBitNo();
-    unsigned Code = Stream.readRecord(Entry.ID, Record, &Blob);
+    Expected<unsigned> MaybeCode = Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeCode)
+      return ReportError(MaybeCode.takeError());
+    unsigned Code = MaybeCode.get();
 
     // Increment the # occurrences of this code.
     if (BlockStats.CodeFreq.size() <= Code)
@@ -742,8 +759,12 @@ static bool ParseBlock(BitstreamCursor &
     }
 
     // Make sure that we can skip the current record.
-    Stream.JumpToBit(CurrentRecordPos);
-    Stream.skipRecord(Entry.ID);
+    if (Error Err = Stream.JumpToBit(CurrentRecordPos))
+      return ReportError(std::move(Err));
+    if (Expected<unsigned> Skipped = Stream.skipRecord(Entry.ID))
+      ; // Do nothing.
+    else
+      return ReportError(Skipped.takeError());
   }
 }
 
@@ -755,27 +776,45 @@ static void PrintSize(uint64_t Bits) {
                    (double)Bits/8, (unsigned long)(Bits/32));
 }
 
-static CurStreamTypeType ReadSignature(BitstreamCursor &Stream) {
+static Expected<CurStreamTypeType> ReadSignature(BitstreamCursor &Stream) {
+  auto tryRead = [&Stream](char &Dest, size_t size) -> Error {
+    if (Expected<SimpleBitstreamCursor::word_t> MaybeWord = Stream.Read(size))
+      Dest = MaybeWord.get();
+    else
+      return MaybeWord.takeError();
+    return Error::success();
+  };
+
   char Signature[6];
-  Signature[0] = Stream.Read(8);
-  Signature[1] = Stream.Read(8);
+  if (Error Err = tryRead(Signature[0], 8))
+    return std::move(Err);
+  if (Error Err = tryRead(Signature[1], 8))
+    return std::move(Err);
 
   // Autodetect the file contents, if it is one we know.
   if (Signature[0] == 'C' && Signature[1] == 'P') {
-    Signature[2] = Stream.Read(8);
-    Signature[3] = Stream.Read(8);
+    if (Error Err = tryRead(Signature[2], 8))
+      return std::move(Err);
+    if (Error Err = tryRead(Signature[3], 8))
+      return std::move(Err);
     if (Signature[2] == 'C' && Signature[3] == 'H')
       return ClangSerializedASTBitstream;
   } else if (Signature[0] == 'D' && Signature[1] == 'I') {
-    Signature[2] = Stream.Read(8);
-    Signature[3] = Stream.Read(8);
+    if (Error Err = tryRead(Signature[2], 8))
+      return std::move(Err);
+    if (Error Err = tryRead(Signature[3], 8))
+      return std::move(Err);
     if (Signature[2] == 'A' && Signature[3] == 'G')
       return ClangSerializedDiagnosticsBitstream;
   } else {
-    Signature[2] = Stream.Read(4);
-    Signature[3] = Stream.Read(4);
-    Signature[4] = Stream.Read(4);
-    Signature[5] = Stream.Read(4);
+    if (Error Err = tryRead(Signature[2], 4))
+      return std::move(Err);
+    if (Error Err = tryRead(Signature[3], 4))
+      return std::move(Err);
+    if (Error Err = tryRead(Signature[4], 4))
+      return std::move(Err);
+    if (Error Err = tryRead(Signature[5], 4))
+      return std::move(Err);
     if (Signature[0] == 'B' && Signature[1] == 'C' &&
         Signature[2] == 0x0 && Signature[3] == 0xC &&
         Signature[4] == 0xE && Signature[5] == 0xD)
@@ -827,7 +866,10 @@ static bool openBitcodeFile(StringRef Pa
   }
 
   Stream = BitstreamCursor(ArrayRef<uint8_t>(BufPtr, EndBufPtr));
-  CurStreamType = ReadSignature(Stream);
+  Expected<CurStreamTypeType> MaybeSignature = ReadSignature(Stream);
+  if (!MaybeSignature)
+    return ReportError(MaybeSignature.takeError());
+  CurStreamType = std::move(MaybeSignature.get());
 
   return false;
 }
@@ -853,21 +895,30 @@ static int AnalyzeBitcode() {
       return true;
 
     while (!BlockInfoCursor.AtEndOfStream()) {
-      unsigned Code = BlockInfoCursor.ReadCode();
-      if (Code != bitc::ENTER_SUBBLOCK)
+      Expected<unsigned> MaybeCode = BlockInfoCursor.ReadCode();
+      if (!MaybeCode)
+        return ReportError(MaybeCode.takeError());
+      if (MaybeCode.get() != bitc::ENTER_SUBBLOCK)
         return ReportError("Invalid record at top-level in block info file");
 
-      unsigned BlockID = BlockInfoCursor.ReadSubBlockID();
-      if (BlockID == bitc::BLOCKINFO_BLOCK_ID) {
-        Optional<BitstreamBlockInfo> NewBlockInfo =
+      Expected<unsigned> MaybeBlockID = BlockInfoCursor.ReadSubBlockID();
+      if (!MaybeBlockID)
+        return ReportError(MaybeBlockID.takeError());
+      if (MaybeBlockID.get() == bitc::BLOCKINFO_BLOCK_ID) {
+        Expected<Optional<BitstreamBlockInfo>> MaybeNewBlockInfo =
             BlockInfoCursor.ReadBlockInfoBlock(/*ReadBlockInfoNames=*/true);
+        if (!MaybeNewBlockInfo)
+          return ReportError(MaybeNewBlockInfo.takeError());
+        Optional<BitstreamBlockInfo> NewBlockInfo =
+            std::move(MaybeNewBlockInfo.get());
         if (!NewBlockInfo)
           return ReportError("Malformed BlockInfoBlock in block info file");
         BlockInfo = std::move(*NewBlockInfo);
         break;
       }
 
-      BlockInfoCursor.SkipBlock();
+      if (Error Err = BlockInfoCursor.SkipBlock())
+        return ReportError(std::move(Err));
     }
   }
 
@@ -875,13 +926,17 @@ static int AnalyzeBitcode() {
 
   // Parse the top-level structure.  We only allow blocks at the top-level.
   while (!Stream.AtEndOfStream()) {
-    unsigned Code = Stream.ReadCode();
-    if (Code != bitc::ENTER_SUBBLOCK)
+    Expected<unsigned> MaybeCode = Stream.ReadCode();
+    if (!MaybeCode)
+      return ReportError(MaybeCode.takeError());
+    if (MaybeCode.get() != bitc::ENTER_SUBBLOCK)
       return ReportError("Invalid record at top-level");
 
-    unsigned BlockID = Stream.ReadSubBlockID();
+    Expected<unsigned> MaybeBlockID = Stream.ReadSubBlockID();
+    if (!MaybeBlockID)
+      return ReportError(MaybeBlockID.takeError());
 
-    if (ParseBlock(Stream, BlockInfo, BlockID, 0, CurStreamType))
+    if (ParseBlock(Stream, BlockInfo, MaybeBlockID.get(), 0, CurStreamType))
       return true;
     ++NumTopBlocks;
   }

Modified: llvm/trunk/unittests/Bitcode/BitstreamReaderTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Bitcode/BitstreamReaderTest.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- llvm/trunk/unittests/Bitcode/BitstreamReaderTest.cpp (original)
+++ llvm/trunk/unittests/Bitcode/BitstreamReaderTest.cpp Wed Jun 26 12:50:12 2019
@@ -22,15 +22,17 @@ TEST(BitstreamReaderTest, AtEndOfStream)
   BitstreamCursor Cursor(Bytes);
 
   EXPECT_FALSE(Cursor.AtEndOfStream());
-  (void)Cursor.Read(8);
+  Expected<SimpleBitstreamCursor::word_t> MaybeRead = Cursor.Read(8);
+  EXPECT_TRUE((bool)MaybeRead);
   EXPECT_FALSE(Cursor.AtEndOfStream());
-  (void)Cursor.Read(24);
+  MaybeRead = Cursor.Read(24);
+  EXPECT_TRUE((bool)MaybeRead);
   EXPECT_TRUE(Cursor.AtEndOfStream());
 
-  Cursor.JumpToBit(0);
+  EXPECT_FALSE(Cursor.JumpToBit(0));
   EXPECT_FALSE(Cursor.AtEndOfStream());
 
-  Cursor.JumpToBit(32);
+  EXPECT_FALSE(Cursor.JumpToBit(32));
   EXPECT_TRUE(Cursor.AtEndOfStream());
 }
 
@@ -40,7 +42,7 @@ TEST(BitstreamReaderTest, AtEndOfStreamJ
   };
   BitstreamCursor Cursor(Bytes);
 
-  Cursor.JumpToBit(32);
+  EXPECT_FALSE(Cursor.JumpToBit(32));
   EXPECT_TRUE(Cursor.AtEndOfStream());
 }
 
@@ -56,7 +58,8 @@ TEST(BitstreamReaderTest, getCurrentByte
 
   for (unsigned I = 0, E = 32; I != E; ++I) {
     EXPECT_EQ(I / 8, Cursor.getCurrentByteNo());
-    (void)Cursor.Read(1);
+    Expected<SimpleBitstreamCursor::word_t> MaybeRead = Cursor.Read(1);
+    EXPECT_TRUE((bool)MaybeRead);
   }
   EXPECT_EQ(4u, Cursor.getCurrentByteNo());
 }
@@ -116,24 +119,33 @@ TEST(BitstreamReaderTest, readRecordWith
 
     // Header.  Included in test so that we can run llvm-bcanalyzer to debug
     // when there are problems.
-    ASSERT_EQ(Magic, Stream.Read(32));
+    Expected<SimpleBitstreamCursor::word_t> MaybeRead = Stream.Read(32);
+    ASSERT_TRUE((bool)MaybeRead);
+    ASSERT_EQ(Magic, MaybeRead.get());
 
     // Block.
-    BitstreamEntry Entry =
+    Expected<BitstreamEntry> MaybeEntry =
         Stream.advance(BitstreamCursor::AF_DontAutoprocessAbbrevs);
+    ASSERT_TRUE((bool)MaybeEntry);
+    BitstreamEntry Entry = MaybeEntry.get();
     ASSERT_EQ(BitstreamEntry::SubBlock, Entry.Kind);
     ASSERT_EQ(BlockID, Entry.ID);
     ASSERT_FALSE(Stream.EnterSubBlock(BlockID));
 
     // Abbreviation.
-    Entry = Stream.advance();
+    MaybeEntry = Stream.advance();
+    ASSERT_TRUE((bool)MaybeEntry);
+    Entry = MaybeEntry.get();
     ASSERT_EQ(BitstreamEntry::Record, Entry.Kind);
     ASSERT_EQ(AbbrevID, Entry.ID);
 
     // Record.
     StringRef BlobOut;
     SmallVector<uint64_t, 1> Record;
-    ASSERT_EQ(RecordID, Stream.readRecord(Entry.ID, Record, &BlobOut));
+    Expected<unsigned> MaybeRecord =
+        Stream.readRecord(Entry.ID, Record, &BlobOut);
+    ASSERT_TRUE((bool)MaybeRecord);
+    ASSERT_EQ(RecordID, MaybeRecord.get());
     EXPECT_TRUE(Record.empty());
     EXPECT_EQ(BlobIn, BlobOut);
   }
@@ -143,7 +155,9 @@ TEST(BitstreamReaderTest, shortRead) {
   uint8_t Bytes[] = {8, 7, 6, 5, 4, 3, 2, 1};
   for (unsigned I = 1; I != 8; ++I) {
     SimpleBitstreamCursor Cursor(ArrayRef<uint8_t>(Bytes, I));
-    EXPECT_EQ(8ull, Cursor.Read(8));
+    Expected<SimpleBitstreamCursor::word_t> MaybeRead = Cursor.Read(8);
+    ASSERT_TRUE((bool)MaybeRead);
+    EXPECT_EQ(8ull, MaybeRead.get());
   }
 }
 




More information about the llvm-commits mailing list