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

llvm-svn: 364464

1 files changed, 60 insertions, 26 deletions

diff --git a/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp
index 4489daf3d92..77e67e896c2 100644
--- a/clang-tools-extra/clang-doc/BitcodeReader.cpp
+++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp
@@ -491,24 +491,27 @@ template <typename T>
 llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID, T I) {
   Record R;
   llvm::StringRef Blob;
-  unsigned RecID = Stream.readRecord(ID, R, &Blob);
-  return parseRecord(R, RecID, Blob, I);
+  llvm::Expected<unsigned> MaybeRecID = Stream.readRecord(ID, R, &Blob);
+  if (!MaybeRecID)
+    return MaybeRecID.takeError();
+  return parseRecord(R, MaybeRecID.get(), Blob, I);
 }
 
 template <>
 llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID, Reference *I) {
   Record R;
   llvm::StringRef Blob;
-  unsigned RecID = Stream.readRecord(ID, R, &Blob);
-  return parseRecord(R, RecID, Blob, I, CurrentReferenceField);
+  llvm::Expected<unsigned> MaybeRecID = Stream.readRecord(ID, R, &Blob);
+  if (!MaybeRecID)
+    return MaybeRecID.takeError();
+  return parseRecord(R, MaybeRecID.get(), Blob, I, CurrentReferenceField);
 }
 
 // Read a block of records into a single info.
 template <typename T>
 llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) {
-  if (Stream.EnterSubBlock(ID))
-    return llvm::make_error<llvm::StringError>("Unable to enter subblock.\n",
-                                               llvm::inconvertibleErrorCode());
+  if (llvm::Error Err = Stream.EnterSubBlock(ID))
+    return Err;
 
   while (true) {
     unsigned BlockOrCode = 0;
@@ -521,9 +524,9 @@ llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) {
     case Cursor::BlockEnd:
       return llvm::Error::success();
     case Cursor::BlockBegin:
-      if (auto Err = readSubBlock(BlockOrCode, I)) {
-        if (!Stream.SkipBlock())
-          continue;
+      if (llvm::Error Err = readSubBlock(BlockOrCode, I)) {
+        if (llvm::Error Skipped = Stream.SkipBlock())
+          return joinErrors(std::move(Err), std::move(Skipped));
         return Err;
       }
       continue;
@@ -605,18 +608,34 @@ ClangDocBitcodeReader::skipUntilRecordOrBlock(unsigned &BlockOrRecordID) {
   BlockOrRecordID = 0;
 
   while (!Stream.AtEndOfStream()) {
-    unsigned Code = Stream.ReadCode();
+    Expected<unsigned> MaybeCode = Stream.ReadCode();
+    if (!MaybeCode) {
+      // FIXME this drops the error on the floor.
+      consumeError(MaybeCode.takeError());
+      return Cursor::BadBlock;
+    }
 
-    switch ((llvm::bitc::FixedAbbrevIDs)Code) {
+    // FIXME check that the enum is in range.
+    auto Code = static_cast<llvm::bitc::FixedAbbrevIDs>(MaybeCode.get());
+
+    switch (Code) {
     case llvm::bitc::ENTER_SUBBLOCK:
-      BlockOrRecordID = Stream.ReadSubBlockID();
+      if (Expected<unsigned> MaybeID = Stream.ReadSubBlockID())
+        BlockOrRecordID = MaybeID.get();
+      else {
+        // FIXME this drops the error on the floor.
+        consumeError(MaybeID.takeError());
+      }
       return Cursor::BlockBegin;
     case llvm::bitc::END_BLOCK:
       if (Stream.ReadBlockEnd())
         return Cursor::BadBlock;
       return Cursor::BlockEnd;
     case llvm::bitc::DEFINE_ABBREV:
-      Stream.ReadAbbrevRecord();
+      if (llvm::Error Err = Stream.ReadAbbrevRecord()) {
+        // FIXME this drops the error on the floor.
+        consumeError(std::move(Err));
+      }
       continue;
     case llvm::bitc::UNABBREV_RECORD:
       return Cursor::BadBlock;
@@ -634,17 +653,24 @@ llvm::Error ClangDocBitcodeReader::validateStream() {
                                                llvm::inconvertibleErrorCode());
 
   // Sniff for the signature.
-  if (Stream.Read(8) != BitCodeConstants::Signature[0] ||
-      Stream.Read(8) != BitCodeConstants::Signature[1] ||
-      Stream.Read(8) != BitCodeConstants::Signature[2] ||
-      Stream.Read(8) != BitCodeConstants::Signature[3])
-    return llvm::make_error<llvm::StringError>("Invalid bitcode signature.\n",
-                                               llvm::inconvertibleErrorCode());
+  for (int Idx = 0; Idx != 4; ++Idx) {
+    Expected<llvm::SimpleBitstreamCursor::word_t> MaybeRead = Stream.Read(8);
+    if (!MaybeRead)
+      return MaybeRead.takeError();
+    else if (MaybeRead.get() != BitCodeConstants::Signature[Idx])
+      return llvm::make_error<llvm::StringError>(
+          "Invalid bitcode signature.\n", llvm::inconvertibleErrorCode());
+  }
   return llvm::Error::success();
 }
 
 llvm::Error ClangDocBitcodeReader::readBlockInfoBlock() {
-  BlockInfo = Stream.ReadBlockInfoBlock();
+  Expected<Optional<llvm::BitstreamBlockInfo>> MaybeBlockInfo =
+      Stream.ReadBlockInfoBlock();
+  if (!MaybeBlockInfo)
+    return MaybeBlockInfo.takeError();
+  else
+    BlockInfo = MaybeBlockInfo.get();
   if (!BlockInfo)
     return llvm::make_error<llvm::StringError>(
         "Unable to parse BlockInfoBlock.\n", llvm::inconvertibleErrorCode());
@@ -687,11 +713,16 @@ ClangDocBitcodeReader::readBitcode() {
 
   // Read the top level blocks.
   while (!Stream.AtEndOfStream()) {
-    unsigned Code = Stream.ReadCode();
-    if (Code != llvm::bitc::ENTER_SUBBLOCK)
+    Expected<unsigned> MaybeCode = Stream.ReadCode();
+    if (!MaybeCode)
+      return MaybeCode.takeError();
+    if (MaybeCode.get() != llvm::bitc::ENTER_SUBBLOCK)
       return llvm::make_error<llvm::StringError>(
           "No blocks in input.\n", llvm::inconvertibleErrorCode());
-    unsigned ID = Stream.ReadSubBlockID();
+    Expected<unsigned> MaybeID = Stream.ReadSubBlockID();
+    if (!MaybeID)
+      return MaybeID.takeError();
+    unsigned ID = MaybeID.get();
     switch (ID) {
     // NamedType and Comment blocks should not appear at the top level
     case BI_TYPE_BLOCK_ID:
@@ -720,8 +751,11 @@ ClangDocBitcodeReader::readBitcode() {
         return std::move(Err);
       continue;
     default:
-      if (!Stream.SkipBlock())
-        continue;
+      if (llvm::Error Err = Stream.SkipBlock()) {
+        // FIXME this drops the error on the floor.
+        consumeError(std::move(Err));
+      }
+      continue;
     }
   }
   return std::move(Infos);