BitStream reader: propagate errors

[jfb]

Summary

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

[jfb]

This is still a work in progress. I still have to debug clang tests that fail, and compile / fix more than clang. Also, clang-format.

[bruno]

Hi JF. Thanks for working on this, nice improvement to error handling!

The overall approach is pretty solid and should prevent a lot of red herring while investigating hard to reproduce crashes in clang, specially when implicit clang modules is involved. Dropping the errors on the floor for previous code that didn't handle errors at all is a fair tradeoff for introducing the functionality. I have omitted any format comments but I noticed several of 80 cols violations. More specific reviews inline.

clang/lib/Frontend/PrecompiledPreamble.cpp

356

Changes like this are so much better!

clang/lib/Frontend/SerializedDiagnosticReader.cpp

59

Can this be simplified as below?

Expected<unsigned> Res = Stream.ReadCode();
if (!Res || Res.get() != llvm::bitc::ENTER_SUBBLOCK)
  return SDError::InvalidDiagnostics; // FIXME propagate the error details.

clang/lib/Serialization/ASTReader.cpp

1158–1170

Not necessarily needed as part of this patch, but I wonder how many of repetitive access patterns (readCode + readRecord, and maybe other patterns) we have that would take advantage of refactoring all these checks out into their own methods.

4307

Very similar to SerializedDiagnosticReader.cpp:44, does it deserve a common helper?

4584

This is a good example of a real issue this patch solves. Sometimes we get signature mismatch problems in implicit modules builds because we read garbage. Having this check and failure here prevents the misleading error message.

clang/lib/Serialization/ASTReaderDecl.cpp

3699

typo on readung

4040

Why? What's a better alternative?

4123

Does this builds fine without assertions?

clang/lib/Serialization/GlobalModuleIndex.cpp

133

Maybe use a similar helper for error checking added in ASTReaderDecl.cpp?

264–269

Very similar to SerializedDiagnosticReader.cpp:44, does it deserve a common helper?

533–538

Very similar to SerializedDiagnosticReader.cpp:44, does it deserve a common helper?

llvm/include/llvm/Bitcode/BitstreamReader.h

230

Alternatively, you can go early return mode for this and other error checking in BitstreamReader.h

Expected<unsigned> Res = Read(NumBits);
if (!Res)
  return Res;
Piece = Res.get();

[jfb]

• Address Bruno's comments.

[jfb]

In D63518#1550827, @bruno wrote:

Hi JF. Thanks for working on this, nice improvement to error handling!

The overall approach is pretty solid and should prevent a lot of red herring while investigating hard to reproduce crashes in clang, specially when implicit clang modules is involved. Dropping the errors on the floor for previous code that didn't handle errors at all is a fair tradeoff for introducing the functionality.

Thanks!

I have omitted any format comments but I noticed several of 80 cols violations. More specific reviews inline.

Yeah I've been avoiding clang-format because it's sometimes easier to manage merge conflicts without reformatting. I'll absolutely run format when the patch is further along.

clang/lib/Frontend/SerializedDiagnosticReader.cpp

59

Yeah, but I think we'll want to propagate errors in a follow-up so we'll end up re-separating them. I'd rather have the right structure here.

clang/lib/Serialization/ASTReader.cpp

1158–1170

Yeah I noticed that too, probably worth adding a helper for. I'll note it in a bug: https://bugs.llvm.org/show_bug.cgi?id=42335

4307

It would be nice, but they don't have the same error handling :(
I'll add a FIXME.

4584

🎉

clang/lib/Serialization/ASTReaderDecl.cpp

3699

lol dung

4040

Anything that can be used as a clang API needs to return errors instead of using report_fatal_error, so that the API can't just abort your process. We should propagate this error, but it's getting tedious here and I think better in a follow-up.

4123

It should, why? The variable is used on both sides of the if.

clang/lib/Serialization/GlobalModuleIndex.cpp

264–269

Left a FIXME in ASTReader.cpp for this.

llvm/include/llvm/Bitcode/BitstreamReader.h

230

Yeah that's the first code I fixed, and later moved to what you suggest. Updated here.

[jfb]

• Bitstream reader's SkipBLock and EnterSubBlock now return Error instead of Expected<bool>.

[jfb]

• Handle error in ASTReader::loadGlobalIndex, it's hit in testing.
• Handle error in CompilerInstance::ExecuteAction, it's hit in testing.

[jfb]

• Fix a few more places that need to consume errors for now.

[jfb]

• Remove on error propagation, which a test relies on. We should fix it in a separate patch.
• pch-from-libclang.c isn't cleaning its temp files before running, which makes reproducing issues that much more painful.

[jfb]

check-clang now passes all tests, so the patch is pretty much ready to review. I'll get started on the other parts of LLVM that use this API.

[jfb]

• Update BitstreamReaderTest.cpp to use the updated API.

[jfb]

• Update BitstreamReaderTest.cpp to use the updated API (now passing all its tests).

[jfb]

• Update expected error output for invalid bitcode in Bitcode/invalid.test

[jfb]

• Ignore another error for now, this one is probably a sign that there's an underlying bug somewhere.

[jfb]

• Update error message string in tools/llvm-lto/error.ll

[jfb]

All of check-llvm now passes. I'll look at other users of this stuff in the monorepo, compile and run their tests.

[jfb]

• Fix clang-tools-extra, check-clang-tools now passes.

[jfb]

• Fix a few more uses of APIs in clang-tools-extra. All of the monorepo now builds.

[jfb]

I did a build of all LLVM monorepo projects, and only LLVM / clang / clang-tools-extra are impacted. I therefore ran tests as follows, and they all pass:

rm -rf debug ; mkdir debug && (cd debug && cmake -G Ninja ../llvm -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" && ninja check-all )

I haven't run clang-format yet so ignore formatting. Otherwise, this patch is ready for review and getting committed.

[jfb]

• Rebase

[jfb]

• Remove attribute deprecated, it was there to help catch API uses that needed updating.

[bruno]

LGTM

[jfb]

• clang-format all the code. If it's ugly, don't complain to me

[thegameg]

llvm/include/llvm/Bitcode/BitstreamReader.h

441–442

llvm:: seems unnecessary here.

491

Any reason why this doesn't return Error?

llvm/lib/Bitcode/Reader/BitstreamReader.cpp

140

return createStringError here too?

[Bigcheese]

clang-tools-extra/clang-doc/BitcodeReader.cpp

527–529 ↗

(On Diff #206520)

This is a pretty big behavior change. Is clang-doc expecting to get files with unknown blocks?

611–618 ↗

(On Diff #206520)

Does consumeError return? MaybeCode.get() will crash if !MaybeCode.

[lhames]

I haven't had a chance to audit the whole patch yet, but in general the error suppression idioms are unsafe (though maybe no more so than the existing code?).

I would be inclined to audit all those FIXMEs and replace them with cantFails or consumeErrors. consumeError will generally match existing behavior in places where you were ignoring errors. cantFail will get you aggressive crashes, which can be handy for debugging but not so fun in release code.

Also, if this patch passes the regression tests we need more failure tests. :)

clang-tools-extra/clang-doc/BitcodeReader.cpp

527–529 ↗

(On Diff #206520)

The inner test here is unsafe, as it will discard the outer error. You need:

if (llvm::Error Err = readSubBlock(BlockOrCode, I)) {
  if (llvm::Error Skipped = Stream.SkipBlock()) {
    consumeError(std::move(Err));
    return Skipped;
  }
  return Err;
}

or:

if (llvm::Error Err = readSubBlock(BlockOrCode, I)) {
  if (llvm::Error Skipped = Stream.SkipBlock()) {
    return joinErrors(std::move(Err), std::move(Skipped));
  return Err;
}

611–618 ↗

(On Diff #206520)

Yes: consumeError returns. This should probably be:

if (!MaybeCode)
  return consumeError(MaybeCode.takeError()), Cursor::Badness;

clang/lib/Frontend/CompilerInstance.cpp

2057–2059

This will crash if writeIndex ever generates an error. For that reason I would suggest writing this as:

// FIXME: Can actually fail! <fixme goes here>
cantFail(GlobalModuleIndex::writeIndex(...));

It has the same effect, but without the control flow.

2086

Another cantFail candidate.

clang/lib/Frontend/FrontendAction.cpp

945–948

I'd suggest cantFail here, if not for that fixme comment. Yikes.

clang/lib/Frontend/Rewrite/FrontendActions.cpp

132–134

You need a consumeError here or you'll get a runtime crash if an error is generated.

clang/lib/Frontend/SerializedDiagnosticReader.cpp

89–91

This needs a consumeError or it will crash if an error is generated. How about:

if (llvm::Error Err = Stream.SkipBlock())

return consumeError(std::move(Err)), SDError::MalformedTopLevelBlock;

148–150

Needs a consumeError.

163–165

Needs a consumeError.

190–192

Needs a consumeError.

210–212

Needs a consumeError.

clang/lib/Frontend/TestModuleFileExtension.cpp

53–54

Needs a consumeError or cantFail.

Fun fact: cantFail will auto-unwrap Expecteds:

Expected<T> foo();

T v = cantFail(foo());

clang/lib/Serialization/ASTReader.cpp

1614

Unsafe on error. Needs cantFail or consumeError.

1622

Unsafe on error. Needs cantFail or consumeError.

1630

Unsafe on error. Needs cantFail or consumeError.

1636

Unsafe on error. Needs cantFail or consumeError.

1663

Unsafe on error. Needs cantFail or consumeError.

2201

Unsafe on error. Needs cantFail or consumeError.

2211

Unsafe on error. Needs cantFail or consumeError.

2242

Unsafe on error. Needs cantFail or consumeError.

2392

Unsafe on error. Needs cantFail or consumeError.

2403

Unsafe on error. Needs cantFail or consumeError.

2425

Unsafe on error. Needs cantFail or consumeError.

[jfb]

clang-tools-extra/clang-doc/BitcodeReader.cpp

527–529 ↗

(On Diff #206520)

I don't think it expects this, and it looks (from the above code) like the intent is to handle errors when parsing (so this would be a bug that we'd want fixed).

611–618 ↗

(On Diff #206520)

Good point, fixed.

llvm/include/llvm/Bitcode/BitstreamReader.h

491

I'm not sure it's really an error: it goes to the end of the block either because it's empty, or because it pops the scope (which doesn't error either). It might be erroneously used, but I'm not sure we should make it an error right now. WDYT?

[jfb]

• Address Francis' and Michael's comments.

[jfb]

In D63518#1558197, @lhames wrote:

I haven't had a chance to audit the whole patch yet, but in general the error suppression idioms are unsafe (though maybe no more so than the existing code?).

I would be inclined to audit all those FIXMEs and replace them with cantFails or consumeErrors. consumeError will generally match existing behavior in places where you were ignoring errors. cantFail will get you aggressive crashes, which can be handy for debugging but not so fun in release code.

I haven't addressed the comments yet, but wanted to respond: yes, it's unsafe and I was wondering what folks would rather see, so thanks for bringing it up! I indeed only consumed errors that we encountered, and in that sense the code isn't bug-compatible with what we had before. Seems like you'd want consumeError in most places, which I can certainly do.

Also, if this patch passes the regression tests we need more failure tests. :)

Indeed! That's a pretty terrifying thing... but I'm not signing up to address *that* particular issue :)

[thegameg]

llvm/include/llvm/Bitcode/BitstreamReader.h

491

Yeah, I'm not sure either... BitstreamCursor::advance seems to return BitstreamEntry::getError(); if this function fails after it encountered an END_BLOCK, and the other users seem to return things like SDError::InvalidDiagnostics or Cursor::BadBlock.

I guess for now, it's fine as it is.

[jfb]

• Address Lang's comments.
• format

clang-tools-extra/clang-doc/BitcodeReader.cpp

611–618 ↗

(On Diff #206520)

You tempt me so with this comma operator... but no, I must not!

clang/lib/Frontend/CompilerInstance.cpp

2057–2059

I think this one shouldn't crash, so I've made it consume the error instead (and it still needs to get fixed).

[jfb]

• Improve one fatal error message.

[jfb]

I've addressed Lang's comments and the re-audited all the new FIXME instances. This patch now actually drops errors on the floor instead of implicitly erroring out unless the error path is tested (which is what I had before). This hides bugs, but I left FIXMEs everywhere and it means the patch will be less disruptive because it's bug-compatible in the areas that aren't tested.

[lhames]

Indeed! That's a pretty terrifying thing... but I'm not signing up to address *that* particular issue :)

Yep. I don't think you need to address that in this patch.

LGTM!

clang-tools-extra/clang-doc/BitcodeReader.cpp

611–618 ↗

(On Diff #206520)

;)

[jfb]

I think this is ready to go! I rebased and ran check-all for LLVM / clang / clang-tools-extras and everything passes.

[abrachet]

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

205 ↗

(On Diff #206727)

This and an identical switch on line 5367 cause an unused variable warning from this commit. I don't know if the build bots report on this, or the proper way to tell you about this but hopefully you will see it :)

[jfb]

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

205 ↗

(On Diff #206727)

Thanks, looks like @hans fixed it in r364505. Odd that it wasn't warning locally for me.

[bjope]

clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp

619 ↗

(On Diff #206727)

This has caused problem for our sanitizer tests the last couple of days:

FAIL: Extra Tools Unit Tests :: clang-doc/./ClangDocTests/BitcodeTest.emitMethodInfoBitcode (20077 of 48746)
******************** TEST 'Extra Tools Unit Tests :: clang-doc/./ClangDocTests/BitcodeTest.emitMethodInfoBitcode' FAILED ********************
Note: Google Test filter = BitcodeTest.emitMethodInfoBitcode
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from BitcodeTest
[ RUN      ] BitcodeTest.emitMethodInfoBitcode
/local/repo/bbiswjenk/fem023-eiffel003/workspace/llvm/llvm-master-sanitize/clang-tools-extra/clang-doc/BitcodeReader.cpp:621:13: runtime error: load of value 9, which is not a valid value for type 'llvm::bitc::FixedAbbrevIDs'

This was seen when building trunk with clang 6.0.0 and LVM_USE_SANITIZER=Undefined

cmake -G Ninja '-DLLVM_ENABLE_PROJECTS='\''clang;clang-tools-extra'\''' -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON '-DCMAKE_CXX_FLAGS='\''-stdlib=libc++'\''' -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_USE_LINKER=gold -DLLVM_USE_SANITIZER=Undefined ../.
-- The C compiler identification is Clang 6.0.0
-- The CXX compiler identification is Clang 6.0.0

Afaict we can't cast the read value to FixedAbbrevIDs as we do not know yet if it matches one of the values defined in the enum, or if we will take the default case.

A similar switch exist at cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp:127, but it is using a slightly different pattern:

unsigned Code;
Code = Res.get();
switch ((llvm::bitc::FixedAbbrevIDs)Code)

I haven't seen any failures for SerializedDiagnosticReader. So either we lack test coverage for that function, or the sanitizer only introduce the check when using the static_cast (and declaring Code as an enum) as done here.

[jfb]

clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp

619 ↗

(On Diff #206727)

That sounds like a pre-existing bug. We should check that the value is in range before casting. Can you send patches to fix both code locations, and add test coverage? This code is indeed poorly tested.

Why do the sanitizers catch static_cast but not C-style casts?

[jfb]

clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp

619 ↗

(On Diff #206727)

To be clear: relying on the default case is still UB because there's a cast to the enum type before it occurs.

[bjope]

clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp

619 ↗

(On Diff #206727)

I made a patch here (assuming the goal would be to keep the cast to the enum, and to let the switch cover all enum values): https://reviews.llvm.org/D64262

clang/include/clang/Basic/Diagnostic.h

 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/Error.h"
 #include <cassert>
 #include <cstdint>
 #include <limits>
 #include <list>
 #include <map>
 #include <memory>
 #include <string>
 #include <type_traits>
inline DiagnosticBuilder DiagnosticsEngine::Report(SourceLocation Loc,
   assert(CurDiagID == std::numeric_limits<unsigned>::max() &&
          "Multiple diagnostics in flight at once!");
   CurDiagLoc = Loc;
   CurDiagID = DiagID;
   FlagValue.clear();
   return DiagnosticBuilder(this);
 }
 
+inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB, llvm::Error &&E) {
+  DB.AddString(toString(std::move(E)));
+  return DB;
+}
+
 inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
   return Report(SourceLocation(), DiagID);
 }
 
 //===----------------------------------------------------------------------===//
 // Diagnostic
 //===----------------------------------------------------------------------===//
 

clang/include/clang/Frontend/FrontendAction.h

 #define LLVM_CLANG_FRONTEND_FRONTENDACTION_H
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace clang {
 class ASTMergeAction;
 class CompilerInstance;
 
public:
   /// automatically be shared with the AST file in between
   /// BeginSourceFile() and EndSourceFile().
   ///
   /// \return True on success; on failure the compilation of this file should
   /// be aborted and neither Execute() nor EndSourceFile() should be called.
   bool BeginSourceFile(CompilerInstance &CI, const FrontendInputFile &Input);
 
   /// Set the source manager's main input file, and run the action.
-  bool Execute();
+  __attribute__((deprecated("The semantics of this API have changed from bool to Error, changing the polarity of the return value"))) // FIXME remove this, it's here to help me catch all uses of this API.
+  llvm::Error Execute();
 
   /// Perform any per-file post processing, deallocate per-file
   /// objects, and run statistics and output file cleanup code.
   void EndSourceFile();
 
   /// @}
 };
 

clang/include/clang/Serialization/ASTReader.h

private:
 
   /// Produce an error diagnostic and return true.
   ///
   /// This routine should only be used for fatal errors that have to
   /// do with non-routine failures (e.g., corrupted AST file).
   void Error(StringRef Msg) const;
   void Error(unsigned DiagID, StringRef Arg1 = StringRef(),
              StringRef Arg2 = StringRef()) const;
+  void Error(llvm::Error &&Err) const;
 
 public:
   /// Load the AST file and validate its contents against the given
   /// Preprocessor.
   ///
   /// \param PP the preprocessor associated with the context in which this
   /// precompiled header will be loaded.
   ///
class ASTRecordReader {
   using RecordDataImpl = ASTReader::RecordDataImpl;
 
 public:
   /// Construct an ASTRecordReader that uses the default encoding scheme.
   ASTRecordReader(ASTReader &Reader, ModuleFile &F) : Reader(&Reader), F(&F) {}
 
   /// Reads a record with id AbbrevID from Cursor, resetting the
   /// internal state.
-  unsigned readRecord(llvm::BitstreamCursor &Cursor, unsigned AbbrevID);
+  Expected<unsigned> readRecord(llvm::BitstreamCursor &Cursor, unsigned AbbrevID);
 
   /// Is this a module file for a module (rather than a PCH or similar).
   bool isModule() const { return F->isModule(); }
 
   /// Retrieve the AST context that this AST reader supplements.
   ASTContext &getContext() { return Reader->getContext(); }
 
   /// The current position in this record.

 
 /// Helper class that saves the current stream position and
 /// then restores it when destroyed.
 struct SavedStreamPosition {
   explicit SavedStreamPosition(llvm::BitstreamCursor &Cursor)
       : Cursor(Cursor), Offset(Cursor.GetCurrentBitNo()) {}
 
   ~SavedStreamPosition() {
-    Cursor.JumpToBit(Offset);
+    if (llvm::Error Err = Cursor.JumpToBit(Offset))
+      llvm::report_fatal_error("Cursor should always be able to go back, failed: " + toString(std::move(Err)));
   }
 
 private:
   llvm::BitstreamCursor &Cursor;
   uint64_t Offset;
 };
 
 inline void PCHValidator::Error(const char *Msg) {

clang/include/clang/Serialization/GlobalModuleIndex.h

 #ifndef LLVM_CLANG_SERIALIZATION_GLOBALMODULEINDEX_H
 #define LLVM_CLANG_SERIALIZATION_GLOBALMODULEINDEX_H
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include <memory>
 #include <utility>
 
 namespace llvm {
 class BitstreamCursor;
 class MemoryBuffer;
 }
 
explicit GlobalModuleIndex(std::unique_ptr<llvm::MemoryBuffer> Buffer,
                              llvm::BitstreamCursor Cursor);
 
   GlobalModuleIndex(const GlobalModuleIndex &) = delete;
   GlobalModuleIndex &operator=(const GlobalModuleIndex &) = delete;
 
 public:
   ~GlobalModuleIndex();
 
-  /// An error code returned when trying to read an index.
-  enum ErrorCode {
-    /// No error occurred.
-    EC_None,
-    /// No index was found.
-    EC_NotFound,
-    /// Some other process is currently building the index; it is not
-    /// available yet.
-    EC_Building,
-    /// There was an unspecified I/O error reading or writing the index.
-    EC_IOError
-  };
-
   /// Read a global index file for the given directory.
   ///
   /// \param Path The path to the specific module cache where the module files
   /// for the intended configuration reside.
   ///
   /// \returns A pair containing the global module index (if it exists) and
-  /// the error code.
-  static std::pair<GlobalModuleIndex *, ErrorCode>
-  readIndex(llvm::StringRef Path);
+  /// the error.
+  static std::pair<GlobalModuleIndex *, llvm::Error> readIndex(llvm::StringRef Path);
 
   /// Returns an iterator for identifiers stored in the index table.
   ///
   /// The caller accepts ownership of the returned object.
   IdentifierIterator *createIdentifierIterator() const;
 
   /// Retrieve the set of modules that have up-to-date indexes.
   ///
public:
 
   /// Write a global index into the given
   ///
   /// \param FileMgr The file manager to use to load module files.
   /// \param PCHContainerRdr - The PCHContainerOperations to use for loading and
   /// creating modules.
   /// \param Path The path to the directory containing module files, into
   /// which the global index will be written.
-  static ErrorCode writeIndex(FileManager &FileMgr,
-                              const PCHContainerReader &PCHContainerRdr,
-                              llvm::StringRef Path);
+  static llvm::Error writeIndex(FileManager &FileMgr, const PCHContainerReader &PCHContainerRdr, llvm::StringRef Path);
 };
 }
 
 #endif

clang/lib/Frontend/ASTUnit.cpp

if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
     goto error;
 
   if (SavedMainFileBuffer)
     TranslateStoredDiagnostics(getFileManager(), getSourceManager(),
                                PreambleDiagnostics, StoredDiagnostics);
   else
     PreambleSrcLocCache.clear();
 
-  if (!Act->Execute())
+  if (llvm::Error Err = Act->Execute()) {
+    consumeError(std::move(Err)); // FIXME this drops errors on the floor.
     goto error;
+  }
 
   transferASTDataFromCompilerInstance(*Clang);
 
   Act->EndSourceFile();
 
   FailedParseDiagnostics.clear();
 
   return false;
if (Persistent && !TrackerAct) {
     std::vector<std::unique_ptr<ASTConsumer>> Consumers;
     if (Clang->hasASTConsumer())
       Consumers.push_back(Clang->takeASTConsumer());
     Consumers.push_back(llvm::make_unique<TopLevelDeclTrackerConsumer>(
         *AST, AST->getCurrentTopLevelHashValue()));
     Clang->setASTConsumer(
         llvm::make_unique<MultiplexConsumer>(std::move(Consumers)));
   }
-  if (!Act->Execute()) {
+  if (llvm::Error Err = Act->Execute()) {
+    consumeError(std::move(Err)); // FIXME this drops errors on the floor.
     AST->transferASTDataFromCompilerInstance(*Clang);
     if (OwnAST && ErrAST)
       ErrAST->swap(OwnAST);
 
     return nullptr;
   }
 
   // Steal the created target, context, and preprocessor.
void ASTUnit::CodeComplete(
 
   // Disable the preprocessing record if modules are not enabled.
   if (!Clang->getLangOpts().Modules)
     PreprocessorOpts.DetailedRecord = false;
 
   std::unique_ptr<SyntaxOnlyAction> Act;
   Act.reset(new SyntaxOnlyAction);
   if (Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0])) {
-    Act->Execute();
+    if (llvm::Error Err = Act->Execute()) {
+      consumeError(std::move(Err)); // FIXME this drops errors on the floor.
+    }
     Act->EndSourceFile();
   }
 }
 
 bool ASTUnit::Save(StringRef File) {
   if (HadModuleLoaderFatalFailure)
     return true;
 

clang/lib/Frontend/CompilerInstance.cpp

bool CompilerInstance::ExecuteAction(FrontendAction &Act) {
 
   for (const FrontendInputFile &FIF : getFrontendOpts().Inputs) {
     // Reset the ID tables if we are reusing the SourceManager and parsing
     // regular files.
     if (hasSourceManager() && !Act.isModelParsingAction())
       getSourceManager().clearIDTables();
 
     if (Act.BeginSourceFile(*this, FIF)) {
-      Act.Execute();
+      if (llvm::Error Err = Act.Execute()) {
+        consumeError(std::move(Err)); // FIXME this drops errors on the floor.
+      }
       Act.EndSourceFile();
     }
   }
 
   // Notify the diagnostic client that all files were processed.
   getDiagnostics().getClient()->finish();
 
   if (getDiagnosticOpts().ShowCarets) {
GlobalModuleIndex *CompilerInstance::loadGlobalModuleIndex(
   // loaded.
   ModuleManager->loadGlobalIndex();
   GlobalModuleIndex *GlobalIndex = ModuleManager->getGlobalIndex();
   // If the global index doesn't exist, create it.
   if (!GlobalIndex && shouldBuildGlobalModuleIndex() && hasFileManager() &&
       hasPreprocessor()) {
     llvm::sys::fs::create_directories(
       getPreprocessor().getHeaderSearchInfo().getModuleCachePath());
-    GlobalModuleIndex::writeIndex(
-        getFileManager(), getPCHContainerReader(),
-        getPreprocessor().getHeaderSearchInfo().getModuleCachePath());
+    if (llvm::Error Err = GlobalModuleIndex::writeIndex(getFileManager(), getPCHContainerReader(), getPreprocessor().getHeaderSearchInfo().getModuleCachePath())) {
+      // FIXME this drops the error on the floor. This code is only used for
+      // typo correction and drops more than just this one source of errors
+      // (such as the directory creation failure above).
+    }
     ModuleManager->resetForReload();
     ModuleManager->loadGlobalIndex();
     GlobalIndex = ModuleManager->getGlobalIndex();
   }
   // For finding modules needing to be imported for fixit messages,
   // we need to make the global index cover all modules, so we do that here.
   if (!HaveFullGlobalModuleIndex && GlobalIndex && !buildingModule()) {
     ModuleMap &MMap = getPreprocessor().getHeaderSearchInfo().getModuleMap();
     bool RecreateIndex = false;
     for (ModuleMap::module_iterator I = MMap.module_begin(),
         E = MMap.module_end(); I != E; ++I) {
       Module *TheModule = I->second;
       const FileEntry *Entry = TheModule->getASTFile();
       if (!Entry) {
         SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
         Path.push_back(std::make_pair(
             getPreprocessor().getIdentifierInfo(TheModule->Name), TriggerLoc));
         std::reverse(Path.begin(), Path.end());
         // Load a module as hidden.  This also adds it to the global index.
         loadModule(TheModule->DefinitionLoc, Path, Module::Hidden, false);
         RecreateIndex = true;
       }
     }
     if (RecreateIndex) {
-      GlobalModuleIndex::writeIndex(
-          getFileManager(), getPCHContainerReader(),
-          getPreprocessor().getHeaderSearchInfo().getModuleCachePath());
+      if (llvm::Error Err = GlobalModuleIndex::writeIndex(getFileManager(), getPCHContainerReader(), getPreprocessor().getHeaderSearchInfo().getModuleCachePath())) {
+        // FIXME As above, this drops the error on the floor.
+      }
       ModuleManager->resetForReload();
       ModuleManager->loadGlobalIndex();
       GlobalIndex = ModuleManager->getGlobalIndex();
     }
     HaveFullGlobalModuleIndex = true;
   }
   return GlobalIndex;
 }
 

clang/lib/Frontend/FrontendAction.cpp

if (HasBegunSourceFile)
     CI.getDiagnosticClient().EndSourceFile();
   CI.clearOutputFiles(/*EraseFiles=*/true);
   CI.getLangOpts().setCompilingModule(LangOptions::CMK_None);
   setCurrentInput(FrontendInputFile());
   setCompilerInstance(nullptr);
   return false;
 }
 
-bool FrontendAction::Execute() {
+llvm::Error FrontendAction::Execute() {
   CompilerInstance &CI = getCompilerInstance();
 
   if (CI.hasFrontendTimer()) {
     llvm::TimeRegion Timer(CI.getFrontendTimer());
     ExecuteAction();
   }
   else ExecuteAction();
 
   // If we are supposed to rebuild the global module index, do so now unless
   // there were any module-build failures.
   if (CI.shouldBuildGlobalModuleIndex() && CI.hasFileManager() &&
       CI.hasPreprocessor()) {
     StringRef Cache =
         CI.getPreprocessor().getHeaderSearchInfo().getModuleCachePath();
-    if (!Cache.empty())
-      GlobalModuleIndex::writeIndex(CI.getFileManager(),
-                                    CI.getPCHContainerReader(), Cache);
+    if (!Cache.empty()) {
+      if (llvm::Error Err = GlobalModuleIndex::writeIndex(CI.getFileManager(), CI.getPCHContainerReader(), Cache)) {
+        // FIXME this drops the error on the floor, but Index/pch-from-libclang.c seems to rely on dropping at least some of the error conditions!
+        consumeError(std::move(Err));
+      }
+    }
   }
 
-  return true;
+  return llvm::Error::success();
 }
 
 void FrontendAction::EndSourceFile() {
   CompilerInstance &CI = getCompilerInstance();
 
   // Inform the diagnostic client we are done with this source file.
   CI.getDiagnosticClient().EndSourceFile();
 

clang/lib/Frontend/PrecompiledPreamble.cpp

llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
 
   std::unique_ptr<PPCallbacks> DelegatedPPCallbacks =
       Callbacks.createPPCallbacks();
   if (DelegatedPPCallbacks)
     Clang->getPreprocessor().addPPCallbacks(std::move(DelegatedPPCallbacks));
   if (auto CommentHandler = Callbacks.getCommentHandler())
     Clang->getPreprocessor().addCommentHandler(CommentHandler);
 
-  Act->Execute();
+  if (llvm::Error Err = Act->Execute())
+    return errorToErrorCode(std::move(Err));
 
   // Run the callbacks.
   Callbacks.AfterExecute(*Clang);
 
   Act->EndSourceFile();
 
   if (!Act->hasEmittedPreamblePCH())
     return BuildPreambleError::CouldntEmitPCH;

clang/lib/Frontend/Rewrite/FrontendActions.cpp

if (FixAction->BeginSourceFile(CI, FEOpts.Inputs[0])) {
         FixItOpts.reset(new FixItRewriteToTemp());
       else
         FixItOpts.reset(new FixItRewriteInPlace());
       FixItOpts->Silent = true;
       FixItOpts->FixWhatYouCan = FEOpts.FixWhatYouCan;
       FixItOpts->FixOnlyWarnings = FEOpts.FixOnlyWarnings;
       FixItRewriter Rewriter(CI.getDiagnostics(), CI.getSourceManager(),
                              CI.getLangOpts(), FixItOpts.get());
-      FixAction->Execute();
+      if (llvm::Error Err = FixAction->Execute())
+        err = true; // FIXME this drops the error on the floor.
 
       err = Rewriter.WriteFixedFiles(&RewrittenFiles);
 
       FixAction->EndSourceFile();
       CI.setSourceManager(nullptr);
       CI.setFileManager(nullptr);
     } else {
       err = true;
     }

clang/lib/Frontend/SerializedDiagnosticReader.cpp

std::error_code SerializedDiagnosticReader::readDiagnostics(StringRef File) {
 
   llvm::BitstreamCursor Stream(**Buffer);
   Optional<llvm::BitstreamBlockInfo> BlockInfo;
 
   if (Stream.AtEndOfStream())
     return SDError::InvalidSignature;
 
   // Sniff for the signature.
-  if (Stream.Read(8) != 'D' ||
-      Stream.Read(8) != 'I' ||
-      Stream.Read(8) != 'A' ||
-      Stream.Read(8) != 'G')
+  for (unsigned char C : {'D', 'I', 'A', 'G'}) {
+    if (Expected<llvm::SimpleBitstreamCursor::word_t> Res = Stream.Read(8)) {
+      if (Res.get() == C)
+        continue;
+    } else
+      (void)Res.takeError(); // FIXME propagate the error details.
     return SDError::InvalidSignature;
+  }
 
   // Read the top level blocks.
   while (!Stream.AtEndOfStream()) {
-    if (Stream.ReadCode() != llvm::bitc::ENTER_SUBBLOCK)
+    if (Expected<unsigned> Res = Stream.ReadCode()) {
+      if (Res.get() != llvm::bitc::ENTER_SUBBLOCK)
         return SDError::InvalidDiagnostics;
+    } else
+        return SDError::InvalidDiagnostics; // FIXME propagate the error details.
 
     std::error_code EC;
-    switch (Stream.ReadSubBlockID()) {
+    Expected<unsigned> SubBlockID = Stream.ReadSubBlockID();
+    if (!SubBlockID)
+        return SDError::InvalidDiagnostics; // FIXME propagate the error details.
+
+    switch (SubBlockID.get()) {
     case llvm::bitc::BLOCKINFO_BLOCK_ID:
-      BlockInfo = Stream.ReadBlockInfoBlock();
+      {
+        Expected<Optional<llvm::BitstreamBlockInfo>> MaybeBlockInfo = Stream.ReadBlockInfoBlock();
+        if (!MaybeBlockInfo) {
+          (void)MaybeBlockInfo.takeError(); // FIXME propagate the error details.
+          return SDError::InvalidDiagnostics;
+        }
+        BlockInfo = std::move(MaybeBlockInfo.get());
+      }
       if (!BlockInfo)
         return SDError::MalformedBlockInfoBlock;
       Stream.setBlockInfo(&*BlockInfo);
       continue;
     case BLOCK_META:
       if ((EC = readMetaBlock(Stream)))
         return EC;
       continue;
     case BLOCK_DIAG:
       if ((EC = readDiagnosticBlock(Stream)))
         return EC;
       continue;
     default:
-      if (!Stream.SkipBlock())
-        return SDError::MalformedTopLevelBlock;
+      if (llvm::Error Err = Stream.SkipBlock())
+        return SDError::MalformedTopLevelBlock; // FIXME propagate the error details.
       continue;
     }
   }
   return {};
 }
 
 enum class SerializedDiagnosticReader::Cursor {
   Record = 1,
   BlockEnd,
   BlockBegin
 };
 
 llvm::ErrorOr<SerializedDiagnosticReader::Cursor>
 SerializedDiagnosticReader::skipUntilRecordOrBlock(
     llvm::BitstreamCursor &Stream, unsigned &BlockOrRecordID) {
   BlockOrRecordID = 0;
 
   while (!Stream.AtEndOfStream()) {
-    unsigned Code = Stream.ReadCode();
+    unsigned Code;
+    if (Expected<unsigned> Res = Stream.ReadCode())
+      Code = Res.get();
+    else
+      return llvm::errorToErrorCode(Res.takeError());
 
     switch ((llvm::bitc::FixedAbbrevIDs)Code) {
     case llvm::bitc::ENTER_SUBBLOCK:
-      BlockOrRecordID = Stream.ReadSubBlockID();
+      if (Expected<unsigned> Res = Stream.ReadSubBlockID())
+        BlockOrRecordID = Res.get();
+      else
+        return llvm::errorToErrorCode(Res.takeError());
       return Cursor::BlockBegin;
 
     case llvm::bitc::END_BLOCK:
       if (Stream.ReadBlockEnd())
         return SDError::InvalidDiagnostics;
       return Cursor::BlockEnd;
 
     case llvm::bitc::DEFINE_ABBREV:
-      Stream.ReadAbbrevRecord();
+      if (llvm::Error Err = Stream.ReadAbbrevRecord())
+        return llvm::errorToErrorCode(std::move(Err));
       continue;
 
     case llvm::bitc::UNABBREV_RECORD:
       return SDError::UnsupportedConstruct;
 
     default:
       // We found a record.
       BlockOrRecordID = Code;
       return Cursor::Record;
     }
   }
 
   return SDError::InvalidDiagnostics;
 }
 
 std::error_code
 SerializedDiagnosticReader::readMetaBlock(llvm::BitstreamCursor &Stream) {
-  if (Stream.EnterSubBlock(clang::serialized_diags::BLOCK_META))
-    return SDError::MalformedMetadataBlock;
+  if (llvm::Error Err = Stream.EnterSubBlock(clang::serialized_diags::BLOCK_META))
+    return SDError::MalformedMetadataBlock; // FIXME propagate the error details.
 
   bool VersionChecked = false;
 
   while (true) {
     unsigned BlockOrCode = 0;
     llvm::ErrorOr<Cursor> Res = skipUntilRecordOrBlock(Stream, BlockOrCode);
     if (!Res)
       Res.getError();
 
     switch (Res.get()) {
     case Cursor::Record:
       break;
     case Cursor::BlockBegin:
-      if (Stream.SkipBlock())
-        return SDError::MalformedMetadataBlock;
+        if (llvm::Error Err = Stream.SkipBlock())
+        return SDError::MalformedMetadataBlock; // FIXME propagate the error details.
       LLVM_FALLTHROUGH;
     case Cursor::BlockEnd:
       if (!VersionChecked)
         return SDError::MissingVersion;
       return {};
     }
 
     SmallVector<uint64_t, 1> Record;
-    unsigned RecordID = Stream.readRecord(BlockOrCode, Record);
+    Expected<unsigned> MaybeRecordID = Stream.readRecord(BlockOrCode, Record);
+    if (!MaybeRecordID)
+      return errorToErrorCode(MaybeRecordID.takeError());
+    unsigned RecordID = MaybeRecordID.get();
 
     if (RecordID == RECORD_VERSION) {
       if (Record.size() < 1)
         return SDError::MissingVersion;
       if (Record[0] > VersionNumber)
         return SDError::VersionMismatch;
       VersionChecked = true;
     }
   }
 }
 
 std::error_code
 SerializedDiagnosticReader::readDiagnosticBlock(llvm::BitstreamCursor &Stream) {
-  if (Stream.EnterSubBlock(clang::serialized_diags::BLOCK_DIAG))
-    return SDError::MalformedDiagnosticBlock;
+  if (llvm::Error Err = Stream.EnterSubBlock(clang::serialized_diags::BLOCK_DIAG))
+    return SDError::MalformedDiagnosticBlock; // FIXME propagate the error details.
 
   std::error_code EC;
   if ((EC = visitStartOfDiagnostic()))
     return EC;
 
   SmallVector<uint64_t, 16> Record;
   while (true) {
     unsigned BlockOrCode = 0;
     llvm::ErrorOr<Cursor> Res = skipUntilRecordOrBlock(Stream, BlockOrCode);
     if (!Res)
       Res.getError();
 
     switch (Res.get()) {
     case Cursor::BlockBegin:
       // The only blocks we care about are subdiagnostics.
       if (BlockOrCode == serialized_diags::BLOCK_DIAG) {
         if ((EC = readDiagnosticBlock(Stream)))
           return EC;
-      } else if (!Stream.SkipBlock())
-        return SDError::MalformedSubBlock;
+      } else if (llvm::Error Err = Stream.SkipBlock())
+        return SDError::MalformedSubBlock; // FIXME propagate the error details.
       continue;
     case Cursor::BlockEnd:
       if ((EC = visitEndOfDiagnostic()))
         return EC;
       return {};
     case Cursor::Record:
       break;
     }
 
     // Read the record.
     Record.clear();
     StringRef Blob;
-    unsigned RecID = Stream.readRecord(BlockOrCode, Record, &Blob);
+    Expected<unsigned> MaybeRecID = Stream.readRecord(BlockOrCode, Record, &Blob);
+    if (!MaybeRecID)
+      return errorToErrorCode(MaybeRecID.takeError());
+    unsigned RecID = MaybeRecID.get();
 
     if (RecID < serialized_diags::RECORD_FIRST ||
         RecID > serialized_diags::RECORD_LAST)
       continue;
 
     switch ((RecordIDs)RecID) {
     case RECORD_CATEGORY:
       // A category has ID and name size.

clang/lib/Frontend/TestModuleFileExtension.cpp

 
 TestModuleFileExtension::Reader::Reader(ModuleFileExtension *Ext,
                                         const llvm::BitstreamCursor &InStream)
   : ModuleFileExtensionReader(Ext), Stream(InStream)
 {
   // Read the extension block.
   SmallVector<uint64_t, 4> Record;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    llvm::Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry)
+      (void)MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::SubBlock:
     case llvm::BitstreamEntry::EndBlock:
     case llvm::BitstreamEntry::Error:
       return;
 
     case llvm::BitstreamEntry::Record:
       break;
     }
 
     Record.clear();
     StringRef Blob;
-    unsigned RecCode = Stream.readRecord(Entry.ID, Record, &Blob);
-    switch (RecCode) {
+    Expected<unsigned> MaybeRecCode = Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeRecCode)
+      fprintf(stderr, "Failed reading rec code: %s\n", toString(MaybeRecCode.takeError()).c_str());
+    switch (MaybeRecCode.get()) {
     case FIRST_EXTENSION_RECORD_ID: {
       StringRef Message = Blob.substr(0, Record[0]);
       fprintf(stderr, "Read extension block message: %s\n",
               Message.str().c_str());
       break;
     }
     }
   }

clang/lib/Serialization/ASTReader.cpp

 
 bool ASTReader::ReadLexicalDeclContextStorage(ModuleFile &M,
                                               BitstreamCursor &Cursor,
                                               uint64_t Offset,
                                               DeclContext *DC) {
   assert(Offset != 0);
 
   SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(Offset);
+  if (llvm::Error Err = Cursor.JumpToBit(Offset)) {
+    Error(std::move(Err));
+    return true;
+  }
 
   RecordData Record;
   StringRef Blob;
-  unsigned Code = Cursor.ReadCode();
-  unsigned RecCode = Cursor.readRecord(Code, Record, &Blob);
+  Expected<unsigned> MaybeCode = Cursor.ReadCode();
+  if (!MaybeCode) {
+    Error(MaybeCode.takeError());
+    return true;
+  }
+  unsigned Code = MaybeCode.get();
+
+  Expected<unsigned> MaybeRecCode = Cursor.readRecord(Code, Record, &Blob);
+  if (!MaybeRecCode) {
+    Error(MaybeRecCode.takeError());
+    return true;
+  }
+  unsigned RecCode = MaybeRecCode.get();
   if (RecCode != DECL_CONTEXT_LEXICAL) {
     Error("Expected lexical block");
     return true;
   }
 
   assert(!isa<TranslationUnitDecl>(DC) &&
          "expected a TU_UPDATE_LEXICAL record for TU");
   // If we are handling a C++ class template instantiation, we can see multiple

 
 bool ASTReader::ReadVisibleDeclContextStorage(ModuleFile &M,
                                               BitstreamCursor &Cursor,
                                               uint64_t Offset,
                                               DeclID ID) {
   assert(Offset != 0);
 
   SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(Offset);
+  if (llvm::Error Err = Cursor.JumpToBit(Offset)) {
+    Error(std::move(Err));
+    return true;
+  }
 
   RecordData Record;
   StringRef Blob;
-  unsigned Code = Cursor.ReadCode();
-  unsigned RecCode = Cursor.readRecord(Code, Record, &Blob);
+  Expected<unsigned> MaybeCode = Cursor.ReadCode();
+  if (!MaybeCode) {
+    Error(MaybeCode.takeError());
+    return true;
+  }
+  unsigned Code = MaybeCode.get();
+
+  Expected<unsigned> MaybeRecCode = Cursor.readRecord(Code, Record, &Blob);
+  if (!MaybeRecCode) {
+    Error(MaybeRecCode.takeError());
+    return true;
+  }
+  unsigned RecCode = MaybeRecCode.get();
   if (RecCode != DECL_CONTEXT_VISIBLE) {
     Error("Expected visible lookup table block");
     return true;
   }
 
   // We can't safely determine the primary context yet, so delay attaching the
   // lookup table until we're done with recursive deserialization.
   auto *Data = (const unsigned char*)Blob.data();

 void ASTReader::Error(unsigned DiagID,
                       StringRef Arg1, StringRef Arg2) const {
   if (Diags.isDiagnosticInFlight())
     Diags.SetDelayedDiagnostic(DiagID, Arg1, Arg2);
   else
     Diag(DiagID) << Arg1 << Arg2;
 }
 
+void ASTReader::Error(llvm::Error &&Err) const {
+  Error(toString(std::move(Err)));
+}
+
 //===----------------------------------------------------------------------===//
 // Source Manager Deserialization
 //===----------------------------------------------------------------------===//
 
 /// Read the line table in the source manager block.
 /// \returns true if there was an error.
 bool ASTReader::ParseLineTable(ModuleFile &F,
                                const RecordData &Record) {
bool ASTReader::ReadSourceManagerBlock(ModuleFile &F) {
 
   // Set the source-location entry cursor to the current position in
   // the stream. This cursor will be used to read the contents of the
   // source manager block initially, and then lazily read
   // source-location entries as needed.
   SLocEntryCursor = F.Stream;
 
   // The stream itself is going to skip over the source manager block.
-  if (F.Stream.SkipBlock()) {
-    Error("malformed block record in AST file");
+  if (llvm::Error Err = F.Stream.SkipBlock()) {
+    Error(std::move(Err));
     return true;
   }
 
   // Enter the source manager block.
-  if (SLocEntryCursor.EnterSubBlock(SOURCE_MANAGER_BLOCK_ID)) {
-    Error("malformed source manager block record in AST file");
+  if (llvm::Error Err = SLocEntryCursor.EnterSubBlock(SOURCE_MANAGER_BLOCK_ID)) {
+    Error(std::move(Err));
     return true;
   }
 
   RecordData Record;
   while (true) {
-    llvm::BitstreamEntry E = SLocEntryCursor.advanceSkippingSubblocks();
+    Expected<llvm::BitstreamEntry> MaybeE = SLocEntryCursor.advanceSkippingSubblocks();
+    if (!MaybeE) {
+      Error(MaybeE.takeError());
+      return true;
+    }
+    llvm::BitstreamEntry E = MaybeE.get();
 
     switch (E.Kind) {
     case llvm::BitstreamEntry::SubBlock: // Handled for us already.
     case llvm::BitstreamEntry::Error:
       Error("malformed block record in AST file");
       return true;
     case llvm::BitstreamEntry::EndBlock:
       return false;
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record.
     Record.clear();
     StringRef Blob;
-    switch (SLocEntryCursor.readRecord(E.ID, Record, &Blob)) {
+    Expected<unsigned> MaybeRecord = SLocEntryCursor.readRecord(E.ID, Record, &Blob);
+    if (!MaybeRecord) {
+      Error(MaybeRecord.takeError());
+      return true;
+    }
+    switch (MaybeRecord.get()) {
     default:  // Default behavior: ignore.
       break;
 
     case SM_SLOC_FILE_ENTRY:
     case SM_SLOC_BUFFER_ENTRY:
     case SM_SLOC_EXPANSION_ENTRY:
       // Once we hit one of the source location entries, we're done.
       return false;
bool ASTReader::ReadSLocEntry(int ID) {
 
   // Local helper to read the (possibly-compressed) buffer data following the
   // entry record.
   auto ReadBuffer = [this](
       BitstreamCursor &SLocEntryCursor,
       StringRef Name) -> std::unique_ptr<llvm::MemoryBuffer> {
     RecordData Record;
     StringRef Blob;
-    unsigned Code = SLocEntryCursor.ReadCode();
-    unsigned RecCode = SLocEntryCursor.readRecord(Code, Record, &Blob);
+    Expected<unsigned> MaybeCode = SLocEntryCursor.ReadCode();
+    if (!MaybeCode) {
+      Error(MaybeCode.takeError());
+      return nullptr;
+    }
+    unsigned Code = MaybeCode.get();
+
+    Expected<unsigned> MaybeRecCode = SLocEntryCursor.readRecord(Code, Record, &Blob);
+    if (!MaybeRecCode) {
+      Error(MaybeRecCode.takeError());
+      return nullptr;
+    }
+    unsigned RecCode = MaybeRecCode.get();
 
     if (RecCode == SM_SLOC_BUFFER_BLOB_COMPRESSED) {
       if (!llvm::zlib::isAvailable()) {
         Error("zlib is not available");
         return nullptr;
       }
       SmallString<0> Uncompressed;
       if (llvm::Error E =
               llvm::zlib::uncompress(Blob, Uncompressed, Record[0])) {
         Error("could not decompress embedded file contents: " +
               llvm::toString(std::move(E)));
         return nullptr;
       }
       return llvm::MemoryBuffer::getMemBufferCopy(Uncompressed, Name);
     } else if (RecCode == SM_SLOC_BUFFER_BLOB) {
       return llvm::MemoryBuffer::getMemBuffer(Blob.drop_back(1), Name, true);
     } else {
       Error("AST record has invalid code");
       return nullptr;
     }
   };
 
   ModuleFile *F = GlobalSLocEntryMap.find(-ID)->second;
-  F->SLocEntryCursor.JumpToBit(F->SLocEntryOffsets[ID - F->SLocEntryBaseID]);
+  if (llvm::Error Err = F->SLocEntryCursor.JumpToBit(F->SLocEntryOffsets[ID - F->SLocEntryBaseID])) {
+    Error(std::move(Err));
+    return true;
+  }
+
   BitstreamCursor &SLocEntryCursor = F->SLocEntryCursor;
   unsigned BaseOffset = F->SLocEntryBaseOffset;
 
   ++NumSLocEntriesRead;
-  llvm::BitstreamEntry Entry = SLocEntryCursor.advance();
+  Expected<llvm::BitstreamEntry> MaybeEntry = SLocEntryCursor.advance();
+  if (!MaybeEntry) {
+    Error(MaybeEntry.takeError());
+    return true;
+  }
+  llvm::BitstreamEntry Entry = MaybeEntry.get();
+
   if (Entry.Kind != llvm::BitstreamEntry::Record) {
     Error("incorrectly-formatted source location entry in AST file");
     return true;
   }
 
   RecordData Record;
   StringRef Blob;
-  switch (SLocEntryCursor.readRecord(Entry.ID, Record, &Blob)) {
+  Expected<unsigned> MaybeSLOC = SLocEntryCursor.readRecord(Entry.ID, Record, &Blob);
+  if (!MaybeSLOC) {
+    Error(MaybeSLOC.takeError());
+    return true;
+  }
+  switch (MaybeSLOC.get()) {
   default:
     Error("incorrectly-formatted source location entry in AST file");
     return true;
 
   case SM_SLOC_FILE_ENTRY: {
     // We will detect whether a file changed and return 'Failure' for it, but
     // we will also try to fail gracefully by setting up the SLocEntry.
     unsigned InputID = Record[4];
SourceLocation ASTReader::getImportLocation(ModuleFile *F) {
   if (F->ImportedBy.empty() || !F->ImportedBy[0]) {
     // Main file is the importer.
     assert(SourceMgr.getMainFileID().isValid() && "missing main file");
     return SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
   }
   return F->ImportedBy[0]->FirstLoc;
 }
 
-/// ReadBlockAbbrevs - Enter a subblock of the specified BlockID with the
-/// specified cursor.  Read the abbreviations that are at the top of the block
-/// and then leave the cursor pointing into the block.
+/// Enter a subblock of the specified BlockID with the specified cursor. Read
+/// the abbreviations that are at the top of the block and then leave the cursor
+/// pointing into the block.
 bool ASTReader::ReadBlockAbbrevs(BitstreamCursor &Cursor, unsigned BlockID) {
-  if (Cursor.EnterSubBlock(BlockID))
+  if (llvm::Error Err = Cursor.EnterSubBlock(BlockID)) {
+    // FIXME this drops errors on the floor.
     return true;
+  }
 
   while (true) {
     uint64_t Offset = Cursor.GetCurrentBitNo();
-    unsigned Code = Cursor.ReadCode();
+    Expected<unsigned> MaybeCode = Cursor.ReadCode();
+    if (!MaybeCode) {
+      // FIXME this drops errors on the floor.
+      return true;
+    }
+    unsigned Code = MaybeCode.get();
 
     // We expect all abbrevs to be at the start of the block.
     if (Code != llvm::bitc::DEFINE_ABBREV) {
-      Cursor.JumpToBit(Offset);
+      if (llvm::Error Err = Cursor.JumpToBit(Offset)) {
+        // FIXME this drops errors on the floor.
+        return true;
+      }
       return false;
     }
-    Cursor.ReadAbbrevRecord();
+    if (llvm::Error Err = Cursor.ReadAbbrevRecord()) {
+      // FIXME this drops errors on the floor.
+      return true;
+    }
   }
 }
 
 Token ASTReader::ReadToken(ModuleFile &F, const RecordDataImpl &Record,
                            unsigned &Idx) {
   Token Tok;
   Tok.startToken();
   Tok.setLocation(ReadSourceLocation(F, Record, Idx));
   Tok.setLength(Record[Idx++]);
   if (IdentifierInfo *II = getLocalIdentifier(F, Record[Idx++]))
     Tok.setIdentifierInfo(II);
   Tok.setKind((tok::TokenKind)Record[Idx++]);
   Tok.setFlag((Token::TokenFlags)Record[Idx++]);
   return Tok;
 }
 
 MacroInfo *ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset) {
   BitstreamCursor &Stream = F.MacroCursor;
 
   // Keep track of where we are in the stream, then jump back there
   // after reading this macro.
   SavedStreamPosition SavedPosition(Stream);
 
-  Stream.JumpToBit(Offset);
+  if (llvm::Error Err = Stream.JumpToBit(Offset)) {
+    // FIXME this drops errors on the floor.
+    return nullptr;
+  }
   RecordData Record;
   SmallVector<IdentifierInfo*, 16> MacroParams;
   MacroInfo *Macro = nullptr;
 
   while (true) {
     // Advance to the next record, but if we get to the end of the block, don't
     // pop it (removing all the abbreviations from the cursor) since we want to
     // be able to reseek within the block and read entries.
     unsigned Flags = BitstreamCursor::AF_DontPopBlockAtEnd;
-    llvm::BitstreamEntry Entry = Stream.advanceSkippingSubblocks(Flags);
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks(Flags);
+    if (!MaybeEntry) {
+      Error(MaybeEntry.takeError());
+      return Macro;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::SubBlock: // Handled for us already.
     case llvm::BitstreamEntry::Error:
       Error("malformed block record in AST file");
       return Macro;
     case llvm::BitstreamEntry::EndBlock:
       return Macro;
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record.
     Record.clear();
-    PreprocessorRecordTypes RecType =
-      (PreprocessorRecordTypes)Stream.readRecord(Entry.ID, Record);
+    PreprocessorRecordTypes RecType;
+    if (Expected<unsigned> MaybeRecType = Stream.readRecord(Entry.ID, Record))
+      RecType = (PreprocessorRecordTypes)MaybeRecType.get();
+    else {
+      Error(MaybeRecType.takeError());
+      return Macro;
+    }
     switch (RecType) {
     case PP_MODULE_MACRO:
     case PP_MACRO_DIRECTIVE_HISTORY:
       return Macro;
 
     case PP_MACRO_OBJECT_LIKE:
     case PP_MACRO_FUNCTION_LIKE: {
       // If we already have a macro, that means that we've hit the end
void ASTReader::ReadDefinedMacros() {
   for (ModuleFile &I : llvm::reverse(ModuleMgr)) {
     BitstreamCursor &MacroCursor = I.MacroCursor;
 
     // If there was no preprocessor block, skip this file.
     if (MacroCursor.getBitcodeBytes().empty())
       continue;
 
     BitstreamCursor Cursor = MacroCursor;
-    Cursor.JumpToBit(I.MacroStartOffset);
+    if (llvm::Error Err = Cursor.JumpToBit(I.MacroStartOffset)) {
+      Error(std::move(Err));
+      return;
+    }
 
     RecordData Record;
     while (true) {
-      llvm::BitstreamEntry E = Cursor.advanceSkippingSubblocks();
+      Expected<llvm::BitstreamEntry> MaybeE = Cursor.advanceSkippingSubblocks();
+      if (!MaybeE) {
+        Error(MaybeE.takeError());
+        return;
+      }
+      llvm::BitstreamEntry E = MaybeE.get();
 
       switch (E.Kind) {
       case llvm::BitstreamEntry::SubBlock: // Handled for us already.
       case llvm::BitstreamEntry::Error:
         Error("malformed block record in AST file");
         return;
       case llvm::BitstreamEntry::EndBlock:
         goto NextCursor;
 
-      case llvm::BitstreamEntry::Record:
+      case llvm::BitstreamEntry::Record: {
         Record.clear();
-        switch (Cursor.readRecord(E.ID, Record)) {
+        Expected<unsigned> MaybeRecord = Cursor.readRecord(E.ID, Record);
+        if (!MaybeRecord) {
+          Error(MaybeRecord.takeError());
+          return;
+        }
+        switch (MaybeRecord.get()) {
         default:  // Default behavior: ignore.
           break;
 
         case PP_MACRO_OBJECT_LIKE:
         case PP_MACRO_FUNCTION_LIKE: {
           IdentifierInfo *II = getLocalIdentifier(I, Record[0]);
           if (II->isOutOfDate())
             updateOutOfDateIdentifier(*II);
           break;
         }
 
         case PP_TOKEN:
           // Ignore tokens.
           break;
         }
         break;
       }
       }
+    }
     NextCursor:  ;
   }
 }
 
 namespace {
 
   /// Visitor class used to look up identifirs in an AST file.
   class IdentifierLookupVisitor {

 }
 
 void ASTReader::resolvePendingMacro(IdentifierInfo *II,
                                     const PendingMacroInfo &PMInfo) {
   ModuleFile &M = *PMInfo.M;
 
   BitstreamCursor &Cursor = M.MacroCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(PMInfo.MacroDirectivesOffset);
+  if (llvm::Error Err = Cursor.JumpToBit(PMInfo.MacroDirectivesOffset)) {
+    Error(std::move(Err));
+    return;
+  }
 
   struct ModuleMacroRecord {
     SubmoduleID SubModID;
     MacroInfo *MI;
     SmallVector<SubmoduleID, 8> Overrides;
   };
   llvm::SmallVector<ModuleMacroRecord, 8> ModuleMacros;
 
   // We expect to see a sequence of PP_MODULE_MACRO records listing exported
   // macros, followed by a PP_MACRO_DIRECTIVE_HISTORY record with the complete
   // macro histroy.
   RecordData Record;
   while (true) {
-    llvm::BitstreamEntry Entry =
+    Expected<llvm::BitstreamEntry> MaybeEntry =
         Cursor.advance(BitstreamCursor::AF_DontPopBlockAtEnd);
+    if (!MaybeEntry) {
+      Error(MaybeEntry.takeError());
+      return;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     if (Entry.Kind != llvm::BitstreamEntry::Record) {
       Error("malformed block record in AST file");
       return;
     }
 
     Record.clear();
-    switch ((PreprocessorRecordTypes)Cursor.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybePP = Cursor.readRecord(Entry.ID, Record);
+    if (!MaybePP) {
+      Error(MaybePP.takeError());
+      return;
+    }
+    switch ((PreprocessorRecordTypes)MaybePP.get()) {
     case PP_MACRO_DIRECTIVE_HISTORY:
       break;
 
     case PP_MODULE_MACRO: {
       ModuleMacros.push_back(ModuleMacroRecord());
       auto &Info = ModuleMacros.back();
       Info.SubModID = getGlobalSubmoduleID(M, Record[0]);
       Info.MI = getMacro(getGlobalMacroID(M, Record[1]));
if (Latest)
     PP.setLoadedMacroDirective(II, Earliest, Latest);
 }
 
 ASTReader::InputFileInfo
 ASTReader::readInputFileInfo(ModuleFile &F, unsigned ID) {
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(F.InputFileOffsets[ID-1]);
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID-1])) {
+    // FIXME this drops errors on the floor.
+  }
 
-  unsigned Code = Cursor.ReadCode();
+  Expected<unsigned> MaybeCode = Cursor.ReadCode();
+  if (!MaybeCode) {
+    // FIXME this drops errors on the floor.
+  }
+  unsigned Code = MaybeCode.get();
   RecordData Record;
   StringRef Blob;
 
-  unsigned Result = Cursor.readRecord(Code, Record, &Blob);
-  assert(static_cast<InputFileRecordTypes>(Result) == INPUT_FILE &&
-         "invalid record type for input file");
-  (void)Result;
+  if (Expected<unsigned> Maybe = Cursor.readRecord(Code, Record, &Blob))
+    assert(static_cast<InputFileRecordTypes>(Maybe.get()) == INPUT_FILE && "invalid record type for input file");
+  else {
+    // FIXME this drops errors on the floor.
+  }
 
   assert(Record[0] == ID && "Bogus stored ID or offset");
   InputFileInfo R;
   R.StoredSize = static_cast<off_t>(Record[1]);
   R.StoredTime = static_cast<time_t>(Record[2]);
   R.Overridden = static_cast<bool>(Record[3]);
   R.Transient = static_cast<bool>(Record[4]);
   R.TopLevelModuleMap = static_cast<bool>(Record[5]);
if (F.InputFilesLoaded[ID-1].getFile())
     return F.InputFilesLoaded[ID-1];
 
   if (F.InputFilesLoaded[ID-1].isNotFound())
     return InputFile();
 
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(F.InputFileOffsets[ID-1]);
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID-1])) {
+    // FIXME this drops errors on the floor.
+  }
 
   InputFileInfo FI = readInputFileInfo(F, ID);
   off_t StoredSize = FI.StoredSize;
   time_t StoredTime = FI.StoredTime;
   bool Overridden = FI.Overridden;
   bool Transient = FI.Transient;
   StringRef Filename = FI.Filename;
 
static bool isDiagnosedResult(ASTReader::ASTReadResult ARR, unsigned Caps) {
 
   llvm_unreachable("unknown ASTReadResult");
 }
 
 ASTReader::ASTReadResult ASTReader::ReadOptionsBlock(
     BitstreamCursor &Stream, unsigned ClientLoadCapabilities,
     bool AllowCompatibleConfigurationMismatch, ASTReaderListener &Listener,
     std::string &SuggestedPredefines) {
-  if (Stream.EnterSubBlock(OPTIONS_BLOCK_ID))
+    if (llvm::Error Err = Stream.EnterSubBlock(OPTIONS_BLOCK_ID)) {
+      // FIXME this drops errors on the floor.
       return Failure;
+    }
 
   // Read all of the records in the options block.
   RecordData Record;
   ASTReadResult Result = Success;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry) {
+      // FIXME this drops errors on the floor.
+      return Failure;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::Error:
     case llvm::BitstreamEntry::SubBlock:
       return Failure;
 
     case llvm::BitstreamEntry::EndBlock:
       return Result;
 
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read and process a record.
     Record.clear();
-    switch ((OptionsRecordTypes)Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecordType = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecordType) {
+      // FIXME this drops errors on the floor.
+      return Failure;
+    }
+    switch ((OptionsRecordTypes)MaybeRecordType.get()) {
     case LANGUAGE_OPTIONS: {
       bool Complain = (ClientLoadCapabilities & ARR_ConfigurationMismatch) == 0;
       if (ParseLanguageOptions(Record, Complain, Listener,
                                AllowCompatibleConfigurationMismatch))
         Result = ConfigurationMismatch;
       break;
     }
 

 ASTReader::ASTReadResult
 ASTReader::ReadControlBlock(ModuleFile &F,
                             SmallVectorImpl<ImportedModule> &Loaded,
                             const ModuleFile *ImportedBy,
                             unsigned ClientLoadCapabilities) {
   BitstreamCursor &Stream = F.Stream;
   ASTReadResult Result = Success;
 
-  if (Stream.EnterSubBlock(CONTROL_BLOCK_ID)) {
-    Error("malformed block record in AST file");
+  if (llvm::Error Err = Stream.EnterSubBlock(CONTROL_BLOCK_ID)) {
+    Error(std::move(Err));
     return Failure;
   }
 
   // Lambda to read the unhashed control block the first time it's called.
   //
   // For PCM files, the unhashed control block cannot be read until after the
   // MODULE_NAME record.  However, PCH files have no MODULE_NAME, and yet still
   // need to look ahead before reading the IMPORTS record.  For consistency,
ASTReader::ReadControlBlock(ModuleFile &F,
   };
 
   // Read all of the records and blocks in the control block.
   RecordData Record;
   unsigned NumInputs = 0;
   unsigned NumUserInputs = 0;
   StringRef BaseDirectoryAsWritten;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry) {
+      Error(MaybeEntry.takeError());
+      return Failure;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::Error:
       Error("malformed block record in AST file");
       return Failure;
     case llvm::BitstreamEntry::EndBlock: {
       // Validate the module before returning.  This call catches an AST with
       // no module name and no imports.
case llvm::BitstreamEntry::EndBlock: {
 
       return Result;
     }
 
     case llvm::BitstreamEntry::SubBlock:
       switch (Entry.ID) {
       case INPUT_FILES_BLOCK_ID:
         F.InputFilesCursor = Stream;
-        if (Stream.SkipBlock() || // Skip with the main cursor
-            // Read the abbreviations
-            ReadBlockAbbrevs(F.InputFilesCursor, INPUT_FILES_BLOCK_ID)) {
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          Error(std::move(Err));
+          return Failure;
+        }
+        if (ReadBlockAbbrevs(F.InputFilesCursor, INPUT_FILES_BLOCK_ID)) {
           Error("malformed block record in AST file");
           return Failure;
         }
         continue;
 
       case OPTIONS_BLOCK_ID:
         // If we're reading the first module for this group, check its options
         // are compatible with ours. For modules it imports, no further checking
case llvm::BitstreamEntry::SubBlock:
               (AllowConfigurationMismatch && Result == ConfigurationMismatch))
             Result = Success;
 
           // If we can't load the module, exit early since we likely
           // will rebuild the module anyway. The stream may be in the
           // middle of a block.
           if (Result != Success)
             return Result;
-        } else if (Stream.SkipBlock()) {
-          Error("malformed block record in AST file");
+        } else if (llvm::Error Err = Stream.SkipBlock()) {
+          Error(std::move(Err));
           return Failure;
         }
         continue;
 
       default:
-        if (Stream.SkipBlock()) {
-          Error("malformed block record in AST file");
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          Error(std::move(Err));
           return Failure;
         }
         continue;
       }
 
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read and process a record.
     Record.clear();
     StringRef Blob;
-    switch ((ControlRecordTypes)Stream.readRecord(Entry.ID, Record, &Blob)) {
+    Expected<unsigned> MaybeRecordType = Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeRecordType) {
+      Error(MaybeRecordType.takeError());
+      return Failure;
+    }
+    switch ((ControlRecordTypes)MaybeRecordType.get()) {
     case METADATA: {
       if (Record[0] != VERSION_MAJOR && !DisableValidation) {
         if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)
           Diag(Record[0] < VERSION_MAJOR? diag::err_pch_version_too_old
                                         : diag::err_pch_version_too_new);
         return VersionMismatch;
       }
 
while (true) {
     }
   }
 }
 
 ASTReader::ASTReadResult
 ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
   BitstreamCursor &Stream = F.Stream;
 
-  if (Stream.EnterSubBlock(AST_BLOCK_ID)) {
-    Error("malformed block record in AST file");
+  if (llvm::Error Err = Stream.EnterSubBlock(AST_BLOCK_ID)) {
+    Error(std::move(Err));
     return Failure;
   }
 
   // Read all of the records and blocks for the AST file.
   RecordData Record;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry) {
+      Error(MaybeEntry.takeError());
+      return Failure;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::Error:
       Error("error at end of module block in AST file");
       return Failure;
     case llvm::BitstreamEntry::EndBlock:
       // Outside of C++, we do not store a lookup map for the translation unit.
       // Instead, mark it as needing a lookup map to be built if this module
while (true) {
     case llvm::BitstreamEntry::SubBlock:
       switch (Entry.ID) {
       case DECLTYPES_BLOCK_ID:
         // We lazily load the decls block, but we want to set up the
         // DeclsCursor cursor to point into it.  Clone our current bitcode
         // cursor to it, enter the block and read the abbrevs in that block.
         // With the main cursor, we just skip over it.
         F.DeclsCursor = Stream;
-        if (Stream.SkipBlock() ||  // Skip with the main cursor.
-            // Read the abbrevs.
-            ReadBlockAbbrevs(F.DeclsCursor, DECLTYPES_BLOCK_ID)) {
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          Error(std::move(Err));
+          return Failure;
+        }
+        if (ReadBlockAbbrevs(F.DeclsCursor, DECLTYPES_BLOCK_ID)) {
           Error("malformed block record in AST file");
           return Failure;
         }
         break;
 
       case PREPROCESSOR_BLOCK_ID:
         F.MacroCursor = Stream;
         if (!PP.getExternalSource())
           PP.setExternalSource(this);
 
-        if (Stream.SkipBlock() ||
-            ReadBlockAbbrevs(F.MacroCursor, PREPROCESSOR_BLOCK_ID)) {
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          Error(std::move(Err));
+          return Failure;
+        }
+        if (ReadBlockAbbrevs(F.MacroCursor, PREPROCESSOR_BLOCK_ID)) {
           Error("malformed block record in AST file");
           return Failure;
         }
         F.MacroStartOffset = F.MacroCursor.GetCurrentBitNo();
         break;
 
       case PREPROCESSOR_DETAIL_BLOCK_ID:
         F.PreprocessorDetailCursor = Stream;
-        if (Stream.SkipBlock() ||
-            ReadBlockAbbrevs(F.PreprocessorDetailCursor,
-                             PREPROCESSOR_DETAIL_BLOCK_ID)) {
-              Error("malformed preprocessor detail record in AST file");
-              return Failure;
-            }
+
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          Error(std::move(Err));
+          return Failure;
+        }
+        if (ReadBlockAbbrevs(F.PreprocessorDetailCursor, PREPROCESSOR_DETAIL_BLOCK_ID)) {
         F.PreprocessorDetailStartOffset
         = F.PreprocessorDetailCursor.GetCurrentBitNo();
 
         if (!PP.getPreprocessingRecord())
           PP.createPreprocessingRecord();
         if (!PP.getPreprocessingRecord()->getExternalSource())
           PP.getPreprocessingRecord()->SetExternalSource(*this);
         break;
 
       case SOURCE_MANAGER_BLOCK_ID:
         if (ReadSourceManagerBlock(F))
           return Failure;
         break;
 
       case SUBMODULE_BLOCK_ID:
         if (ASTReadResult Result =
                 ReadSubmoduleBlock(F, ClientLoadCapabilities))
           return Result;
         break;
 
       case COMMENTS_BLOCK_ID: {
         BitstreamCursor C = Stream;
-        if (Stream.SkipBlock() ||
-            ReadBlockAbbrevs(C, COMMENTS_BLOCK_ID)) {
+
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          Error(std::move(Err));
+          return Failure;
+        }
+        if (ReadBlockAbbrevs(C, COMMENTS_BLOCK_ID)) {
           Error("malformed comments block in AST file");
           return Failure;
         }
         CommentsCursors.push_back(std::make_pair(C, &F));
         break;
       }
 
       default:
-        if (Stream.SkipBlock()) {
-          Error("malformed block record in AST file");
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          Error(std::move(Err));
           return Failure;
         }
         break;
       }
       continue;
 
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read and process a record.
     Record.clear();
     StringRef Blob;
-    auto RecordType =
-        (ASTRecordTypes)Stream.readRecord(Entry.ID, Record, &Blob);
+    Expected<unsigned> MaybeRecordType = Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeRecordType) {
+      Error(MaybeRecordType.takeError());
+      return Failure;
+    }
+    ASTRecordTypes RecordType = (ASTRecordTypes)MaybeRecordType.get();
 
     // If we're not loading an AST context, we don't care about most records.
     if (!ContextObj) {
       switch (RecordType) {
       case IDENTIFIER_TABLE:
       case IDENTIFIER_OFFSET:
       case INTERESTING_IDENTIFIERS:
       case STATISTICS:
bool ASTReader::loadGlobalIndex() {
   if (TriedLoadingGlobalIndex || !UseGlobalIndex ||
       !PP.getLangOpts().Modules)
     return true;
 
   // Try to load the global index.
   TriedLoadingGlobalIndex = true;
   StringRef ModuleCachePath
     = getPreprocessor().getHeaderSearchInfo().getModuleCachePath();
-  std::pair<GlobalModuleIndex *, GlobalModuleIndex::ErrorCode> Result
-    = GlobalModuleIndex::readIndex(ModuleCachePath);
-  if (!Result.first)
+  std::pair<GlobalModuleIndex *, llvm::Error> Result = GlobalModuleIndex::readIndex(ModuleCachePath);
+  if (llvm::Error Err = std::move(Result.second)) {
+    assert(!Result.first);
+    consumeError(std::move(Err)); // FIXME this drops errors on the floor.
     return true;
+  }
 
   GlobalIndex.reset(Result.first);
   ModuleMgr.setGlobalIndex(GlobalIndex.get());
   return false;
 }
 
 bool ASTReader::isGlobalIndexUnavailable() const {
   return PP.getLangOpts().Modules && UseGlobalIndex &&
static void updateModuleTimestamp(ModuleFile &MF) {
   OS.clear_error(); // Avoid triggering a fatal error.
 }
 
 /// Given a cursor at the start of an AST file, scan ahead and drop the
 /// cursor into the start of the given block ID, returning false on success and
 /// true on failure.
 static bool SkipCursorToBlock(BitstreamCursor &Cursor, unsigned BlockID) {
   while (true) {
-    llvm::BitstreamEntry Entry = Cursor.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Cursor.advance();
+    if (!MaybeEntry) {
+      // FIXME this drops errors on the floor.
+      return true;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::Error:
     case llvm::BitstreamEntry::EndBlock:
       return true;
 
     case llvm::BitstreamEntry::Record:
       // Ignore top-level records.
-      Cursor.skipRecord(Entry.ID);
+      if (Expected<unsigned> Skipped = Cursor.skipRecord(Entry.ID))
         break;
+      else {
+        // FIXME this drops errors on the floor.
+        return true;
+      }
 
     case llvm::BitstreamEntry::SubBlock:
       if (Entry.ID == BlockID) {
-        if (Cursor.EnterSubBlock(BlockID))
+        if (llvm::Error Err = Cursor.EnterSubBlock(BlockID)) {
+          // FIXME this drops the error on the floor.
           return true;
+        }
         // Found it!
         return false;
       }
 
-      if (Cursor.SkipBlock())
+      if (llvm::Error Err = Cursor.SkipBlock()) {
+        // FIXME this drops the error on the floor.
         return true;
       }
     }
   }
+}
 
 ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
                                             ModuleKind Type,
                                             SourceLocation ImportLoc,
                                             unsigned ClientLoadCapabilities,
                                             SmallVectorImpl<ImportedSubmodule> *Imported) {
   llvm::SaveAndRestore<SourceLocation>
     SetCurImportLocRAII(CurrentImportLoc, ImportLoc);
if (PP.getHeaderSearchInfo()
     }
   }
 
   return Success;
 }
 
 static ASTFileSignature readASTFileSignature(StringRef PCH);
 
-/// Whether \p Stream starts with the AST/PCH file magic number 'CPCH'.
-static bool startsWithASTFileMagic(BitstreamCursor &Stream) {
-  return Stream.canSkipToPos(4) &&
-         Stream.Read(8) == 'C' &&
-         Stream.Read(8) == 'P' &&
-         Stream.Read(8) == 'C' &&
-         Stream.Read(8) == 'H';
+/// Whether \p Stream doesn't start with the AST/PCH file magic number 'CPCH'.
+static llvm::Error doesntStartWithASTFileMagic(BitstreamCursor &Stream) {
+  // FIXME checking magic headers is done in other places such as SerializedDiagnosticReader and GlobalModuleIndex, but error handling isn't always done the same. Unify it all with a helper.
+  if (!Stream.canSkipToPos(4))
+    return llvm::createStringError(std::errc::illegal_byte_sequence, "file too small to contain AST file magic");
+  for (unsigned C : {'C','P','C','H'})
+    if (Expected<llvm::SimpleBitstreamCursor::word_t> Res = Stream.Read(8)) {
+      if (Res.get() != C)
+        return llvm::createStringError(std::errc::illegal_byte_sequence, "file doesn't start with AST file magic");
+    } else
+      return Res.takeError();
+  return llvm::Error::success();
 }
 
 static unsigned moduleKindForDiagnostic(ModuleKind Kind) {
   switch (Kind) {
   case MK_PCH:
     return 0; // PCH
   case MK_ImplicitModule:
   case MK_ExplicitModule:
else
       MC.tryToDropPCM(FileName);
   });
   ModuleFile &F = *M;
   BitstreamCursor &Stream = F.Stream;
   Stream = BitstreamCursor(PCHContainerRdr.ExtractPCH(*F.Buffer));
   F.SizeInBits = F.Buffer->getBufferSize() * 8;
 
   // Sniff for the signature.
-  if (!startsWithASTFileMagic(Stream)) {
-    Diag(diag::err_module_file_invalid) << moduleKindForDiagnostic(Type)
-                                        << FileName;
+  if (llvm::Error Err = doesntStartWithASTFileMagic(Stream)) {
+    Diag(diag::err_module_file_invalid) << moduleKindForDiagnostic(Type) << FileName << std::move(Err);
     return Failure;
   }
 
   // This is used for compatibility with older PCH formats.
   bool HaveReadControlBlock = false;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry) {
+      Error(MaybeEntry.takeError());
+      return Failure;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::Error:
     case llvm::BitstreamEntry::Record:
     case llvm::BitstreamEntry::EndBlock:
       Error("invalid record at top-level of AST file");
       return Failure;
 
while (true) {
 
     case UNHASHED_CONTROL_BLOCK_ID:
       // This block is handled using look-ahead during ReadControlBlock.  We
       // shouldn't get here!
       Error("malformed block record in AST file");
       return Failure;
 
     default:
-      if (Stream.SkipBlock()) {
-        Error("malformed block record in AST file");
+      if (llvm::Error Err = Stream.SkipBlock()) {
+        Error(std::move(Err));
         return Failure;
       }
       break;
     }
   }
 
   llvm_unreachable("unexpected break; expected return");
 }

 ASTReader::ASTReadResult ASTReader::readUnhashedControlBlockImpl(
     ModuleFile *F, llvm::StringRef StreamData, unsigned ClientLoadCapabilities,
     bool AllowCompatibleConfigurationMismatch, ASTReaderListener *Listener,
     bool ValidateDiagnosticOptions) {
   // Initialize a stream.
   BitstreamCursor Stream(StreamData);
 
   // Sniff for the signature.
-  if (!startsWithASTFileMagic(Stream))
+  if (llvm::Error Err = doesntStartWithASTFileMagic(Stream)) {
+    // FIXME this drops the error on the floor.
     return Failure;
+  }
 
   // Scan for the UNHASHED_CONTROL_BLOCK_ID block.
   if (SkipCursorToBlock(Stream, UNHASHED_CONTROL_BLOCK_ID))
     return Failure;
 
   // Read all of the records in the options block.
   RecordData Record;
   ASTReadResult Result = Success;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry) {
+      // FIXME this drops the error on the floor.
+      return Failure;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::Error:
     case llvm::BitstreamEntry::SubBlock:
       return Failure;
 
     case llvm::BitstreamEntry::EndBlock:
       return Result;
 
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read and process a record.
     Record.clear();
-    switch (
-        (UnhashedControlBlockRecordTypes)Stream.readRecord(Entry.ID, Record)) {
+    Expected<unsigned> MaybeRecordType = Stream.readRecord(Entry.ID, Record);
+    if (!MaybeRecordType) {
+      // FIXME this drops the error.
+      return Failure;
+    }
+    switch ((UnhashedControlBlockRecordTypes)MaybeRecordType.get()) {
     case SIGNATURE:
       if (F)
         std::copy(Record.begin(), Record.end(), F->Signature.data());
       break;
     case DIAGNOSTIC_OPTIONS: {
       bool Complain = (ClientLoadCapabilities & ARR_OutOfDate) == 0;
       if (Listener && ValidateDiagnosticOptions &&
           !AllowCompatibleConfigurationMismatch &&
static bool parseModuleFileExtensionMetadata(
   return false;
 }
 
 ASTReader::ASTReadResult ASTReader::ReadExtensionBlock(ModuleFile &F) {
   BitstreamCursor &Stream = F.Stream;
 
   RecordData Record;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry) {
+      Error(MaybeEntry.takeError());
+      return Failure;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::SubBlock:
-      if (Stream.SkipBlock())
+      if (llvm::Error Err = Stream.SkipBlock()) {
+        Error(std::move(Err));
         return Failure;
-
+      }
       continue;
 
     case llvm::BitstreamEntry::EndBlock:
       return Success;
 
     case llvm::BitstreamEntry::Error:
       return HadErrors;
 
     case llvm::BitstreamEntry::Record:
       break;
     }
 
     Record.clear();
     StringRef Blob;
-    unsigned RecCode = Stream.readRecord(Entry.ID, Record, &Blob);
-    switch (RecCode) {
+    Expected<unsigned> MaybeRecCode = Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeRecCode) {
+      Error(MaybeRecCode.takeError());
+      return Failure;
+    }
+    switch (MaybeRecCode.get()) {
     case EXTENSION_METADATA: {
       ModuleFileExtensionMetadata Metadata;
       if (parseModuleFileExtensionMetadata(Record, Blob, Metadata))
         return Failure;
 
       // Find a module file extension with this block name.
       auto Known = ModuleFileExtensions.find(Metadata.BlockName);
       if (Known == ModuleFileExtensions.end()) break;

 void ASTReader::finalizeForWriting() {
   // Nothing to do for now.
 }
 
 /// Reads and return the signature record from \p PCH's control block, or
 /// else returns 0.
 static ASTFileSignature readASTFileSignature(StringRef PCH) {
   BitstreamCursor Stream(PCH);
-  if (!startsWithASTFileMagic(Stream))
+  if (llvm::Error Err = doesntStartWithASTFileMagic(Stream)) {
+    // FIXME this drops the error on the floor.
     return ASTFileSignature();
+  }
 
   // Scan for the UNHASHED_CONTROL_BLOCK_ID block.
   if (SkipCursorToBlock(Stream, UNHASHED_CONTROL_BLOCK_ID))
     return ASTFileSignature();
 
   // Scan for SIGNATURE inside the diagnostic options block.
   ASTReader::RecordData Record;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry) {
+      // FIXME this drops the error on the floor.
+      return ASTFileSignature();
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     if (Entry.Kind != llvm::BitstreamEntry::Record)
       return ASTFileSignature();
 
     Record.clear();
     StringRef Blob;
-    if (SIGNATURE == Stream.readRecord(Entry.ID, Record, &Blob))
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeRecord) {
+      // FIXME this drops the error on the floor.
+      return ASTFileSignature();
+    }
+    if (SIGNATURE == MaybeRecord.get())
       return {{{(uint32_t)Record[0], (uint32_t)Record[1], (uint32_t)Record[2],
                 (uint32_t)Record[3], (uint32_t)Record[4]}}};
   }
 }
 
 /// Retrieve the name of the original source file name
 /// directly from the AST file, without actually loading the AST
 /// file.
 std::string ASTReader::getOriginalSourceFile(
     const std::string &ASTFileName, FileManager &FileMgr,
     const PCHContainerReader &PCHContainerRdr, DiagnosticsEngine &Diags) {
   // Open the AST file.
   auto Buffer = FileMgr.getBufferForFile(ASTFileName);
   if (!Buffer) {
     Diags.Report(diag::err_fe_unable_to_read_pch_file)
         << ASTFileName << Buffer.getError().message();
     return std::string();
   }
 
   // Initialize the stream
   BitstreamCursor Stream(PCHContainerRdr.ExtractPCH(**Buffer));
 
   // Sniff for the signature.
-  if (!startsWithASTFileMagic(Stream)) {
-    Diags.Report(diag::err_fe_not_a_pch_file) << ASTFileName;
+  if (llvm::Error Err = doesntStartWithASTFileMagic(Stream)) {
+    Diags.Report(diag::err_fe_not_a_pch_file) << ASTFileName << std::move(Err);
     return std::string();
   }
 
   // Scan for the CONTROL_BLOCK_ID block.
   if (SkipCursorToBlock(Stream, CONTROL_BLOCK_ID)) {
     Diags.Report(diag::err_fe_pch_malformed_block) << ASTFileName;
     return std::string();
   }
 
   // Scan for ORIGINAL_FILE inside the control block.
   RecordData Record;
   while (true) {
-    llvm::BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry) {
+      // FIXME this drops errors on the floor.
+      return std::string();
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     if (Entry.Kind == llvm::BitstreamEntry::EndBlock)
       return std::string();
 
     if (Entry.Kind != llvm::BitstreamEntry::Record) {
       Diags.Report(diag::err_fe_pch_malformed_block) << ASTFileName;
       return std::string();
     }
 
     Record.clear();
     StringRef Blob;
-    if (Stream.readRecord(Entry.ID, Record, &Blob) == ORIGINAL_FILE)
+    Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeRecord) {
+      // FIXME this drops the errors on the floor.
+      return std::string();
+    }
+    if (ORIGINAL_FILE == MaybeRecord.get())
       return Blob.str();
   }
 }
 
 namespace {
 
   class SimplePCHValidator : public ASTReaderListener {
     const LangOptions &ExistingLangOpts;
if (!Buffer) {
     return true;
   }
 
   // Initialize the stream
   StringRef Bytes = PCHContainerRdr.ExtractPCH(**Buffer);
   BitstreamCursor Stream(Bytes);
 
   // Sniff for the signature.
-  if (!startsWithASTFileMagic(Stream))
+  if (llvm::Error Err = doesntStartWithASTFileMagic(Stream)) {
+    consumeError(std::move(Err)); // FIXME this drops errors on the floor.
     return true;
+  }
 
   // Scan for the CONTROL_BLOCK_ID block.
   if (SkipCursorToBlock(Stream, CONTROL_BLOCK_ID))
     return true;
 
   bool NeedsInputFiles = Listener.needsInputFileVisitation();
   bool NeedsSystemInputFiles = Listener.needsSystemInputFileVisitation();
   bool NeedsImports = Listener.needsImportVisitation();
   BitstreamCursor InputFilesCursor;
 
   RecordData Record;
   std::string ModuleDir;
   bool DoneWithControlBlock = false;
   while (!DoneWithControlBlock) {
-    llvm::BitstreamEntry Entry = Stream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry) {
+      // FIXME this drops the error on the floor.
+      return true;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::SubBlock: {
       switch (Entry.ID) {
       case OPTIONS_BLOCK_ID: {
         std::string IgnoredSuggestedPredefines;
         if (ReadOptionsBlock(Stream, ARR_ConfigurationMismatch | ARR_OutOfDate,
                              /*AllowCompatibleConfigurationMismatch*/ false,
                              Listener, IgnoredSuggestedPredefines) != Success)
           return true;
         break;
       }
 
       case INPUT_FILES_BLOCK_ID:
         InputFilesCursor = Stream;
-        if (Stream.SkipBlock() ||
-            (NeedsInputFiles &&
-             ReadBlockAbbrevs(InputFilesCursor, INPUT_FILES_BLOCK_ID)))
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          // FIXME this drops the error on the floor.
+          return true;
+        }
+        if (NeedsInputFiles && ReadBlockAbbrevs(InputFilesCursor, INPUT_FILES_BLOCK_ID))
           return true;
         break;
 
       default:
-        if (Stream.SkipBlock())
+        if (llvm::Error Err = Stream.SkipBlock()) {
+          // FIXME this drops the error on the floor.
           return true;
+        }
         break;
       }
 
       continue;
     }
 
     case llvm::BitstreamEntry::EndBlock:
       DoneWithControlBlock = true;
       break;
 
     case llvm::BitstreamEntry::Error:
       return true;
 
     case llvm::BitstreamEntry::Record:
       break;
     }
 
     if (DoneWithControlBlock) break;
 
     Record.clear();
     StringRef Blob;
-    unsigned RecCode = Stream.readRecord(Entry.ID, Record, &Blob);
-    switch ((ControlRecordTypes)RecCode) {
+    Expected<unsigned> MaybeRecCode = Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeRecCode) {
+      // FIXME this drops the error.
+      return Failure;
+    }
+    switch ((ControlRecordTypes)MaybeRecCode.get()) {
     case METADATA:
       if (Record[0] != VERSION_MAJOR)
         return true;
       if (Listener.ReadFullVersionInformation(Blob))
         return true;
       break;
     case MODULE_NAME:
       Listener.ReadModuleName(Blob);
case INPUT_FILE_OFFSETS: {
         // Go find this input file.
         bool isSystemFile = I >= NumUserFiles;
 
         if (isSystemFile && !NeedsSystemInputFiles)
           break; // the rest are system input files
 
         BitstreamCursor &Cursor = InputFilesCursor;
         SavedStreamPosition SavedPosition(Cursor);
-        Cursor.JumpToBit(InputFileOffs[I]);
+        if (llvm::Error Err = Cursor.JumpToBit(InputFileOffs[I])) {
+          // FIXME this drops errors on the floor.
+        }
+
+        Expected<unsigned> MaybeCode = Cursor.ReadCode();
+        if (!MaybeCode) {
+          // FIXME this drops errors on the floor.
+        }
+        unsigned Code = MaybeCode.get();
 
-        unsigned Code = Cursor.ReadCode();
         RecordData Record;
         StringRef Blob;
         bool shouldContinue = false;
-        switch ((InputFileRecordTypes)Cursor.readRecord(Code, Record, &Blob)) {
+        Expected<unsigned> MaybeRecordType = Cursor.readRecord(Code, Record, &Blob);
+        if (!MaybeRecordType) {
+          // FIXME this drops errors on the floor.
+        }
+        switch ((InputFileRecordTypes)MaybeRecordType.get()) {
         case INPUT_FILE:
           bool Overridden = static_cast<bool>(Record[3]);
           std::string Filename = Blob;
           ResolveImportedPath(Filename, ModuleDir);
           shouldContinue = Listener.visitInputFile(
               Filename, isSystemFile, Overridden, /*IsExplicitModule*/false);
           break;
         }
bool ASTReader::readASTFileControlBlock(
   }
 
   // Look for module file extension blocks, if requested.
   if (FindModuleFileExtensions) {
     BitstreamCursor SavedStream = Stream;
     while (!SkipCursorToBlock(Stream, EXTENSION_BLOCK_ID)) {
       bool DoneWithExtensionBlock = false;
       while (!DoneWithExtensionBlock) {
-       llvm::BitstreamEntry Entry = Stream.advance();
+        Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+        if (!MaybeEntry) {
+          // FIXME this drops the error.
+          return true;
+        }
+        llvm::BitstreamEntry Entry = MaybeEntry.get();
 
        switch (Entry.Kind) {
        case llvm::BitstreamEntry::SubBlock:
-         if (Stream.SkipBlock())
+         if (llvm::Error Err = Stream.SkipBlock()) {
+           // FIXME this drops the error on the floor.
            return true;
-
+         }
          continue;
 
        case llvm::BitstreamEntry::EndBlock:
          DoneWithExtensionBlock = true;
          continue;
 
        case llvm::BitstreamEntry::Error:
          return true;
 
        case llvm::BitstreamEntry::Record:
          break;
        }
 
        Record.clear();
        StringRef Blob;
-       unsigned RecCode = Stream.readRecord(Entry.ID, Record, &Blob);
-       switch (RecCode) {
+       Expected<unsigned> MaybeRecCode = Stream.readRecord(Entry.ID, Record, &Blob);
+       if (!MaybeRecCode) {
+          // FIXME this drops the error.
+         return true;
+       }
+       switch (MaybeRecCode.get()) {
        case EXTENSION_METADATA: {
          ModuleFileExtensionMetadata Metadata;
          if (parseModuleFileExtensionMetadata(Record, Blob, Metadata))
            return true;
 
          Listener.readModuleFileExtension(Metadata);
          break;
        }
return !readASTFileControlBlock(Filename, FileMgr, PCHContainerRdr,
                                   /*FindModuleFileExtensions=*/false,
                                   validator,
                                   /*ValidateDiagnosticOptions=*/true);
 }
 
 ASTReader::ASTReadResult
 ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
   // Enter the submodule block.
-  if (F.Stream.EnterSubBlock(SUBMODULE_BLOCK_ID)) {
-    Error("malformed submodule block record in AST file");
+  if (llvm::Error Err = F.Stream.EnterSubBlock(SUBMODULE_BLOCK_ID)) {
+    Error(std::move(Err));
     return Failure;
   }
 
   ModuleMap &ModMap = PP.getHeaderSearchInfo().getModuleMap();
   bool First = true;
   Module *CurrentModule = nullptr;
   RecordData Record;
   while (true) {
-    llvm::BitstreamEntry Entry = F.Stream.advanceSkippingSubblocks();
+    Expected<llvm::BitstreamEntry> MaybeEntry = F.Stream.advanceSkippingSubblocks();
+    if (!MaybeEntry) {
+      Error(MaybeEntry.takeError());
+      return Failure;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::SubBlock: // Handled for us already.
     case llvm::BitstreamEntry::Error:
       Error("malformed block record in AST file");
       return Failure;
     case llvm::BitstreamEntry::EndBlock:
       return Success;
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record.
     StringRef Blob;
     Record.clear();
-    auto Kind = F.Stream.readRecord(Entry.ID, Record, &Blob);
+    Expected<unsigned> MaybeKind = F.Stream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeKind) {
+      Error(MaybeKind.takeError());
+      return Failure;
+    }
+    unsigned Kind = MaybeKind.get();
 
     if ((Kind == SUBMODULE_METADATA) != First) {
       Error("submodule metadata record should be at beginning of block");
       return Failure;
     }
     First = false;
 
     // Submodule information is only valid if we have a current module.
PreprocessedEntity *ASTReader::ReadPreprocessedEntity(unsigned Index) {
   const PPEntityOffset &PPOffs = M.PreprocessedEntityOffsets[LocalIndex];
 
   if (!PP.getPreprocessingRecord()) {
     Error("no preprocessing record");
     return nullptr;
   }
 
   SavedStreamPosition SavedPosition(M.PreprocessorDetailCursor);
-  M.PreprocessorDetailCursor.JumpToBit(PPOffs.BitOffset);
+  if (llvm::Error Err = M.PreprocessorDetailCursor.JumpToBit(PPOffs.BitOffset)) {
+    Error(std::move(Err));
+    return nullptr;
+  }
+
+  Expected<llvm::BitstreamEntry> MaybeEntry = M.PreprocessorDetailCursor.advance(BitstreamCursor::AF_DontPopBlockAtEnd);
+  if (!MaybeEntry) {
+    Error(MaybeEntry.takeError());
+    return nullptr;
+  }
+  llvm::BitstreamEntry Entry = MaybeEntry.get();
     
-  llvm::BitstreamEntry Entry =
-    M.PreprocessorDetailCursor.advance(BitstreamCursor::AF_DontPopBlockAtEnd);
   if (Entry.Kind != llvm::BitstreamEntry::Record)
     return nullptr;
 
   // Read the record.
   SourceRange Range(TranslateSourceLocation(M, PPOffs.getBegin()),
                     TranslateSourceLocation(M, PPOffs.getEnd()));
   PreprocessingRecord &PPRec = *PP.getPreprocessingRecord();
   StringRef Blob;
   RecordData Record;
-  PreprocessorDetailRecordTypes RecType =
-    (PreprocessorDetailRecordTypes)M.PreprocessorDetailCursor.readRecord(
-                                          Entry.ID, Record, &Blob);
-  switch (RecType) {
+  Expected<unsigned> MaybeRecType = M.PreprocessorDetailCursor.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeRecType) {
+      Error(MaybeRecType.takeError());
+      return nullptr;
+    }
+  switch ((PreprocessorDetailRecordTypes)MaybeRecType.get()) {
   case PPD_MACRO_EXPANSION: {
     bool isBuiltin = Record[0];
     IdentifierInfo *Name = nullptr;
     MacroDefinitionRecord *Def = nullptr;
     if (isBuiltin)
       Name = getLocalIdentifier(M, Record[1]);
     else {
       PreprocessedEntityID GlobalID =
QualType ASTReader::readTypeRecord(unsigned Index) {
   SavedStreamPosition SavedPosition(DeclsCursor);
 
   ReadingKindTracker ReadingKind(Read_Type, *this);
 
   // Note that we are loading a type record.
   Deserializing AType(this);
 
   unsigned Idx = 0;
-  DeclsCursor.JumpToBit(Loc.Offset);
+  if (llvm::Error Err = DeclsCursor.JumpToBit(Loc.Offset)) {
+    Error(std::move(Err));
+    return QualType();
+  }
   RecordData Record;
-  unsigned Code = DeclsCursor.ReadCode();
-  switch ((TypeCode)DeclsCursor.readRecord(Code, Record)) {
+  Expected<unsigned> MaybeCode = DeclsCursor.ReadCode();
+  if (!MaybeCode) {
+    Error(MaybeCode.takeError());
+    return QualType();
+  }
+  unsigned Code = MaybeCode.get();
+
+  Expected<unsigned> MaybeTypeCode = DeclsCursor.readRecord(Code, Record);
+  if (!MaybeTypeCode) {
+    Error(MaybeTypeCode.takeError());
+    return QualType();
+  }
+  switch ((TypeCode)MaybeTypeCode.get()) {
   case TYPE_EXT_QUAL: {
     if (Record.size() != 2) {
       Error("Incorrect encoding of extended qualifier type");
       return QualType();
     }
     QualType Base = readType(*Loc.F, Record, Idx);
     Qualifiers Quals = Qualifiers::fromOpaqueValue(Record[Idx++]);
     return Context.getQualifiedType(Base, Quals);
void ASTReader::CompleteRedeclChain(const Decl *D) {
   }
 }
 
 CXXCtorInitializer **
 ASTReader::GetExternalCXXCtorInitializers(uint64_t Offset) {
   RecordLocation Loc = getLocalBitOffset(Offset);
   BitstreamCursor &Cursor = Loc.F->DeclsCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(Loc.Offset);
+  if (llvm::Error Err = Cursor.JumpToBit(Loc.Offset)) {
+    Error(std::move(Err));
+    return nullptr;
+  }
   ReadingKindTracker ReadingKind(Read_Decl, *this);
 
   RecordData Record;
-  unsigned Code = Cursor.ReadCode();
-  unsigned RecCode = Cursor.readRecord(Code, Record);
-  if (RecCode != DECL_CXX_CTOR_INITIALIZERS) {
+  Expected<unsigned> MaybeCode = Cursor.ReadCode();
+  if (!MaybeCode) {
+    Error(MaybeCode.takeError());
+    return nullptr;
+  }
+  unsigned Code = MaybeCode.get();
+
+  Expected<unsigned> MaybeRecCode = Cursor.readRecord(Code, Record);
+  if (!MaybeRecCode) {
+    Error(MaybeRecCode.takeError());
+    return nullptr;
+  }
+  if (MaybeRecCode.get() != DECL_CXX_CTOR_INITIALIZERS) {
     Error("malformed AST file: missing C++ ctor initializers");
     return nullptr;
   }
 
   unsigned Idx = 0;
   return ReadCXXCtorInitializers(*Loc.F, Record, Idx);
 }
 
 CXXBaseSpecifier *ASTReader::GetExternalCXXBaseSpecifiers(uint64_t Offset) {
   assert(ContextObj && "reading base specifiers with no AST context");
   ASTContext &Context = *ContextObj;
 
   RecordLocation Loc = getLocalBitOffset(Offset);
   BitstreamCursor &Cursor = Loc.F->DeclsCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(Loc.Offset);
+  if (llvm::Error Err = Cursor.JumpToBit(Loc.Offset)) {
+    Error(std::move(Err));
+    return nullptr;
+  }
   ReadingKindTracker ReadingKind(Read_Decl, *this);
   RecordData Record;
-  unsigned Code = Cursor.ReadCode();
-  unsigned RecCode = Cursor.readRecord(Code, Record);
+
+  Expected<unsigned> MaybeCode = Cursor.ReadCode();
+  if (!MaybeCode) {
+    Error(MaybeCode.takeError());
+    return nullptr;
+  }
+  unsigned Code = MaybeCode.get();
+
+  Expected<unsigned> MaybeRecCode = Cursor.readRecord(Code, Record);
+  if (!MaybeRecCode) {
+    Error(MaybeCode.takeError());
+    return nullptr;
+  }
+  unsigned RecCode = MaybeRecCode.get();
+
   if (RecCode != DECL_CXX_BASE_SPECIFIERS) {
     Error("malformed AST file: missing C++ base specifiers");
     return nullptr;
   }
 
   unsigned Idx = 0;
   unsigned NumBases = Record[Idx++];
   void *Mem = Context.Allocate(sizeof(CXXBaseSpecifier) * NumBases);

 /// source each time it is called, and is meant to be used via a
 /// LazyOffsetPtr (which is used by Decls for the body of functions, etc).
 Stmt *ASTReader::GetExternalDeclStmt(uint64_t Offset) {
   // Switch case IDs are per Decl.
   ClearSwitchCaseIDs();
 
   // Offset here is a global offset across the entire chain.
   RecordLocation Loc = getLocalBitOffset(Offset);
-  Loc.F->DeclsCursor.JumpToBit(Loc.Offset);
+  if (llvm::Error Err = Loc.F->DeclsCursor.JumpToBit(Loc.Offset)) {
+    Error(std::move(Err));
+    return nullptr;
+  }
   assert(NumCurrentElementsDeserializing == 0 &&
          "should not be called while already deserializing");
   Deserializing D(this);
   return ReadStmtFromStream(*Loc.F);
 }
 
 void ASTReader::FindExternalLexicalDecls(
     const DeclContext *DC, llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
for (SmallVectorImpl<std::pair<BitstreamCursor,
        I != E; ++I) {
     Comments.clear();
     BitstreamCursor &Cursor = I->first;
     serialization::ModuleFile &F = *I->second;
     SavedStreamPosition SavedPosition(Cursor);
 
     RecordData Record;
     while (true) {
-      llvm::BitstreamEntry Entry =
-        Cursor.advanceSkippingSubblocks(BitstreamCursor::AF_DontPopBlockAtEnd);
+      Expected<llvm::BitstreamEntry> MaybeEntry = Cursor.advanceSkippingSubblocks(BitstreamCursor::AF_DontPopBlockAtEnd);
+      if (!MaybeEntry) {
+        Error(MaybeEntry.takeError());
+        return;
+      }
+      llvm::BitstreamEntry Entry = MaybeEntry.get();
 
       switch (Entry.Kind) {
       case llvm::BitstreamEntry::SubBlock: // Handled for us already.
       case llvm::BitstreamEntry::Error:
         Error("malformed block record in AST file");
         return;
       case llvm::BitstreamEntry::EndBlock:
         goto NextCursor;
       case llvm::BitstreamEntry::Record:
         // The interesting case.
         break;
       }
 
       // Read a record.
       Record.clear();
-      switch ((CommentRecordTypes)Cursor.readRecord(Entry.ID, Record)) {
+      Expected<unsigned> MaybeComment = Cursor.readRecord(Entry.ID, Record);
+      if (!MaybeComment) {
+        Error(MaybeComment.takeError());
+        return;
+      }
+      switch ((CommentRecordTypes)MaybeComment.get()) {
       case COMMENTS_RAW_COMMENT: {
         unsigned Idx = 0;
         SourceRange SR = ReadSourceRange(F, Record, Idx);
         RawComment::CommentKind Kind =
             (RawComment::CommentKind) Record[Idx++];
         bool IsTrailingComment = Record[Idx++];
         bool IsAlmostTrailingComment = Record[Idx++];
         Comments.push_back(new (Context) RawComment(
ASTReader::~ASTReader() {
   if (OwnsDeserializationListener)
     delete DeserializationListener;
 }
 
 IdentifierResolver &ASTReader::getIdResolver() {
   return SemaObj ? SemaObj->IdResolver : DummyIdResolver;
 }
 
-unsigned ASTRecordReader::readRecord(llvm::BitstreamCursor &Cursor,
-                                     unsigned AbbrevID) {
+Expected<unsigned> ASTRecordReader::readRecord(llvm::BitstreamCursor &Cursor, unsigned AbbrevID) {
   Idx = 0;
   Record.clear();
   return Cursor.readRecord(AbbrevID, Record);
 }
 //===----------------------------------------------------------------------===//
 //// OMPClauseReader implementation
 ////===----------------------------------------------------------------------===//
 

clang/lib/Serialization/ASTReaderDecl.cpp

Decl *ASTReader::ReadDeclRecord(DeclID ID) {
   // after reading this declaration.
   SavedStreamPosition SavedPosition(DeclsCursor);
 
   ReadingKindTracker ReadingKind(Read_Decl, *this);
 
   // Note that we are loading a declaration record.
   Deserializing ADecl(this);
 
-  DeclsCursor.JumpToBit(Loc.Offset);
+  auto Fail = [] (const char *what, llvm::Error &&Err) {
+    llvm::report_fatal_error(Twine("ASTReader::ReadDeclRecord failed ") + what + ": " + toString(std::move(Err)));
+  };
+
+  if (llvm::Error JumpFailed = DeclsCursor.JumpToBit(Loc.Offset))
+    Fail("jumping", std::move(JumpFailed));
   ASTRecordReader Record(*this, *Loc.F);
   ASTDeclReader Reader(*this, Record, Loc, ID, DeclLoc);
-  unsigned Code = DeclsCursor.ReadCode();
+  Expected<unsigned> MaybeCode = DeclsCursor.ReadCode();
+  if (!MaybeCode)
+    Fail("reading code", MaybeCode.takeError());
+  unsigned Code = MaybeCode.get();
 
   ASTContext &Context = getContext();
   Decl *D = nullptr;
-  switch ((DeclCode)Record.readRecord(DeclsCursor, Code)) {
+  Expected<unsigned> MaybeDeclCode = Record.readRecord(DeclsCursor, Code);
+  if (!MaybeDeclCode)
+    llvm::report_fatal_error("ASTReader::ReadDeclRecord failed reading decl code: " + toString(MaybeDeclCode.takeError()));
+  switch ((DeclCode)MaybeDeclCode.get()) {
   case DECL_CONTEXT_LEXICAL:
   case DECL_CONTEXT_VISIBLE:
     llvm_unreachable("Record cannot be de-serialized with ReadDeclRecord");
   case DECL_TYPEDEF:
     D = TypedefDecl::CreateDeserialized(Context, ID);
     break;
   case DECL_TYPEALIAS:
     D = TypeAliasDecl::CreateDeserialized(Context, ID);
if (UpdI != DeclUpdateOffsets.end()) {
     // current ASTReader state.
     bool WasInteresting =
         Record.JustLoaded || isConsumerInterestedIn(getContext(), D, false);
     for (auto &FileAndOffset : UpdateOffsets) {
       ModuleFile *F = FileAndOffset.first;
       uint64_t Offset = FileAndOffset.second;
       llvm::BitstreamCursor &Cursor = F->DeclsCursor;
       SavedStreamPosition SavedPosition(Cursor);
-      Cursor.JumpToBit(Offset);
-      unsigned Code = Cursor.ReadCode();
+      if (llvm::Error JumpFailed = Cursor.JumpToBit(Offset))
+        // FIXME don't do a fatal error.
+        llvm::report_fatal_error("ASTReader::loadDeclUpdateRecords failed jumping: " + toString(std::move(JumpFailed)));
+      Expected<unsigned> MaybeCode = Cursor.ReadCode();
+      if (!MaybeCode)
+        llvm::report_fatal_error("ASTReader::loadDeclUpdateRecords failed reading code: " + toString(MaybeCode.takeError()));
+      unsigned Code = MaybeCode.get();
       ASTRecordReader Record(*this, *F);
-      unsigned RecCode = Record.readRecord(Cursor, Code);
-      (void)RecCode;
-      assert(RecCode == DECL_UPDATES && "Expected DECL_UPDATES record!");
+      if (Expected<unsigned> MaybeRecCode = Record.readRecord(Cursor, Code))
+        assert(MaybeRecCode.get() == DECL_UPDATES && "Expected DECL_UPDATES record!");
+      else
+        llvm::report_fatal_error("ASTReader::loadDeclUpdateRecords failed reading rec code: " + toString(MaybeCode.takeError()));
 
       ASTDeclReader Reader(*this, Record, RecordLocation(F, Offset), ID,
                            SourceLocation());
       Reader.UpdateDecl(D, PendingLazySpecializationIDs);
 
       // We might have made this declaration interesting. If so, remember that
       // we need to hand it off to the consumer.
       if (!WasInteresting &&
void ASTReader::loadPendingDeclChain(Decl *FirstLocal, uint64_t LocalOffset) {
   }
 
   // Load the list of other redeclarations from this module file.
   ModuleFile *M = getOwningModuleFile(FirstLocal);
   assert(M && "imported decl from no module file");
 
   llvm::BitstreamCursor &Cursor = M->DeclsCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(LocalOffset);
+  if (llvm::Error JumpFailed = Cursor.JumpToBit(LocalOffset))
+    llvm::report_fatal_error("ASTReader::loadPendingDeclChain failed jumping: " + toString(std::move(JumpFailed)));
 
   RecordData Record;
-  unsigned Code = Cursor.ReadCode();
-  unsigned RecCode = Cursor.readRecord(Code, Record);
-  (void)RecCode;
-  assert(RecCode == LOCAL_REDECLARATIONS && "expected LOCAL_REDECLARATIONS record!");
+  Expected<unsigned> MaybeCode = Cursor.ReadCode();
+  if (!MaybeCode)
+    llvm::report_fatal_error("ASTReader::loadPendingDeclChain failed reading code: " + toString(MaybeCode.takeError()));
+  unsigned Code = MaybeCode.get();
+  if (Expected<unsigned> MaybeRecCode = Cursor.readRecord(Code, Record))
+    assert(MaybeRecCode.get() == LOCAL_REDECLARATIONS && "expected LOCAL_REDECLARATIONS record!");
+  else
+    llvm::report_fatal_error("ASTReader::loadPendingDeclChain failed reading rec code: " + toString(MaybeCode.takeError()));
 
   // FIXME: We have several different dispatches on decl kind here; maybe
   // we should instead generate one loop per kind and dispatch up-front?
   Decl *MostRecent = FirstLocal;
   for (unsigned I = 0, N = Record.size(); I != N; ++I) {
     auto *D = GetLocalDecl(*M, Record[N - I - 1]);
     ASTDeclReader::attachPreviousDecl(*this, D, MostRecent, CanonDecl);
     MostRecent = D;

clang/lib/Serialization/ASTReaderStmt.cpp

#ifndef NDEBUG
   unsigned PrevNumStmts = StmtStack.size();
 #endif
 
   ASTRecordReader Record(*this, F);
   ASTStmtReader Reader(Record, Cursor);
   Stmt::EmptyShell Empty;
 
   while (true) {
-    llvm::BitstreamEntry Entry = Cursor.advanceSkippingSubblocks();
+    llvm::Expected<llvm::BitstreamEntry> MaybeEntry = Cursor.advanceSkippingSubblocks();
+    if (!MaybeEntry) {
+      Error(toString(MaybeEntry.takeError()));
+      return nullptr;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::SubBlock: // Handled for us already.
     case llvm::BitstreamEntry::Error:
       Error("malformed block record in AST file");
       return nullptr;
     case llvm::BitstreamEntry::EndBlock:
       goto Done;
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     ASTContext &Context = getContext();
     Stmt *S = nullptr;
     bool Finished = false;
     bool IsStmtReference = false;
-    switch ((StmtCode)Record.readRecord(Cursor, Entry.ID)) {
+    Expected<unsigned> MaybeStmtCode = Record.readRecord(Cursor, Entry.ID);
+    if (!MaybeStmtCode) {
+      Error(toString(MaybeStmtCode.takeError()));
+      return nullptr;
+    }
+    switch ((StmtCode)MaybeStmtCode.get()) {
     case STMT_STOP:
       Finished = true;
       break;
 
     case STMT_REF_PTR:
       IsStmtReference = true;
       assert(StmtEntries.find(Record[0]) != StmtEntries.end() &&
              "No stmt was recorded for this offset reference!");

clang/lib/Serialization/GlobalModuleIndex.cpp

typedef llvm::OnDiskIterableChainedHashTable<IdentifierIndexReaderTrait>
     IdentifierIndexTable;
 
 }
 
 GlobalModuleIndex::GlobalModuleIndex(std::unique_ptr<llvm::MemoryBuffer> Buffer,
                                      llvm::BitstreamCursor Cursor)
     : Buffer(std::move(Buffer)), IdentifierIndex(), NumIdentifierLookups(),
       NumIdentifierLookupHits() {
+  auto Fail = [&Buffer] (llvm::Error &&Err) {
+    report_fatal_error("Module index '" + Buffer->getBufferIdentifier() + "' failed: " + toString(std::move(Err)));
+  };
+
   llvm::TimeTraceScope TimeScope("Module LoadIndex", StringRef(""));
   // Read the global index.
   bool InGlobalIndexBlock = false;
   bool Done = false;
   while (!Done) {
-    llvm::BitstreamEntry Entry = Cursor.advance();
+    llvm::BitstreamEntry Entry;
+    if (Expected<llvm::BitstreamEntry> Res = Cursor.advance())
+      Entry = Res.get();
+    else
+      Fail(Res.takeError());
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::Error:
       return;
 
     case llvm::BitstreamEntry::EndBlock:
       if (InGlobalIndexBlock) {
         InGlobalIndexBlock = false;
         Done = true;
         continue;
       }
       return;
 
 
     case llvm::BitstreamEntry::Record:
       // Entries in the global index block are handled below.
       if (InGlobalIndexBlock)
         break;
 
       return;
 
     case llvm::BitstreamEntry::SubBlock:
       if (!InGlobalIndexBlock && Entry.ID == GLOBAL_INDEX_BLOCK_ID) {
-        if (Cursor.EnterSubBlock(GLOBAL_INDEX_BLOCK_ID))
-          return;
-
+        if (llvm::Error Err = Cursor.EnterSubBlock(GLOBAL_INDEX_BLOCK_ID))
+          Fail(std::move(Err));
         InGlobalIndexBlock = true;
-      } else if (Cursor.SkipBlock()) {
-        return;
-      }
+      } else if (llvm::Error Err = Cursor.SkipBlock())
+        Fail(std::move(Err));
       continue;
     }
 
     SmallVector<uint64_t, 64> Record;
     StringRef Blob;
-    switch ((IndexRecordTypes)Cursor.readRecord(Entry.ID, Record, &Blob)) {
+    Expected<unsigned> MaybeIndexRecord = Cursor.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeIndexRecord)
+      Fail(MaybeIndexRecord.takeError());
+    IndexRecordTypes IndexRecord = static_cast<IndexRecordTypes>(MaybeIndexRecord.get());
+    switch (IndexRecord) {
     case INDEX_METADATA:
       // Make sure that the version matches.
       if (Record.size() < 1 || Record[0] != CurrentVersion)
         return;
       break;
 
     case MODULE: {
       unsigned Idx = 0;
while (!Done) {
     }
   }
 }
 
 GlobalModuleIndex::~GlobalModuleIndex() {
   delete static_cast<IdentifierIndexTable *>(IdentifierIndex);
 }
 
-std::pair<GlobalModuleIndex *, GlobalModuleIndex::ErrorCode>
+std::pair<GlobalModuleIndex *, llvm::Error>
 GlobalModuleIndex::readIndex(StringRef Path) {
   // Load the index file, if it's there.
   llvm::SmallString<128> IndexPath;
   IndexPath += Path;
   llvm::sys::path::append(IndexPath, IndexFileName);
 
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> BufferOrErr =
       llvm::MemoryBuffer::getFile(IndexPath.c_str());
   if (!BufferOrErr)
-    return std::make_pair(nullptr, EC_NotFound);
+    return std::make_pair(nullptr, llvm::errorCodeToError(BufferOrErr.getError()));
   std::unique_ptr<llvm::MemoryBuffer> Buffer = std::move(BufferOrErr.get());
 
   /// The main bitstream cursor for the main block.
   llvm::BitstreamCursor Cursor(*Buffer);
 
   // Sniff for the signature.
-  if (Cursor.Read(8) != 'B' ||
-      Cursor.Read(8) != 'C' ||
-      Cursor.Read(8) != 'G' ||
-      Cursor.Read(8) != 'I') {
-    return std::make_pair(nullptr, EC_IOError);
+  for (unsigned char C : {'B', 'C', 'G', 'I'}) {
+    if (Expected<llvm::SimpleBitstreamCursor::word_t> Res = Cursor.Read(8)) {
+      if (Res.get() != C)
+        return std::make_pair(nullptr, llvm::createStringError(std::errc::illegal_byte_sequence, "expected signature BCGI"));
+    } else
+      return std::make_pair(nullptr, Res.takeError());
   }
 
-  return std::make_pair(new GlobalModuleIndex(std::move(Buffer), Cursor),
-                        EC_None);
+  return std::make_pair(new GlobalModuleIndex(std::move(Buffer), Cursor), llvm::Error::success());
 }
 
 void
 GlobalModuleIndex::getKnownModules(SmallVectorImpl<ModuleFile *> &ModuleFiles) {
   ModuleFiles.clear();
   for (unsigned I = 0, N = Modules.size(); I != N; ++I) {
     if (ModuleFile *MF = Modules[I].File)
       ModuleFiles.push_back(MF);
class GlobalModuleIndexBuilder {
     }
 
   public:
     explicit GlobalModuleIndexBuilder(
         FileManager &FileMgr, const PCHContainerReader &PCHContainerRdr)
         : FileMgr(FileMgr), PCHContainerRdr(PCHContainerRdr) {}
 
     /// Load the contents of the given module file into the builder.
-    ///
-    /// \returns true if an error occurred, false otherwise.
-    bool loadModuleFile(const FileEntry *File);
+    llvm::Error loadModuleFile(const FileEntry *File);
 
     /// Write the index to the given bitstream.
     /// \returns true if an error occurred, false otherwise.
     bool writeIndex(llvm::BitstreamWriter &Stream);
   };
 }
 
 static void emitBlockID(unsigned ID, const char *Name,
data_type ReadData(const internal_key_type& k,
       using namespace llvm::support;
       unsigned RawID = endian::readNext<uint32_t, little, unaligned>(d);
       bool IsInteresting = RawID & 0x01;
       return std::make_pair(k, IsInteresting);
     }
   };
 }
 
-bool GlobalModuleIndexBuilder::loadModuleFile(const FileEntry *File) {
+llvm::Error GlobalModuleIndexBuilder::loadModuleFile(const FileEntry *File) {
   // Open the module file.
 
   auto Buffer = FileMgr.getBufferForFile(File, /*isVolatile=*/true);
-  if (!Buffer) {
-    return true;
-  }
+  if (!Buffer)
+    return llvm::createStringError(Buffer.getError(), "failed getting buffer for module file");
 
   // Initialize the input stream
   llvm::BitstreamCursor InStream(PCHContainerRdr.ExtractPCH(**Buffer));
 
   // Sniff for the signature.
-  if (InStream.Read(8) != 'C' ||
-      InStream.Read(8) != 'P' ||
-      InStream.Read(8) != 'C' ||
-      InStream.Read(8) != 'H') {
-    return true;
-  }
+  for (unsigned char C : {'C','P','C','H'})
+    if (Expected<llvm::SimpleBitstreamCursor::word_t> Res = InStream.Read(8)) {
+      if (Res.get() != C)
+        return llvm::createStringError(std::errc::illegal_byte_sequence, "expected signature CPCH");
+    } else
+      return Res.takeError();
 
   // Record this module file and assign it a unique ID (if it doesn't have
   // one already).
   unsigned ID = getModuleFileInfo(File).ID;
 
   // Search for the blocks and records we care about.
   enum { Other, ControlBlock, ASTBlock, DiagnosticOptionsBlock } State = Other;
   bool Done = false;
   while (!Done) {
-    llvm::BitstreamEntry Entry = InStream.advance();
+    Expected<llvm::BitstreamEntry> MaybeEntry = InStream.advance();
+    if (!MaybeEntry)
+      return MaybeEntry.takeError();
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::Error:
       Done = true;
       continue;
 
     case llvm::BitstreamEntry::Record:
       // In the 'other' state, just skip the record. We don't care.
       if (State == Other) {
-        InStream.skipRecord(Entry.ID);
+        if (llvm::Expected<unsigned> Skipped = InStream.skipRecord(Entry.ID))
           continue;
+        else
+          return Skipped.takeError();
       }
 
       // Handle potentially-interesting records below.
       break;
 
     case llvm::BitstreamEntry::SubBlock:
       if (Entry.ID == CONTROL_BLOCK_ID) {
-        if (InStream.EnterSubBlock(CONTROL_BLOCK_ID))
-          return true;
+        if (llvm::Error Err = InStream.EnterSubBlock(CONTROL_BLOCK_ID))
+          return Err;
 
         // Found the control block.
         State = ControlBlock;
         continue;
       }
 
       if (Entry.ID == AST_BLOCK_ID) {
-        if (InStream.EnterSubBlock(AST_BLOCK_ID))
-          return true;
+        if (llvm::Error Err = InStream.EnterSubBlock(AST_BLOCK_ID))
+          return Err;
 
         // Found the AST block.
         State = ASTBlock;
         continue;
       }
 
       if (Entry.ID == UNHASHED_CONTROL_BLOCK_ID) {
-        if (InStream.EnterSubBlock(UNHASHED_CONTROL_BLOCK_ID))
-          return true;
+        if (llvm::Error Err = InStream.EnterSubBlock(UNHASHED_CONTROL_BLOCK_ID))
+          return Err;
 
         // Found the Diagnostic Options block.
         State = DiagnosticOptionsBlock;
         continue;
       }
 
-      if (InStream.SkipBlock())
-        return true;
+      if (llvm::Error Err = InStream.SkipBlock())
+        return Err;
 
       continue;
 
     case llvm::BitstreamEntry::EndBlock:
       State = Other;
       continue;
     }
 
     // Read the given record.
     SmallVector<uint64_t, 64> Record;
     StringRef Blob;
-    unsigned Code = InStream.readRecord(Entry.ID, Record, &Blob);
+    Expected<unsigned> MaybeCode = InStream.readRecord(Entry.ID, Record, &Blob);
+    if (!MaybeCode)
+      return MaybeCode.takeError();
+    unsigned Code = MaybeCode.get();
 
     // Handle module dependencies.
     if (State == ControlBlock && Code == IMPORTS) {
       // Load each of the imported PCH files.
       unsigned Idx = 0, N = Record.size();
       while (Idx < N) {
         // Read information about the AST file.
 
if (State == ControlBlock && Code == IMPORTS) {
         Idx += Length;
 
         // Find the imported module file.
         const FileEntry *DependsOnFile
           = FileMgr.getFile(ImportedFile, /*openFile=*/false,
                             /*cacheFailure=*/false);
 
         if (!DependsOnFile)
-          return true;
+          return llvm::createStringError(std::errc::bad_file_descriptor, "imported file \"%s\" not found", ImportedFile.c_str());
 
         // Save the information in ImportedModuleFileInfo so we can verify after
         // loading all pcms.
         ImportedModuleFiles.insert(std::make_pair(
             DependsOnFile, ImportedModuleFileInfo(StoredSize, StoredModTime,
                                                   StoredSignature)));
 
         // Record the dependency.
while (!Done) {
     if (State == DiagnosticOptionsBlock && Code == SIGNATURE)
       getModuleFileInfo(File).Signature = {
           {{(uint32_t)Record[0], (uint32_t)Record[1], (uint32_t)Record[2],
             (uint32_t)Record[3], (uint32_t)Record[4]}}};
 
     // We don't care about this record.
   }
 
-  return false;
+  return llvm::Error::success();
 }
 
 namespace {
 
 /// Trait used to generate the identifier index as an on-disk hash
 /// table.
 class IdentifierIndexWriterTrait {
 public:
// Write the identifier -> module file mapping.
     uint64_t Record[] = {IDENTIFIER_INDEX, BucketOffset};
     Stream.EmitRecordWithBlob(IDTableAbbrev, Record, IdentifierTable);
   }
 
   Stream.ExitBlock();
   return false;
 }
 
-GlobalModuleIndex::ErrorCode
+llvm::Error
 GlobalModuleIndex::writeIndex(FileManager &FileMgr,
                               const PCHContainerReader &PCHContainerRdr,
                               StringRef Path) {
   llvm::SmallString<128> IndexPath;
   IndexPath += Path;
   llvm::sys::path::append(IndexPath, IndexFileName);
 
   // Coordinate building the global index file with other processes that might
   // try to do the same.
   llvm::LockFileManager Locked(IndexPath);
   switch (Locked) {
   case llvm::LockFileManager::LFS_Error:
-    return EC_IOError;
+    return llvm::createStringError(std::errc::io_error, "LFS error");
 
   case llvm::LockFileManager::LFS_Owned:
     // We're responsible for building the index ourselves. Do so below.
     break;
 
   case llvm::LockFileManager::LFS_Shared:
     // Someone else is responsible for building the index. We don't care
     // when they finish, so we're done.
-    return EC_Building;
+    return llvm::createStringError(std::errc::device_or_resource_busy, "someone else is building the index");
   }
 
   // The module index builder.
   GlobalModuleIndexBuilder Builder(FileMgr, PCHContainerRdr);
 
   // Load each of the module files.
   std::error_code EC;
   for (llvm::sys::fs::directory_iterator D(Path, EC), DEnd;
        D != DEnd && !EC;
        D.increment(EC)) {
     // If this isn't a module file, we don't care.
     if (llvm::sys::path::extension(D->path()) != ".pcm") {
       // ... unless it's a .pcm.lock file, which indicates that someone is
       // in the process of rebuilding a module. They'll rebuild the index
       // at the end of that translation unit, so we don't have to.
       if (llvm::sys::path::extension(D->path()) == ".pcm.lock")
-        return EC_Building;
+        return llvm::createStringError(std::errc::device_or_resource_busy, "someone else is building the index");
 
       continue;
     }
 
     // If we can't find the module file, skip it.
     const FileEntry *ModuleFile = FileMgr.getFile(D->path());
     if (!ModuleFile)
       continue;
 
     // Load this module file.
-    if (Builder.loadModuleFile(ModuleFile))
-      return EC_IOError;
+    if (llvm::Error Err = Builder.loadModuleFile(ModuleFile))
+      return Err;
   }
 
   // The output buffer, into which the global index will be written.
   SmallVector<char, 16> OutputBuffer;
   {
     llvm::BitstreamWriter OutputStream(OutputBuffer);
     if (Builder.writeIndex(OutputStream))
-      return EC_IOError;
+      return llvm::createStringError(std::errc::io_error, "failed writing index");
   }
 
   // Write the global index file to a temporary file.
   llvm::SmallString<128> IndexTmpPath;
   int TmpFD;
   if (llvm::sys::fs::createUniqueFile(IndexPath + "-%%%%%%%%", TmpFD,
                                       IndexTmpPath))
-    return EC_IOError;
+    return llvm::createStringError(std::errc::io_error, "failed creating unique file");
 
   // Open the temporary global index file for output.
   llvm::raw_fd_ostream Out(TmpFD, true);
   if (Out.has_error())
-    return EC_IOError;
+    return llvm::createStringError(Out.error(), "failed outputting to stream");
 
   // Write the index.
   Out.write(OutputBuffer.data(), OutputBuffer.size());
   Out.close();
   if (Out.has_error())
-    return EC_IOError;
+    return llvm::createStringError(Out.error(), "failed writing to stream");
 
   // Remove the old index file. It isn't relevant any more.
   llvm::sys::fs::remove(IndexPath);
 
   // Rename the newly-written index file to the proper name.
-  if (llvm::sys::fs::rename(IndexTmpPath, IndexPath)) {
-    // Rename failed; just remove the
+  if (std::error_code Err = llvm::sys::fs::rename(IndexTmpPath, IndexPath)) {
+    // Remove the file on failure, don't check whether removal succeeded.
     llvm::sys::fs::remove(IndexTmpPath);
-    return EC_IOError;
+    return llvm::createStringError(Err, "failed renaming file \"%s\" to \"%s\"", IndexTmpPath.c_str(), IndexPath.c_str());
   }
 
-  // We're done.
-  return EC_None;
+  return llvm::Error::success();
 }
 
 namespace {
   class GlobalIndexIdentifierIterator : public IdentifierIterator {
     /// The current position within the identifier lookup table.
     IdentifierIndexTable::key_iterator Current;
 
     /// The end position within the identifier lookup table.

clang/test/Index/pch-from-libclang.c

 // Check that clang can use a PCH created from libclang.
 
 // This test doesn't use -fdisable-module-hash and hence requires that
 // CompilerInvocation::getModuleHash() computes exactly the same hash
 // for c-index-test and clang, which in turn requires that the both use
 // exactly the same resource-dir, even without calling realpath() on it:
 // - a/../b/ and b/ are not considered the same
 // - on Windows, c:\ and C:\ (only different in case) are not the same
 
+// RUN: rm -rf %t.mcp %t.h.pch
 // RUN: %clang_cc1 -fsyntax-only %s -verify
 // RUN: c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin
 // RUN: %clang -fsyntax-only -include %t.h %s -Xclang -verify -fmodules -fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -Xclang -triple -Xclang x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors
 // RUN: %clang -x c-header %s -o %t.clang.h.pch -fmodules -fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -Xclang -triple -Xclang x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors -Xclang -verify
 // RUN: c-index-test -test-load-source local %s -include %t.clang.h -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin | FileCheck %s
 
 // FIXME: Still fails on at least some linux boxen.
 // REQUIRES: system-darwin

llvm/include/llvm/Bitcode/BitstreamReader.h

 private:
   word_t CurWord = 0;
 
   /// This is the number of bits in CurWord that are valid. This is always from
   /// [0...bits_of(size_t)-1] inclusive.
   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)
       : BitcodeBytes(BitcodeBytes) {}
   explicit SimpleBitstreamCursor(StringRef BitcodeBytes)
       : BitcodeBytes(arrayRefFromStringRef(BitcodeBytes)) {}
   explicit SimpleBitstreamCursor(MemoryBufferRef BitcodeBytes)
       : SimpleBitstreamCursor(BitcodeBytes.getBuffer()) {}
public:
   }
 
   // Return the byte # of the current bit.
   uint64_t getCurrentByteNo() const { return GetCurrentBitNo() / 8; }
 
   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");
 
     // Move the cursor to the right word.
     NextChar = ByteNo;
     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.
   const uint8_t *getPointerToByte(uint64_t ByteNo, uint64_t NumBytes) {
     return BitcodeBytes.data() + ByteNo;
   }
 
   /// Get a pointer into the bitstream at the specified bit offset.
   ///
   /// The bit offset must be on a byte boundary.
   const uint8_t *getPointerToBit(uint64_t BitNo, uint64_t NumBytes) {
     assert(!(BitNo % 8) && "Expected bit on byte boundary");
     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;
     unsigned BytesRead;
     if (BitcodeBytes.size() >= NextChar + sizeof(word_t)) {
       BytesRead = sizeof(word_t);
       CurWord =
           support::endian::read<word_t, support::little, support::unaligned>(
               NextCharPtr);
     } else {
       // Short read.
       BytesRead = BitcodeBytes.size() - NextChar;
       CurWord = 0;
       for (unsigned B = 0; B != BytesRead; ++B)
         CurWord |= uint64_t(NextCharPtr[B]) << (B * 8);
     }
     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 &&
            "Cannot return zero or more than BitsInWord bits!");
 
     static const unsigned Mask = sizeof(word_t) > 4 ? 0x3f : 0x1f;
 
     // If the field is fully contained by CurWord, return it quickly.
     if (BitsInCurWord >= NumBits) {
       word_t R = CurWord & (~word_t(0) >> (BitsInWord - NumBits));
 
       // Use a mask to avoid undefined behavior.
       CurWord >>= (NumBits & Mask);
 
       BitsInCurWord -= NumBits;
       return R;
     }
 
     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));
 
     // Use a mask to avoid undefined behavior.
     CurWord >>= (BitsLeft & Mask);
 
     BitsInCurWord -= BitsLeft;
 
     R |= R2 << (NumBits - BitsLeft);
 
     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;
 
     uint32_t Result = 0;
     unsigned NextBit = 0;
     while (true) {
       Result |= (Piece & ((1U << (NumBits-1))-1)) << NextBit;
 
       if ((Piece & (1U << (NumBits-1))) == 0)
         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);
 
     uint64_t Result = 0;
     unsigned NextBit = 0;
     while (true) {
       Result |= uint64_t(Piece & ((1U << (NumBits-1))-1)) << NextBit;
 
       if ((Piece & (1U << (NumBits-1))) == 0)
         return Result;
 
       NextBit += NumBits-1;
-      Piece = Read(NumBits);
+      MaybeRead = Read(NumBits);
+      if (!MaybeRead)
+        return MaybeRead;
+      Piece = MaybeRead.get();
     }
   }
 
   void SkipToFourByteBoundary() {
     // If word_t is 64-bits and if we've read less than 32 bits, just dump
     // the bits we have up to the next 32-bit boundary.
     if (sizeof(word_t) > 4 &&
         BitsInCurWord >= 32) {
enum {
     AF_DontPopBlockAtEnd = 1,
 
     /// If this flag is used, abbrev entries are returned just like normal
     /// records.
     AF_DontAutoprocessAbbrevs = 2
   };
 
   /// 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())
           return BitstreamEntry::getError();
         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;
       }
 
       return BitstreamEntry::getRecord(Code);
     }
   }
 
   /// 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 (llvm::Error Err = SkipBlock())
+        return std::move(Err);
     }
   }
 
-  unsigned ReadCode() {
+  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() {
+  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);
+  /// Having read the ENTER_SUBBLOCK abbrevid and a BlockID, skip over the body of this block.
+  __attribute__((deprecated("update all uses of this API"))) // FIXME remove this, it's here to help me catch all uses of this API.
+  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 llvm::createStringError(std::errc::illegal_byte_sequence, "can't skip block: already at end of stream");
+    if (!canSkipToPos(SkipTo / 8))
+      return llvm::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.
+  __attribute__((deprecated("update all uses of this API"))) // FIXME remove this, it's here to help me catch all uses of this API.
+  Error EnterSubBlock(unsigned BlockID, unsigned *NumWordsP = nullptr);
 
   bool ReadBlockEnd() {
     if (BlockScope.empty()) return true;
 
     // Block tail:
     //    [END_BLOCK, <align4bytes>]
     SkipToFourByteBoundary();
 
     popBlockScope();
     return false;
public:
   const BitCodeAbbrev *getAbbrev(unsigned AbbrevID) {
     unsigned AbbrevNo = AbbrevID - bitc::FIRST_APPLICATION_ABBREV;
     if (AbbrevNo >= CurAbbrevs.size())
       report_fatal_error("Invalid abbrev number");
     return CurAbbrevs[AbbrevNo].get();
   }
 
   /// 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
   /// abbreviated records.
   void setBlockInfo(BitstreamBlockInfo *BI) { BlockInfo = BI; }
 };
 
 } // end llvm namespace
 
 #endif // LLVM_BITCODE_BITSTREAMREADER_H

llvm/include/llvm/Support/Error.h

 private:
   std::string Msg;
   std::error_code EC;
   const bool PrintMsgOnly = false;
 };
 
 /// Create formatted StringError object.
 template <typename... Ts>
-Error createStringError(std::error_code EC, char const *Fmt,
+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...);
   return make_error<StringError>(Stream.str(), EC);
 }
 
 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
 /// show more detailed information to the user.
 class FileError final : public ErrorInfo<FileError> {
 
   friend Error createFileError(const Twine &, Error);
   friend Error createFileError(const Twine &, size_t, Error);

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

 
 } // end anonymous namespace
 
 static Error error(const Twine &Message) {
   return make_error<StringError>(
       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) {
   const unsigned char *BufPtr = (const unsigned char *)Buffer.getBufferStart();
   const unsigned char *BufEnd = BufPtr + Buffer.getBufferSize();
 
   if (Buffer.getBufferSize() & 3)
     return error("Invalid bitcode signature");
 
   // If we have a wrapper header, parse it and ignore the non-bc file contents.
   // The magic number is 0x0B17C0DE stored in little endian.
   if (isBitcodeWrapper(BufPtr, BufEnd))
     if (SkipBitcodeWrapperHeader(BufPtr, BufEnd, true))
       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);
 }
 
 /// Convert a string from a record into an std::string, return true on failure.
 template <typename StrTy>
 static bool convertToString(ArrayRef<uint64_t> Record, unsigned Idx,
                             StrTy &Result) {
for (auto &F : *M) {
     for (auto &I : instructions(F))
       I.setMetadata(LLVMContext::MD_tbaa, nullptr);
   }
 }
 
 /// 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;
 
   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:
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return ProducerIdentification;
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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]
       convertToString(Record, 0, ProducerIdentification);
       break;
     case bitc::IDENTIFICATION_CODE_EPOCH: { // EPOCH: [epoch#]
       unsigned epoch = (unsigned)Record[0];
       if (epoch != bitc::BITCODE_CURRENT_EPOCH) {
         return error(
           Twine("Incompatible epoch: Bitcode '") + Twine(epoch) +
           "' vs current: '" + Twine(bitc::BITCODE_CURRENT_EPOCH) + "'");
       }
     }
     }
   }
 }
 
 static Expected<std::string> readIdentificationCode(BitstreamCursor &Stream) {
   // We expect a number of well-defined blocks, though we don't necessarily
   // need to understand them all.
   while (true) {
     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:
       return error("Malformed block");
 
     case BitstreamEntry::SubBlock:
       if (Entry.ID == bitc::IDENTIFICATION_BLOCK_ID)
         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);
+      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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return false;
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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]
       std::string S;
       if (convertToString(Record, 0, S))
         return error("Invalid record");
       // Check for the i386 and other (x86_64, ARM) conventions
       if (S.find("__DATA,__objc_catlist") != std::string::npos ||
           S.find("__OBJC,__category") != std::string::npos)
         return true;
       break;
     }
     }
     Record.clear();
   }
   llvm_unreachable("Exit infinite loop");
 }
 
 static Expected<bool> hasObjCCategory(BitstreamCursor &Stream) {
   // 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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return false;
 
     case BitstreamEntry::SubBlock:
       if (Entry.ID == bitc::MODULE_BLOCK_ID)
         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);
+      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;
 
   std::string Triple;
 
   // 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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Triple;
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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;
       if (convertToString(Record, 0, S))
         return error("Invalid record");
       Triple = S;
       break;
     }
     }
     Record.clear();
   }
   llvm_unreachable("Exit infinite loop");
 }
 
 static Expected<std::string> readTriple(BitstreamCursor &Stream) {
   // 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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return "";
 
     case BitstreamEntry::SubBlock:
       if (Entry.ID == bitc::MODULE_BLOCK_ID)
         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);
+      if (llvm::Expected<unsigned> Skipped = Stream.skipRecord(Entry.ID))
         continue;
+      else
+        return Skipped.takeError();
     }
   }
 }
 
 namespace {
 
 class BitcodeReaderBase {
 protected:
static void decodeLLVMAttributesForBitcode(AttrBuilder &B,
 
   if (Alignment)
     B.addAlignmentAttr(Alignment);
   addRawAttributeValue(B, ((EncodedAttrs & (0xfffffULL << 32)) >> 11) |
                           (EncodedAttrs & 0xffff));
 }
 
 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");
 
   SmallVector<uint64_t, 64> Record;
 
   SmallVector<AttributeList, 8> Attrs;
 
   // 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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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, ...]
       // FIXME: Remove in 4.0.
       if (Record.size() & 1)
         return error("Invalid record");
 
       for (unsigned i = 0, e = Record.size(); i != e; i += 2) {

 Error BitcodeReader::parseAttrKind(uint64_t Code, Attribute::AttrKind *Kind) {
   *Kind = getAttrFromCode(Code);
   if (*Kind == Attribute::None)
     return error("Unknown attribute kind (" + Twine(Code) + ")");
   return Error::success();
 }
 
 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");
 
   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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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, ...]
       if (Record.size() < 3)
         return error("Invalid record");
 
       uint64_t GrpID = Record[0];
       uint64_t Idx = Record[1]; // Index of the object this attribute refers to.
case bitc::PARAMATTR_GRP_CODE_ENTRY: { // ENTRY: [grpid, idx, a0, a1, ...]
       MAttributeGroups[GrpID] = AttributeList::get(Context, Idx, B);
       break;
     }
     }
   }
 }
 
 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();
 }
 
 Error BitcodeReader::parseTypeTableBody() {
   if (!TypeList.empty())
     return error("Invalid multiple blocks");
 
   SmallVector<uint64_t, 64> Record;
   unsigned NumRecords = 0;
 
   SmallString<64> TypeName;
 
   // 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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       if (NumRecords != TypeList.size())
         return error("Malformed block");
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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]
       // TYPE_CODE_NUMENTRY contains a count of the number of types in the
       // type list.  This allows us to reserve space.
       if (Record.size() < 1)
         return error("Invalid record");
       TypeList.resize(Record[0]);
if (TypeList[NumRecords])
       return error(
           "Invalid TYPE table: Only named structs can be forward referenced");
     assert(ResultTy && "Didn't read a type?");
     TypeList[NumRecords++] = ResultTy;
   }
 }
 
 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");
 
   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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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]
     BundleTags.emplace_back();
     if (convertToString(Record, 0, BundleTags.back()))
       return error("Invalid record");
     Record.clear();
   }
 }
 
 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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       if (SSIDs.empty())
         return error("Invalid empty synchronization scope names block");
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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;
     if (convertToString(Record, 0, SSN))
       return error("Invalid record");
 
     SSIDs.push_back(Context.getOrInsertSyncScopeID(SSN));
     Record.clear();
if (GO->getComdat() == reinterpret_cast<Comdat *>(1)) {
         GO->setComdat(nullptr);
     }
   }
   return V;
 }
 
 /// 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;
 }
 
 void BitcodeReader::setDeferredFunctionInfo(unsigned FuncBitcodeOffsetDelta,
                                             Function *F,
                                             ArrayRef<uint64_t> Record) {
   // Note that we subtract 1 here because the offset is relative to one word
   // before the start of the identification or module block, which was
   // historically always the start of the regular bitcode header.
   uint64_t FuncWordOffset = Record[1] - 1;
   uint64_t FuncBitOffset = FuncWordOffset * 32;
   DeferredFunctionInfo[F] = FuncBitOffset + FuncBitcodeOffsetDelta;
   // Set the LastFunctionBlockBit to point to the last function block.
   // Later when parsing is resumed after function materialization,
   // we can simply skip that last function block.
   if (FuncBitOffset > LastFunctionBlockBit)
     LastFunctionBlockBit = FuncBitOffset;
 }
 
 /// Read a new-style GlobalValue symbol table.
 Error BitcodeReader::parseGlobalValueSymbolTable() {
   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:
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
     case BitstreamEntry::Record:
       break;
     }
 
     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);
       break;
     }
   }
 }
 
 /// Parse the value symbol table at either the current parsing location or
 /// at the given bit offset if provided.
 Error BitcodeReader::parseValueSymbolTable(uint64_t Offset) {
   uint64_t CurrentBit;
   // Pass in the Offset to distinguish between calling for the module-level
   // 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,
     // and will contain symbol names.
   }
 
   // Compute the delta between the bitcode indices in the VST (the word offset
   // to the word-aligned ENTER_SUBBLOCK for the function block, and that
   // expected by the lazy reader. The reader's EnterSubBlock expects to have
   // already read the ENTER_SUBBLOCK code (size getAbbrevIDWidth) and BlockID
   // (size BlockIDWidth). Note that we access the stream's AbbrevID width here
   // just before entering the VST subblock because: 1) the EnterSubBlock
   // changes the AbbrevID width; 2) the VST block is nested within the same
   // outer MODULE_BLOCK as the FUNCTION_BLOCKs and therefore have the same
   // AbbrevID width before calling EnterSubBlock; and 3) when we want to
   // jump to the FUNCTION_BLOCK using this offset later, we don't want
   // to rely on the stream's AbbrevID width being that of the MODULE_BLOCK.
   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;
 
   Triple TT(TheModule->getTargetTriple());
 
   // Read all the records for this value table.
   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.
     case BitstreamEntry::Error:
       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.
       break;
     }
 
     // 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]
       Expected<Value *> ValOrErr = recordValue(Record, 1, TT);
       if (Error Err = ValOrErr.takeError())
         return Err;
       ValOrErr.get();
       break;
static APInt readWideAPInt(ArrayRef<uint64_t> Vals, unsigned TypeBits) {
   SmallVector<uint64_t, 8> Words(Vals.size());
   transform(Vals, Words.begin(),
                  BitcodeReader::decodeSignRotatedValue);
 
   return APInt(TypeBits, Words);
 }
 
 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;
 
   // Read all the records for this value table.
   Type *CurTy = Type::getInt32Ty(Context);
   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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       if (NextCstNo != ValueList.size())
         return error("Invalid constant reference");
 
       // Once all the constants have been read, go through and resolve forward
       // references.
       ValueList.resolveConstantForwardRefs();
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record.
     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);
       break;
     case bitc::CST_CODE_SETTYPE:   // SETTYPE: [typeid]
       if (Record.empty())
         return error("Invalid record");
       if (Record[0] >= TypeList.size() || !TypeList[Record[0]])
while (true) {
     }
 
     ValueList.assignValue(V, NextCstNo);
     ++NextCstNo;
   }
 }
 
 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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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:
       IsBB = true;
       LLVM_FALLTHROUGH;
     case bitc::USELIST_CODE_DEFAULT: {
       unsigned RecordLength = Record.size();
       if (RecordLength < 3)

 /// When we see the block for metadata, remember where it is and then skip it.
 /// This lets us lazily deserialize the metadata.
 Error BitcodeReader::rememberAndSkipMetadata() {
   // Save the current stream state.
   uint64_t CurBit = Stream.GetCurrentBitNo();
   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;
   }
 
   // Upgrade "Linker Options" module flag to "llvm.linker.options" module-level
   // metadata.
   if (Metadata *Val = TheModule->getModuleFlag("Linker Options")) {
     NamedMDNode *LinkerOpts =
Error BitcodeReader::rememberAndSkipFunctionBody() {
   // Save the current stream state.
   uint64_t CurBit = Stream.GetCurrentBitNo();
   assert(
       (DeferredFunctionInfo[Fn] == 0 || DeferredFunctionInfo[Fn] == CurBit) &&
       "Mismatch between VST and scanned function offsets");
   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();
 }
 
 Error BitcodeReader::globalCleanup() {
   // Patch the initializers for globals and aliases up.
   if (Error Err = resolveGlobalAndIndirectSymbolInits())
     return Err;
   if (!GlobalInits.empty() || !IndirectSymbolInits.empty())
Error BitcodeReader::globalCleanup() {
   return Error::success();
 }
 
 /// Support for lazy parsing of function bodies. This is required if we
 /// either have an old bitcode file without a VST forward declaration record,
 /// 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");
 
   if (!SeenFirstFunctionBody)
     return error("Trying to materialize functions before seeing function blocks");
 
   // An old bitcode file with the symbol table at the end would have
   // finished the parse greedily.
   assert(SeenValueSymbolTable);
 
   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");
     case BitstreamEntry::SubBlock:
       switch (Entry.ID) {
       default:
         return error("Expect function block");
       case bitc::FUNCTION_BLOCK_ID:
         if (Error Err = rememberAndSkipFunctionBody())
           return Err;
         NextUnreadBit = Stream.GetCurrentBitNo();
         return Error::success();
       }
     }
   }
 }
 
 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);
   return false;
 }
 
 Error BitcodeReader::parseComdatRecord(ArrayRef<uint64_t> Record) {
   // v1: [selection_kind, name]
Error BitcodeReader::parseGlobalIndirectSymbolRecord(
 
   ValueList.push_back(NewGA);
   IndirectSymbolInits.push_back(std::make_pair(NewGA, Val));
   return Error::success();
 }
 
 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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return globalCleanup();
 
     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())
           return error("Malformed block");
         break;
       case bitc::PARAMATTR_BLOCK_ID:
         if (Error Err = parseAttributeBlock())
           return Err;
case BitstreamEntry::SubBlock:
           assert(VSTOffset == 0 || FunctionsWithBodies.empty());
           if (Error Err = parseValueSymbolTable())
             return Err;
           SeenValueSymbolTable = true;
         } else {
           // 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:
         if (Error Err = parseConstants())
           return Err;
         if (Error Err = resolveGlobalAndIndirectSymbolInits())
           return Err;
         break;
case BitstreamEntry::SubBlock:
             // need to fall back to the lazy parse to find its offset.
           } else {
             // If we have a VST forward declaration record, but have already
             // parsed the VST (just above, when the first function body was
             // encountered here), then we are resuming the parse after
             // 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;
           }
         }
 
         // Support older bitcode files that did not have the function
         // index in the VST, nor a VST forward declaration record, as
         // well as anonymous functions that do not have VST entries.
         // Build the DeferredFunctionInfo vector on the fly.
case BitstreamEntry::SubBlock:
       continue;
 
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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);
       if (!VersionOrErr)
         return VersionOrErr.takeError();
       UseRelativeIDs = *VersionOrErr >= 1;
       break;
     }
if (CB->paramHasAttr(i, Attribute::ByVal) &&
                  Context,
                  CB->getArgOperand(i)->getType()->getPointerElementType()));
     }
   }
 }
 
 /// 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())
     return error("Invalid function metadata: incoming forward references");
 
   InstructionList.clear();
   unsigned ModuleValueListSize = ValueList.size();
   unsigned ModuleMDLoaderSize = MDLoader->size();
Error BitcodeReader::parseFunctionBody(Function *F) {
   };
 
   std::vector<OperandBundleDef> OperandBundles;
 
   // Read all the records.
   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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       goto OutOfRecordLoop;
 
     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())
           return Err;
         NextValueNo = ValueList.size();
         break;
       case bitc::VALUE_SYMTAB_BLOCK_ID:
         if (Error Err = parseValueSymbolTable())
while (true) {
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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]
       if (Record.size() < 1 || Record[0] == 0)
         return error("Invalid record");
       // Create all the basic blocks for the function.
       FunctionBBs.resize(Record[0]);
 
if (DFII->second == 0)
     if (Error Err = findFunctionInStream(F, DFII))
       return Err;
 
   // Materialize metadata before parsing any function bodies.
   if (Error Err = materializeMetadata())
     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);
 
   if (StripDebugInfo)
     stripDebugInfo(*F);
 
   // Upgrade any old intrinsic calls in the function.

 Error ModuleSummaryIndexBitcodeReader::parseValueSymbolTable(
     uint64_t Offset,
     DenseMap<unsigned, GlobalValue::LinkageTypes> &ValueIdToLinkageMap) {
   // With a strtab the VST is not required to parse the summary.
   if (UseStrtab)
     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;
 
   // Read all the records for this value table.
   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.
     case BitstreamEntry::Error:
       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.
       break;
     }
 
     // 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]
       if (convertToString(Record, 1, ValueName))
         return error("Invalid record");
       unsigned ValueID = Record[0];
       assert(!SourceFileName.empty());
       auto VLI = ValueIdToLinkageMap.find(ValueID);
while (true) {
     }
   }
 }
 
 // Parse just the blocks needed for building the index out of the module.
 // 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;
   unsigned ValueId = 0;
 
   // 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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
 
     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)
         if (readBlockInfo())
           return error("Malformed block");
         break;
       case bitc::VALUE_SYMTAB_BLOCK_ID:
         // Should have been parsed earlier via VSTOffset, unless there
         // is no summary section.
         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:
         // Add the module if it is a per-module index (has a source file name).
         if (!SourceFileName.empty())
           addThisModule();
         assert(!SeenValueSymbolTable &&
                "Already read VST when parsing summary block?");
case BitstreamEntry::SubBlock:
         if (Error Err = parseModuleStringTable())
           return Err;
         break;
       }
       continue;
 
     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: {
           if (Error Err = parseVersionRecord(Record).takeError())
             return Err;
           break;
         }
         /// MODULE_CODE_SOURCE_FILENAME: [namechar x N]
static void setImmutableRefs(std::vector<ValueInfo> &Refs, unsigned Count) {
   // Read-only refs are in the end of the refs list.
   for (unsigned RefNo = Refs.size() - Count; RefNo < Refs.size(); ++RefNo)
     Refs[RefNo].setReadOnly();
 }
 
 // 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];
   const bool IsOldProfileFormat = Version == 1;
   if (Version < 1 || Version > 6)
     return error("Invalid summary version " + Twine(Version) +
                  ". Version should be in the range [1-6].");
   Record.clear();
 
   // Keep around the last seen summary to be used when we see an optional
   // "OriginalName" attachement.
   GlobalValueSummary *LastSeenSummary = nullptr;
   GlobalValue::GUID LastSeenGUID = 0;
 
   // We can expect to see any number of type ID information records before
   // each function summary records; these variables store the information
   // collected so far so that it can be used to create the summary object.
   std::vector<GlobalValue::GUID> PendingTypeTests;
   std::vector<FunctionSummary::VFuncId> PendingTypeTestAssumeVCalls,
       PendingTypeCheckedLoadVCalls;
   std::vector<FunctionSummary::ConstVCall> PendingTypeTestAssumeConstVCalls,
       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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record. The record format depends on whether this
     // is a per-module index or a combined index file. In the per-module
     // case the records contain the associated value's ID for correlation
     // with VST entries. In the combined index the correlation is done
     // via the bitcode offset of the summary records (which were saved
     // 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]
       uint64_t Flags = Record[0];
       // Scan flags.
       assert(Flags <= 0x1f && "Unexpected bits in flag");
 
       // 1 bit: WithGlobalValueDeadStripping flag.
while (true) {
     }
   }
   llvm_unreachable("Exit infinite loop");
 }
 
 // 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;
 
   SmallString<128> ModulePath;
   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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     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: {
       // MST_ENTRY: [modid, namechar x N]
       uint64_t ModuleId = Record[0];
 
       if (convertToString(Record, 1, ModulePath))
         return error("Invalid record");

 static ManagedStatic<BitcodeErrorCategoryType> ErrorCategory;
 
 const std::error_category &llvm::BitcodeErrorCategory() {
   return *ErrorCategory;
 }
 
 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;
 
     case BitstreamEntry::Error:
       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;
     }
   }
 }
 
 //===----------------------------------------------------------------------===//
 // External interface
while (true) {
     uint64_t BCBegin = Stream.getCurrentByteNo();
 
     // We may be consuming bitcode from a client that leaves garbage at the end
     // of the bitcode stream (e.g. Apple's ar tool). If we are close enough to
     // the end that there cannot possibly be another module, stop looking.
     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:
       return error("Malformed block");
 
     case BitstreamEntry::SubBlock: {
       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");
       }
 
       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),
                           Buffer.getBufferIdentifier(), IdentificationBit,
                           ModuleBit});
         continue;
       }
 
case BitstreamEntry::SubBlock: {
         // low level function. The client is expected to notice that the number
         // of modules in the symbol table does not match the number of modules
         // in the input file and regenerate the symbol table.
         if (F.Symtab.empty())
           F.Symtab = *SymtabOrErr;
         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);
+      if (Expected<unsigned> StreamFailed = Stream.skipRecord(Entry.ID))
         continue;
+      else
+        return StreamFailed.takeError();
     }
   }
 }
 
 /// Get a lazy one-at-time loading module from bitcode.
 ///
 /// This isn't always used in a lazy context.  In particular, it's also used by
 /// \a parseModule().  If this is truly lazy, then we need to eagerly pull
 /// in forward-referenced functions from block address references.
 ///
 /// \param[in] MaterializeAll Set to \c true if we should materialize
 /// everything.
 Expected<std::unique_ptr<Module>>
 BitcodeModule::getModuleImpl(LLVMContext &Context, bool MaterializeAll,
                              bool ShouldLazyLoadMetadata, bool IsImporting) {
   BitstreamCursor Stream(Buffer);
 
   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)
       return ProducerIdentificationOrErr.takeError();
 
     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);
 
   std::unique_ptr<Module> M =
       llvm::make_unique<Module>(ModuleIdentifier, Context);
   M->setMaterializer(R);
 
   // Delay parsing Metadata if ShouldLazyLoadMetadata is true.
-  if (Error Err =
-          R->parseBitcodeInto(M.get(), ShouldLazyLoadMetadata, IsImporting))
+  if (Error Err = R->parseBitcodeInto(M.get(), ShouldLazyLoadMetadata, IsImporting))
     return std::move(Err);
 
   if (MaterializeAll) {
     // Read in the entire module, and destroy the BitcodeReader.
     if (Error Err = M->materializeAll())
       return std::move(Err);
   } else {
     // Resolve forward references from blockaddresses.

 
 // Parse the specified bitcode buffer and merge the index into CombinedIndex.
 // We don't use ModuleIdentifier here because the client may need to control the
 // module path used in the combined summary (e.g. when reading summaries for
 // regular LTO modules).
 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);
   return R.parseModule();
 }
 
 // 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,
                                     ModuleIdentifier, 0);
 
   if (Error Err = R.parseModule())
     return std::move(Err);
 
   return std::move(Index);
 }
 
 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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       // If no flags record found, conservatively return true to mimic
       // behavior before this flag was added.
       return true;
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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]
       uint64_t Flags = Record[0];
       // Scan flags.
       assert(Flags <= 0x1f && "Unexpected bits in flag");
 
       return Flags & 0x8;
     }
     }
   }
   llvm_unreachable("Exit infinite loop");
 }
 
 // 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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return BitcodeLTOInfo{/*IsThinLTO=*/false, /*HasSummary=*/false,
                             /*EnableSplitLTOUnit=*/false};
 
case BitstreamEntry::SubBlock:
             getEnableSplitLTOUnitFlag(Stream, Entry.ID);
         if (!EnableSplitLTOUnit)
           return EnableSplitLTOUnit.takeError();
         return BitcodeLTOInfo{/*IsThinLTO=*/false, /*HasSummary=*/true,
                               *EnableSplitLTOUnit};
       }
 
       // 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);
+      if (Expected<unsigned> StreamFailed = Stream.skipRecord(Entry.ID))
         continue;
+      else
+        return StreamFailed.takeError();
     }
   }
 }
 
 static Expected<BitcodeModule> getSingleModule(MemoryBufferRef Buffer) {
   Expected<std::vector<BitcodeModule>> MsOrErr = getBitcodeModuleList(Buffer);
   if (!MsOrErr)
     return MsOrErr.takeError();

llvm/lib/Bitcode/Reader/BitstreamReader.cpp

 #include <string>
 
 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);
 
   // Add the abbrevs specific to this block to the CurAbbrevs list.
   if (BlockInfo) {
     if (const BitstreamBlockInfo::BlockInfo *Info =
             BlockInfo->getBlockInfo(BlockID)) {
       CurAbbrevs.insert(CurAbbrevs.end(), Info->Abbrevs.begin(),
                         Info->Abbrevs.end());
     }
   }
 
   // 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.
   switch (Op.getEncoding()) {
   case BitCodeAbbrevOp::Array:
   case BitCodeAbbrevOp::Blob:
     llvm_unreachable("Should not reach here");
   case BitCodeAbbrevOp::Fixed:
     assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
     return Cursor.Read((unsigned)Op.getEncodingData());
   case BitCodeAbbrevOp::VBR:
     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.
   switch (Op.getEncoding()) {
   case BitCodeAbbrevOp::Array:
   case BitCodeAbbrevOp::Blob:
     llvm_unreachable("Should not reach here");
   case BitCodeAbbrevOp::Fixed:
     assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
-    Cursor.Read((unsigned)Op.getEncodingData());
+    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());
+    if (Expected<uint64_t> Res = Cursor.ReadVBR64((unsigned)Op.getEncodingData()))
       break;
+    else
+      return Res.takeError();
   case BitCodeAbbrevOp::Char6:
-    Cursor.Read(6);
+    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;
   }
 
   const BitCodeAbbrev *Abbv = getAbbrev(AbbrevID);
   const BitCodeAbbrevOp &CodeOp = Abbv->getOperandInfo(0);
   unsigned Code;
   if (CodeOp.isLiteral())
     Code = CodeOp.getLiteralValue();
   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);
+    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) {
     const BitCodeAbbrevOp &Op = Abbv->getOperandInfo(i);
     if (Op.isLiteral())
       continue;
 
     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?");
       const BitCodeAbbrevOp &EltEnc = Abbv->getOperandInfo(++i);
 
       // Read all the elements.
       // Decode the value as we are commanded.
       switch (EltEnc.getEncoding()) {
       default:
         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;
     }
 
     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.
     size_t NewEnd = GetCurrentBitNo()+((NumElts+3)&~3)*8;
 
     // If this would read off the end of the bitcode file, just set the
     // record to empty and return.
     if (!canSkipToPos(NewEnd/8)) {
       skipToEnd();
       break;
     }
 
     // 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;
   }
 
   const BitCodeAbbrev *Abbv = getAbbrev(AbbrevID);
 
   // Read the record code first.
   assert(Abbv->getNumOperandInfos() != 0 && "no record code in abbreviation?");
   const BitCodeAbbrevOp &CodeOp = Abbv->getOperandInfo(0);
   unsigned Code;
   if (CodeOp.isLiteral())
     Code = CodeOp.getLiteralValue();
   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);
+    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) {
     const BitCodeAbbrevOp &Op = Abbv->getOperandInfo(i);
     if (Op.isLiteral()) {
       Vals.push_back(Op.getLiteralValue());
       continue;
     }
 
     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)
         report_fatal_error("Array op not second to last");
       const BitCodeAbbrevOp &EltEnc = Abbv->getOperandInfo(++i);
       if (!EltEnc.isEncoding())
         report_fatal_error(
             "Array element type has to be an encoding of a type");
 
       // Read all the elements.
       switch (EltEnc.getEncoding()) {
       default:
         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.
     size_t CurBitPos = GetCurrentBitNo();
     size_t NewEnd = CurBitPos+((NumElts+3)&~3)*8;
 
     // If this would read off the end of the bitcode file, just set the
     // record to empty and return.
     if (!canSkipToPos(NewEnd/8)) {
       Vals.append(NumElts, 0);
       skipToEnd();
       break;
     }
 
     // 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.
     if (Blob) {
       *Blob = StringRef(Ptr, NumElts);
     } else {
       // Otherwise, unpack into Vals with zero extension.
       for (; NumElts; --NumElts)
         Vals.push_back((unsigned char)*Ptr++);
     }
   }
 
   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
       // a slow path in Read() to have to handle reading zero bits.
       if ((E == BitCodeAbbrevOp::Fixed || E == BitCodeAbbrevOp::VBR) &&
           Data == 0) {
         Abbv->Add(BitCodeAbbrevOp(0));
         continue;
       }
 
       if ((E == BitCodeAbbrevOp::Fixed || E == BitCodeAbbrevOp::VBR) &&
           Data > MaxChunkSize)
         report_fatal_error(
             "Fixed or VBR abbrev record with size > MaxChunkData");
 
       Abbv->Add(BitCodeAbbrevOp(E, Data));
     } else
       Abbv->Add(BitCodeAbbrevOp(E));
   }
 
   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;
 
   SmallVector<uint64_t, 64> Record;
   BitstreamBlockInfo::BlockInfo *CurBlockInfo = nullptr;
 
   // 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.
     case llvm::BitstreamEntry::Error:
       return None;
     case llvm::BitstreamEntry::EndBlock:
       return std::move(NewBlockInfo);
     case llvm::BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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.
       CurBlockInfo->Abbrevs.push_back(std::move(CurAbbrevs.back()));
       CurAbbrevs.pop_back();
       continue;
     }
 
     // Read a record.
     Record.clear();
-    switch (readRecord(Entry.ID, Record)) {
+    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)

llvm/lib/Bitcode/Reader/MetadataLoader.cpp

 };
 
 Expected<bool>
 MetadataLoader::MetadataLoaderImpl::lazyLoadModuleMetadataBlock() {
   IndexCursor = Stream;
   SmallVector<uint64_t, 64> Record;
   // Get the abbrevs, and preload record positions to make them lazy-loadable.
   while (true) {
-    BitstreamEntry Entry = IndexCursor.advanceSkippingSubblocks(
-        BitstreamCursor::AF_DontPopBlockAtEnd);
+    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:
       return error("Malformed block");
     case BitstreamEntry::EndBlock: {
       return true;
     }
     case BitstreamEntry::Record: {
       // 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) {
           MDStringRef.push_back(Str);
         };
         if (auto Err = parseMetadataStrings(Record, Blob, IndexNextMDString))
           return std::move(Err);
         break;
       }
       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());
         for (auto &Elt : Record) {
           CurrentValue += Elt;
           GlobalMetadataBitPosIndex.push_back(CurrentValue);
         }
         break;
       }
       case bitc::METADATA_INDEX:
         // We don't expect to get there, the Index is loaded when we encounter
         // the offset.
         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);
+
+        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();
         NamedMDNode *NMD = TheModule.getOrInsertNamedMetadata(Name);
         for (unsigned i = 0; i != Size; ++i) {
           // FIXME: We could use a placeholder here, however NamedMDNode are
           // taking MDNode as operand and not using the Metadata infrastructure.
           // It is acknowledged by 'TODO: Inherit from Metadata' in the
           // NamedMDNode class definition.
           MDNode *MD = MetadataList.getMDNodeFwdRefOrNull(Record[i]);
           assert(MD && "Invalid metadata: expect fwd ref to MDNode");
           NMD->addOperand(MD);
         }
         break;
       }
       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];
         if (ValueID >= ValueList.size())
           return error("Invalid record");
         if (auto *GO = dyn_cast<GlobalObject>(ValueList[ValueID]))
           if (Error Err = parseGlobalObjectAttachment(
                   *GO, ArrayRef<uint64_t>(Record).slice(1)))

 Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
   if (!ModuleLevel && MetadataList.hasFwdRefs())
     return error("Invalid metadata: fwd refs into function blocks");
 
   // Record the entry position so that we can jump back here and efficiently
   // 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;
 
   // We lazy-load module-level metadata: we build an index for each record, and
   // then load individual record as needed, starting with the named metadata.
   if (ModuleLevel && IsImporting && MetadataList.empty() &&
       !DisableLazyLoading) {
     auto SuccessOrErr = lazyLoadModuleMetadataBlock();
     if (!SuccessOrErr)
       return SuccessOrErr.takeError();
     if (SuccessOrErr.get()) {
       // An index was successfully created and we will be able to load metadata
       // on-demand.
       MetadataList.resize(MDStringRef.size() +
                           GlobalMetadataBitPosIndex.size());
 
       // Reading the named metadata created forward references and/or
       // placeholders, that we flush here.
       resolveForwardRefsAndPlaceholders(Placeholders);
       upgradeDebugInfo();
       // 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 (Error Err = IndexCursor.JumpToBit(EntryPos))
+          return Err;
       if (Stream.SkipBlock())
         return error("Invalid record");
       return Error::success();
     }
     // Couldn't load an index, fallback to loading all the block "old-style".
   }
 
   unsigned NextMetadataNo = MetadataList.size();
 
   // 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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       resolveForwardRefsAndPlaceholders(Placeholders);
       upgradeDebugInfo();
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record.
     Record.clear();
     StringRef Blob;
     ++NumMDRecordLoaded;
-    unsigned Code = Stream.readRecord(Entry.ID, Record, &Blob);
-    if (Error Err =
-            parseOneMetadata(Record, Code, Placeholders, Blob, NextMetadataNo))
+    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();
   }
 }
 
 MDString *MetadataLoader::MetadataLoaderImpl::lazyLoadOneMDString(unsigned ID) {
   ++NumMDStringLoaded;
   if (Metadata *MD = MetadataList.lookup(ID))
     return cast<MDString>(MD);
   auto MDS = MDString::get(Context, MDStringRef[ID]);
   MetadataList.assignValue(MDS, ID);
   return MDS;
 }
 
 void MetadataLoader::MetadataLoaderImpl::lazyLoadOneMetadata(
     unsigned ID, PlaceholderQueue &Placeholders) {
   assert(ID < (MDStringRef.size()) + GlobalMetadataBitPosIndex.size());
   assert(ID >= MDStringRef.size() && "Unexpected lazy-loading of MDString");
   // Lookup first if the metadata hasn't already been loaded.
   if (auto *MD = MetadataList.lookup(ID)) {
     auto *N = dyn_cast_or_null<MDNode>(MD);
     if (!N->isTemporary())
       return;
   }
   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");
+  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.
 /// Iteratively lazy-loading metadata on-demand if needed.
 void MetadataLoader::MetadataLoaderImpl::resolveForwardRefsAndPlaceholders(
     PlaceholderQueue &Placeholders) {
   DenseSet<unsigned> Temporaries;
   while (1) {
#define GET_OR_DISTINCT(CLASS, ARGS)                                           \
 
   switch (Code) {
   default: // Default behavior: ignore.
     break;
   case bitc::METADATA_NAME: {
     // 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)
+    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();
     NamedMDNode *NMD = TheModule.getOrInsertNamedMetadata(Name);
     for (unsigned i = 0; i != Size; ++i) {
       MDNode *MD = MetadataList.getMDNodeFwdRefOrNull(Record[i]);
       if (!MD)
         return error("Invalid named metadata: expect fwd ref to MDNode");
Error MetadataLoader::MetadataLoaderImpl::parseMetadataStrings(
   StringRef Lengths = Blob.slice(0, StringsOffset);
   SimpleBitstreamCursor R(Lengths);
 
   StringRef Strings = Blob.drop_front(StringsOffset);
   do {
     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");
 
     CallBack(Strings.slice(0, Size));
     Strings = Strings.drop_front(Size);
   } while (--NumStrings);
 
   return Error::success();
for (unsigned I = 0, E = Record.size(); I != E; I += 2) {
     GO.addMetadata(K->second, *MD);
   }
   return Error::success();
 }
 
 /// 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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       resolveForwardRefsAndPlaceholders(Placeholders);
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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: {
       unsigned RecordLength = Record.size();
       if (Record.empty())
         return error("Invalid record");
       if (RecordLength % 2 == 0) {
         // A function attachment.
Error MetadataLoader::MetadataLoaderImpl::parseMetadataKindRecord(
   unsigned NewKind = TheModule.getMDKindID(Name.str());
   if (!MDKindMap.insert(std::make_pair(Kind, NewKind)).second)
     return error("Conflicting METADATA_KIND records");
   return Error::success();
 }
 
 /// 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.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // 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: {
       if (Error Err = parseMetadataKindRecord(Record))
         return Err;
       break;
     }
     }

llvm/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

PerBlockIDStats()
     : NumInstances(0), NumBits(0),
       NumSubBlocks(0), NumAbbrevs(0), NumRecords(0), NumAbbreviatedRecords(0) {}
 };
 
 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) {
   if (Blob.empty())
     return true;
 
   if (Record.size() != 2)
     return true;
 
   unsigned NumStrings = Record[0];
   unsigned StringsOffset = Record[1];
   outs() << " num-strings = " << NumStrings << " {\n";
 
   StringRef Lengths = Blob.slice(0, StringsOffset);
   SimpleBitstreamCursor R(Lengths);
   StringRef Strings = Blob.drop_front(StringsOffset);
   do {
     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");
 
     outs() << Indent << "    '";
     outs().write_escaped(Strings.slice(0, Size), /*hex=*/true);
     outs() << "'\n";
     Strings = Strings.drop_front(Size);
   } while (--NumStrings);
static bool ParseBlock(BitstreamCursor &Stream, BitstreamBlockInfo &BlockInfo,
   PerBlockIDStats &BlockStats = BlockIDStats[BlockID];
 
   BlockStats.NumInstances++;
 
   // BLOCKINFO is a special part of the stream.
   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();
 
   const char *BlockName = nullptr;
   if (DumpRecords) {
     outs() << Indent << "<";
     if ((BlockName = GetBlockName(BlockID, BlockInfo, CurStreamType)))
static bool ParseBlock(BitstreamCursor &Stream, BitstreamBlockInfo &BlockInfo,
 
   // Read all the records for this block.
   while (1) {
     if (Stream.AtEndOfStream())
       return ReportError("Premature end of bitstream");
 
     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");
     case BitstreamEntry::EndBlock: {
       uint64_t BlockBitEnd = Stream.GetCurrentBitNo();
       BlockStats.NumBits += BlockBitEnd-BlockBitStart;
       if (DumpRecords) {
         outs() << Indent << "</";
case BitstreamEntry::SubBlock: {
       continue;
     }
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     if (Entry.ID == bitc::DEFINE_ABBREV) {
-      Stream.ReadAbbrevRecord();
+      if (Error Err = Stream.ReadAbbrevRecord())
+        return ReportError(std::move(Err));
       ++BlockStats.NumAbbrevs;
       continue;
     }
     
     Record.clear();
 
     ++BlockStats.NumRecords;
 
     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)
       BlockStats.CodeFreq.resize(Code+1);
     BlockStats.CodeFreq[Code].NumInstances++;
     BlockStats.CodeFreq[Code].TotalBits +=
       Stream.GetCurrentBitNo()-RecordStartBit;
     if (Entry.ID != bitc::UNABBREV_RECORD) {
if (DumpRecords) {
             outs() << "unprintable, " << Blob.size() << " bytes.";          
         }
       }
 
       outs() << "\n";
     }
 
     // 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());
   }
 }
 
 static void PrintSize(double Bits) {
   outs() << format("%.2f/%.2fB/%luW", Bits, Bits/8,(unsigned long)(Bits/32));
 }
 static void PrintSize(uint64_t Bits) {
   outs() << format("%lub/%.2fB/%luW", (unsigned long)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)
       return LLVMIRBitstream;
   }
   return UnknownBitstream;
 }
 
if (Dump) {
              << " CPUType=" << format_hex(CPUType, 10) << "/>\n";
     }
 
     if (SkipBitcodeWrapperHeader(BufPtr, EndBufPtr, true))
       return ReportError("Invalid bitcode wrapper header");
   }
 
   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;
 }
 
 /// AnalyzeBitcode - Analyze the bitcode file specified by InputFilename.
 static int AnalyzeBitcode() {
   std::unique_ptr<MemoryBuffer> StreamBuffer;
   BitstreamCursor Stream;
if (!BlockInfoFilename.empty()) {
     std::unique_ptr<MemoryBuffer> BlockInfoBuffer;
     BitstreamCursor BlockInfoCursor;
     CurStreamTypeType BlockInfoStreamType;
     if (openBitcodeFile(BlockInfoFilename, BlockInfoBuffer, BlockInfoCursor,
                         BlockInfoStreamType))
       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 =
-            BlockInfoCursor.ReadBlockInfoBlock(/*ReadBlockInfoNames=*/true);
+      Expected<unsigned> MaybeBlockID = BlockInfoCursor.ReadSubBlockID();
+      if (!MaybeBlockID)
+        return ReportError(MaybeBlockID.takeError());
+      if (MaybeBlockID.get() == bitc::BLOCKINFO_BLOCK_ID) {
+        Expected<Optional<BitstreamBlockInfo>> MaybeNewBlockInfo =
+        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));
     }
   }
 
   unsigned NumTopBlocks = 0;
 
   // 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;
   }
 
   if (Dump) outs() << "\n\n";
 
   uint64_t BufferSizeBits = Stream.getBitcodeBytes().size() * CHAR_BIT;
   // Print a summary of the read file.

llvm/unittests/Bitcode/BitstreamReaderTest.cpp

 
 TEST(BitstreamReaderTest, AtEndOfStream) {
   uint8_t Bytes[4] = {
     0x00, 0x01, 0x02, 0x03
   };
   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());
 }
 
 TEST(BitstreamReaderTest, AtEndOfStreamJump) {
   uint8_t Bytes[4] = {
     0x00, 0x01, 0x02, 0x03
   };
   BitstreamCursor Cursor(Bytes);
 
-  Cursor.JumpToBit(32);
+  EXPECT_FALSE(Cursor.JumpToBit(32));
   EXPECT_TRUE(Cursor.AtEndOfStream());
 }
 
 TEST(BitstreamReaderTest, AtEndOfStreamEmpty) {
   BitstreamCursor Cursor(ArrayRef<uint8_t>{});
 
   EXPECT_TRUE(Cursor.AtEndOfStream());
 }
 
 TEST(BitstreamReaderTest, getCurrentByteNo) {
   uint8_t Bytes[] = {0x00, 0x01, 0x02, 0x03};
   SimpleBitstreamCursor Cursor(Bytes);
 
   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());
 }
 
 TEST(BitstreamReaderTest, getPointerToByte) {
   uint8_t Bytes[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07};
   SimpleBitstreamCursor Cursor(Bytes);
 
for (unsigned I = 0, BlobSize = 0, E = BlobData.size(); BlobSize < E;
     }
 
     // Stream the buffer into the reader.
     BitstreamCursor Stream(
         ArrayRef<uint8_t>((const uint8_t *)Buffer.begin(), Buffer.size()));
 
     // 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 =
-        Stream.advance(BitstreamCursor::AF_DontAutoprocessAbbrevs);
+    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);
   }
 }
 
 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());
   }
 }
 
 static_assert(is_trivially_copyable<BitCodeAbbrevOp>::value,
               "trivially copyable");
 
 } // end anonymous namespace