Author: jfb Date: Wed Jun 26 12:50:12 2019 New Revision: 364464 URL: http://llvm.org/viewvc/llvm-project?rev=364464&view=rev Log: BitStream reader: propagate errors
The bitstream reader handles errors poorly. This has two effects: * Bugs in file handling (especially modules) manifest as an "unexpected end of file" crash * Users of clang as a library end up aborting because the code unconditionally calls `report_fatal_error` The bitstream reader should be more resilient and return Expected / Error as soon as an error is encountered, not way late like it does now. This patch starts doing so and adopting the error handling where I think it makes sense. There's plenty more to do: this patch propagates errors to be minimally useful, and follow-ups will propagate them further and improve diagnostics. https://bugs.llvm.org/show_bug.cgi?id=42311 <rdar://problem/33159405> Differential Revision: https://reviews.llvm.org/D63518 Modified: cfe/trunk/include/clang/Basic/Diagnostic.h cfe/trunk/include/clang/Frontend/FrontendAction.h cfe/trunk/include/clang/Serialization/ASTReader.h cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h cfe/trunk/lib/Frontend/ASTUnit.cpp cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/lib/Frontend/FrontendAction.cpp cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp cfe/trunk/lib/Frontend/TestModuleFileExtension.cpp cfe/trunk/lib/Serialization/ASTReader.cpp cfe/trunk/lib/Serialization/ASTReaderDecl.cpp cfe/trunk/lib/Serialization/ASTReaderStmt.cpp cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp cfe/trunk/test/Index/pch-from-libclang.c Modified: cfe/trunk/include/clang/Basic/Diagnostic.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/Diagnostic.h (original) +++ cfe/trunk/include/clang/Basic/Diagnostic.h Wed Jun 26 12:50:12 2019 @@ -25,6 +25,7 @@ #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> @@ -1303,6 +1304,12 @@ inline DiagnosticBuilder DiagnosticsEngi 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); } Modified: cfe/trunk/include/clang/Frontend/FrontendAction.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendAction.h?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/include/clang/Frontend/FrontendAction.h (original) +++ cfe/trunk/include/clang/Frontend/FrontendAction.h Wed Jun 26 12:50:12 2019 @@ -23,6 +23,7 @@ #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> @@ -229,7 +230,7 @@ public: bool BeginSourceFile(CompilerInstance &CI, const FrontendInputFile &Input); /// Set the source manager's main input file, and run the action. - bool Execute(); + llvm::Error Execute(); /// Perform any per-file post processing, deallocate per-file /// objects, and run statistics and output file cleanup code. Modified: cfe/trunk/include/clang/Serialization/ASTReader.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/include/clang/Serialization/ASTReader.h (original) +++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Jun 26 12:50:12 2019 @@ -1437,6 +1437,7 @@ private: 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 @@ -2379,7 +2380,8 @@ public: /// 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(); } @@ -2679,7 +2681,10 @@ struct SavedStreamPosition { : 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: Modified: cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h (original) +++ cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h Wed Jun 26 12:50:12 2019 @@ -20,6 +20,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" #include <memory> #include <utility> @@ -122,27 +123,14 @@ class GlobalModuleIndex { 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> + /// the error. + static std::pair<GlobalModuleIndex *, llvm::Error> readIndex(llvm::StringRef Path); /// Returns an iterator for identifiers stored in the index table. @@ -194,9 +182,9 @@ public: /// 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); }; } Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/ASTUnit.cpp (original) +++ cfe/trunk/lib/Frontend/ASTUnit.cpp Wed Jun 26 12:50:12 2019 @@ -1206,8 +1206,10 @@ bool ASTUnit::Parse(std::shared_ptr<PCHC 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); @@ -1632,7 +1634,8 @@ ASTUnit *ASTUnit::LoadFromCompilerInvoca 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); @@ -2280,7 +2283,9 @@ void ASTUnit::CodeComplete( 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(); } } Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Jun 26 12:50:12 2019 @@ -941,7 +941,9 @@ bool CompilerInstance::ExecuteAction(Fro 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(); } } @@ -2043,9 +2045,16 @@ GlobalModuleIndex *CompilerInstance::loa 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). It should handle the + // error. + consumeError(std::move(Err)); + return nullptr; + } ModuleManager->resetForReload(); ModuleManager->loadGlobalIndex(); GlobalIndex = ModuleManager->getGlobalIndex(); @@ -2070,9 +2079,13 @@ GlobalModuleIndex *CompilerInstance::loa } } 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. + consumeError(std::move(Err)); + return nullptr; + } ModuleManager->resetForReload(); ModuleManager->loadGlobalIndex(); GlobalIndex = ModuleManager->getGlobalIndex(); Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/FrontendAction.cpp (original) +++ cfe/trunk/lib/Frontend/FrontendAction.cpp Wed Jun 26 12:50:12 2019 @@ -924,7 +924,7 @@ failure: return false; } -bool FrontendAction::Execute() { +llvm::Error FrontendAction::Execute() { CompilerInstance &CI = getCompilerInstance(); if (CI.hasFrontendTimer()) { @@ -939,12 +939,18 @@ bool FrontendAction::Execute() { 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() { Modified: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp (original) +++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp Wed Jun 26 12:50:12 2019 @@ -352,7 +352,8 @@ llvm::ErrorOr<PrecompiledPreamble> Preco 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); Modified: cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp (original) +++ cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp Wed Jun 26 12:50:12 2019 @@ -129,7 +129,11 @@ bool FixItRecompile::BeginInvocation(Com FixItOpts->FixOnlyWarnings = FEOpts.FixOnlyWarnings; FixItRewriter Rewriter(CI.getDiagnostics(), CI.getSourceManager(), CI.getLangOpts(), FixItOpts.get()); - FixAction->Execute(); + if (llvm::Error Err = FixAction->Execute()) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); + return false; + } err = Rewriter.WriteFixedFiles(&RewrittenFiles); Modified: cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp (original) +++ cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp Wed Jun 26 12:50:12 2019 @@ -41,21 +41,47 @@ std::error_code SerializedDiagnosticRead 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 { + // FIXME this drops the error on the floor. + consumeError(Res.takeError()); + } 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 { + // FIXME this drops the error on the floor. + consumeError(Res.takeError()); return SDError::InvalidDiagnostics; + } std::error_code EC; - switch (Stream.ReadSubBlockID()) { - case llvm::bitc::BLOCKINFO_BLOCK_ID: - BlockInfo = Stream.ReadBlockInfoBlock(); + Expected<unsigned> MaybeSubBlockID = Stream.ReadSubBlockID(); + if (!MaybeSubBlockID) { + // FIXME this drops the error on the floor. + consumeError(MaybeSubBlockID.takeError()); + return SDError::InvalidDiagnostics; + } + + switch (MaybeSubBlockID.get()) { + case llvm::bitc::BLOCKINFO_BLOCK_ID: { + Expected<Optional<llvm::BitstreamBlockInfo>> MaybeBlockInfo = + Stream.ReadBlockInfoBlock(); + if (!MaybeBlockInfo) { + // FIXME this drops the error on the floor. + consumeError(MaybeBlockInfo.takeError()); + return SDError::InvalidDiagnostics; + } + BlockInfo = std::move(MaybeBlockInfo.get()); + } if (!BlockInfo) return SDError::MalformedBlockInfoBlock; Stream.setBlockInfo(&*BlockInfo); @@ -69,8 +95,11 @@ std::error_code SerializedDiagnosticRead return EC; continue; default: - if (!Stream.SkipBlock()) + if (llvm::Error Err = Stream.SkipBlock()) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); return SDError::MalformedTopLevelBlock; + } continue; } } @@ -89,11 +118,18 @@ SerializedDiagnosticReader::skipUntilRec 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: @@ -102,7 +138,8 @@ SerializedDiagnosticReader::skipUntilRec 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: @@ -120,8 +157,12 @@ SerializedDiagnosticReader::skipUntilRec std::error_code SerializedDiagnosticReader::readMetaBlock(llvm::BitstreamCursor &Stream) { - if (Stream.EnterSubBlock(clang::serialized_diags::BLOCK_META)) + if (llvm::Error Err = + Stream.EnterSubBlock(clang::serialized_diags::BLOCK_META)) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); return SDError::MalformedMetadataBlock; + } bool VersionChecked = false; @@ -135,8 +176,11 @@ SerializedDiagnosticReader::readMetaBloc case Cursor::Record: break; case Cursor::BlockBegin: - if (Stream.SkipBlock()) + if (llvm::Error Err = Stream.SkipBlock()) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); return SDError::MalformedMetadataBlock; + } LLVM_FALLTHROUGH; case Cursor::BlockEnd: if (!VersionChecked) @@ -145,7 +189,10 @@ SerializedDiagnosticReader::readMetaBloc } 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) @@ -159,8 +206,12 @@ SerializedDiagnosticReader::readMetaBloc std::error_code SerializedDiagnosticReader::readDiagnosticBlock(llvm::BitstreamCursor &Stream) { - if (Stream.EnterSubBlock(clang::serialized_diags::BLOCK_DIAG)) + if (llvm::Error Err = + Stream.EnterSubBlock(clang::serialized_diags::BLOCK_DIAG)) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); return SDError::MalformedDiagnosticBlock; + } std::error_code EC; if ((EC = visitStartOfDiagnostic())) @@ -179,8 +230,11 @@ SerializedDiagnosticReader::readDiagnost if (BlockOrCode == serialized_diags::BLOCK_DIAG) { if ((EC = readDiagnosticBlock(Stream))) return EC; - } else if (!Stream.SkipBlock()) + } else if (llvm::Error Err = Stream.SkipBlock()) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); return SDError::MalformedSubBlock; + } continue; case Cursor::BlockEnd: if ((EC = visitEndOfDiagnostic())) @@ -193,7 +247,11 @@ SerializedDiagnosticReader::readDiagnost // 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) Modified: cfe/trunk/lib/Frontend/TestModuleFileExtension.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TestModuleFileExtension.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/TestModuleFileExtension.cpp (original) +++ cfe/trunk/lib/Frontend/TestModuleFileExtension.cpp Wed Jun 26 12:50:12 2019 @@ -48,7 +48,12 @@ TestModuleFileExtension::Reader::Reader( // 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: @@ -61,8 +66,12 @@ TestModuleFileExtension::Reader::Reader( 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", Modified: cfe/trunk/lib/Serialization/ASTReader.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Jun 26 12:50:12 2019 @@ -1148,12 +1148,26 @@ bool ASTReader::ReadLexicalDeclContextSt 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; @@ -1184,12 +1198,26 @@ bool ASTReader::ReadVisibleDeclContextSt 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; @@ -1219,6 +1247,10 @@ void ASTReader::Error(unsigned DiagID, Diag(DiagID) << Arg1 << Arg2; } +void ASTReader::Error(llvm::Error &&Err) const { + Error(toString(std::move(Err))); +} + //===----------------------------------------------------------------------===// // Source Manager Deserialization //===----------------------------------------------------------------------===// @@ -1282,20 +1314,27 @@ bool ASTReader::ReadSourceManagerBlock(M 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. @@ -1312,7 +1351,13 @@ bool ASTReader::ReadSourceManagerBlock(M // 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; @@ -1376,8 +1421,20 @@ bool ASTReader::ReadSLocEntry(int ID) { 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()) { @@ -1401,12 +1458,23 @@ bool ASTReader::ReadSLocEntry(int ID) { }; 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; @@ -1414,7 +1482,13 @@ bool ASTReader::ReadSLocEntry(int ID) { 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; @@ -1538,23 +1612,40 @@ SourceLocation ASTReader::getImportLocat 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. + consumeError(std::move(Err)); 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. + consumeError(MaybeCode.takeError()); + 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. + consumeError(std::move(Err)); + return true; + } return false; } - Cursor.ReadAbbrevRecord(); + if (llvm::Error Err = Cursor.ReadAbbrevRecord()) { + // FIXME this drops errors on the floor. + consumeError(std::move(Err)); + return true; + } } } @@ -1578,7 +1669,11 @@ MacroInfo *ASTReader::ReadMacroRecord(Mo // after reading this macro. SavedStreamPosition SavedPosition(Stream); - Stream.JumpToBit(Offset); + if (llvm::Error Err = Stream.JumpToBit(Offset)) { + // FIXME this drops errors on the floor. + consumeError(std::move(Err)); + return nullptr; + } RecordData Record; SmallVector<IdentifierInfo*, 16> MacroParams; MacroInfo *Macro = nullptr; @@ -1588,7 +1683,13 @@ MacroInfo *ASTReader::ReadMacroRecord(Mo // 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. @@ -1604,8 +1705,13 @@ MacroInfo *ASTReader::ReadMacroRecord(Mo // 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: @@ -1828,11 +1934,19 @@ void ASTReader::ReadDefinedMacros() { 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. @@ -1842,9 +1956,14 @@ void ASTReader::ReadDefinedMacros() { 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; @@ -1862,6 +1981,7 @@ void ASTReader::ReadDefinedMacros() { } break; } + } } NextCursor: ; } @@ -1962,7 +2082,10 @@ void ASTReader::resolvePendingMacro(Iden 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; @@ -1976,15 +2099,26 @@ void ASTReader::resolvePendingMacro(Iden // 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; @@ -2070,16 +2204,27 @@ ASTReader::readInputFileInfo(ModuleFile // 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. + consumeError(std::move(Err)); + } - unsigned Code = Cursor.ReadCode(); + Expected<unsigned> MaybeCode = Cursor.ReadCode(); + if (!MaybeCode) { + // FIXME this drops errors on the floor. + consumeError(MaybeCode.takeError()); + } + 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. + consumeError(Maybe.takeError()); + } assert(Record[0] == ID && "Bogus stored ID or offset"); InputFileInfo R; @@ -2109,7 +2254,10 @@ InputFile ASTReader::getInputFile(Module // 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. + consumeError(std::move(Err)); + } InputFileInfo FI = readInputFileInfo(F, ID); off_t StoredSize = FI.StoredSize; @@ -2258,14 +2406,23 @@ ASTReader::ASTReadResult ASTReader::Read 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. + consumeError(std::move(Err)); 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. + consumeError(MaybeEntry.takeError()); + return Failure; + } + llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::Error: @@ -2282,7 +2439,13 @@ ASTReader::ASTReadResult ASTReader::Read // 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. + consumeError(MaybeRecordType.takeError()); + return Failure; + } + switch ((OptionsRecordTypes)MaybeRecordType.get()) { case LANGUAGE_OPTIONS: { bool Complain = (ClientLoadCapabilities & ARR_ConfigurationMismatch) == 0; if (ParseLanguageOptions(Record, Complain, Listener, @@ -2334,8 +2497,8 @@ ASTReader::ReadControlBlock(ModuleFile & 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; } @@ -2362,7 +2525,12 @@ ASTReader::ReadControlBlock(ModuleFile & 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: @@ -2425,9 +2593,11 @@ ASTReader::ReadControlBlock(ModuleFile & 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; } @@ -2463,15 +2633,15 @@ ASTReader::ReadControlBlock(ModuleFile & // 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; @@ -2485,7 +2655,13 @@ ASTReader::ReadControlBlock(ModuleFile & // 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) @@ -2682,15 +2858,20 @@ 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: @@ -2717,9 +2898,11 @@ ASTReader::ReadASTBlock(ModuleFile &F, u // 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; } @@ -2730,8 +2913,11 @@ ASTReader::ReadASTBlock(ModuleFile &F, u 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; } @@ -2740,12 +2926,16 @@ ASTReader::ReadASTBlock(ModuleFile &F, u case PREPROCESSOR_DETAIL_BLOCK_ID: F.PreprocessorDetailCursor = Stream; - if (Stream.SkipBlock() || - ReadBlockAbbrevs(F.PreprocessorDetailCursor, + + if (llvm::Error Err = Stream.SkipBlock()) { + Error(std::move(Err)); + return Failure; + } + if (ReadBlockAbbrevs(F.PreprocessorDetailCursor, PREPROCESSOR_DETAIL_BLOCK_ID)) { - Error("malformed preprocessor detail record in AST file"); - return Failure; - } + Error("malformed preprocessor detail record in AST file"); + return Failure; + } F.PreprocessorDetailStartOffset = F.PreprocessorDetailCursor.GetCurrentBitNo(); @@ -2768,8 +2958,12 @@ ASTReader::ReadASTBlock(ModuleFile &F, u 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; } @@ -2778,8 +2972,8 @@ ASTReader::ReadASTBlock(ModuleFile &F, u } default: - if (Stream.SkipBlock()) { - Error("malformed block record in AST file"); + if (llvm::Error Err = Stream.SkipBlock()) { + Error(std::move(Err)); return Failure; } break; @@ -2794,8 +2988,13 @@ ASTReader::ReadASTBlock(ModuleFile &F, u // 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) { @@ -3814,10 +4013,13 @@ bool ASTReader::loadGlobalIndex() { 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()); @@ -3846,7 +4048,14 @@ static void updateModuleTimestamp(Module /// 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. + consumeError(MaybeEntry.takeError()); + return true; + } + llvm::BitstreamEntry Entry = MaybeEntry.get(); + switch (Entry.Kind) { case llvm::BitstreamEntry::Error: case llvm::BitstreamEntry::EndBlock: @@ -3854,19 +4063,30 @@ static bool SkipCursorToBlock(BitstreamC case llvm::BitstreamEntry::Record: // Ignore top-level records. - Cursor.skipRecord(Entry.ID); - break; + if (Expected<unsigned> Skipped = Cursor.skipRecord(Entry.ID)) + break; + else { + // FIXME this drops errors on the floor. + consumeError(Skipped.takeError()); + 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. + consumeError(std::move(Err)); return true; + } // Found it! return false; } - if (Cursor.SkipBlock()) + if (llvm::Error Err = Cursor.SkipBlock()) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); return true; + } } } } @@ -4108,13 +4328,23 @@ ASTReader::ASTReadResult ASTReader::Read 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) { @@ -4201,16 +4431,21 @@ ASTReader::ReadASTCore(StringRef FileNam 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: @@ -4271,8 +4506,8 @@ ASTReader::ReadASTCore(StringRef FileNam 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; @@ -4342,8 +4577,11 @@ ASTReader::ASTReadResult ASTReader::read BitstreamCursor Stream(StreamData); // Sniff for the signature. - if (!startsWithASTFileMagic(Stream)) + if (llvm::Error Err = doesntStartWithASTFileMagic(Stream)) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); return Failure; + } // Scan for the UNHASHED_CONTROL_BLOCK_ID block. if (SkipCursorToBlock(Stream, UNHASHED_CONTROL_BLOCK_ID)) @@ -4353,7 +4591,13 @@ ASTReader::ASTReadResult ASTReader::read 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. + consumeError(MaybeEntry.takeError()); + return Failure; + } + llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::Error: @@ -4370,8 +4614,12 @@ ASTReader::ASTReadResult ASTReader::read // 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()); @@ -4423,12 +4671,19 @@ ASTReader::ASTReadResult ASTReader::Read 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: @@ -4443,8 +4698,13 @@ ASTReader::ASTReadResult ASTReader::Read 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)) @@ -4615,8 +4875,11 @@ void ASTReader::finalizeForWriting() { /// 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. + consumeError(std::move(Err)); return ASTFileSignature(); + } // Scan for the UNHASHED_CONTROL_BLOCK_ID block. if (SkipCursorToBlock(Stream, UNHASHED_CONTROL_BLOCK_ID)) @@ -4625,13 +4888,27 @@ static ASTFileSignature readASTFileSigna // 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. + consumeError(MaybeEntry.takeError()); + 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. + consumeError(MaybeRecord.takeError()); + 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]}}}; } @@ -4655,8 +4932,8 @@ std::string ASTReader::getOriginalSource 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(); } @@ -4669,7 +4946,15 @@ std::string ASTReader::getOriginalSource // 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. + consumeError(MaybeEntry.takeError()); + return std::string(); + } + llvm::BitstreamEntry Entry = MaybeEntry.get(); + if (Entry.Kind == llvm::BitstreamEntry::EndBlock) return std::string(); @@ -4680,7 +4965,13 @@ std::string ASTReader::getOriginalSource 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. + consumeError(MaybeRecord.takeError()); + return std::string(); + } + if (ORIGINAL_FILE == MaybeRecord.get()) return Blob.str(); } } @@ -4754,8 +5045,10 @@ bool ASTReader::readASTFileControlBlock( 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)) @@ -4770,7 +5063,13 @@ bool ASTReader::readASTFileControlBlock( 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. + consumeError(MaybeEntry.takeError()); + return true; + } + llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::SubBlock: { @@ -4786,15 +5085,22 @@ bool ASTReader::readASTFileControlBlock( 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. + consumeError(std::move(Err)); + 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. + consumeError(std::move(Err)); return true; + } break; } @@ -4816,8 +5122,13 @@ bool ASTReader::readASTFileControlBlock( 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; @@ -4854,13 +5165,28 @@ bool ASTReader::readASTFileControlBlock( 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. + consumeError(std::move(Err)); + } + + Expected<unsigned> MaybeCode = Cursor.ReadCode(); + if (!MaybeCode) { + // FIXME this drops errors on the floor. + consumeError(MaybeCode.takeError()); + } + 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. + consumeError(MaybeRecordType.takeError()); + } + switch ((InputFileRecordTypes)MaybeRecordType.get()) { case INPUT_FILE: bool Overridden = static_cast<bool>(Record[3]); std::string Filename = Blob; @@ -4903,30 +5229,42 @@ bool ASTReader::readASTFileControlBlock( while (!SkipCursorToBlock(Stream, EXTENSION_BLOCK_ID)) { bool DoneWithExtensionBlock = false; while (!DoneWithExtensionBlock) { - llvm::BitstreamEntry Entry = Stream.advance(); - - switch (Entry.Kind) { - case llvm::BitstreamEntry::SubBlock: - if (Stream.SkipBlock()) - return true; + Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance(); + if (!MaybeEntry) { + // FIXME this drops the error. + return true; + } + llvm::BitstreamEntry Entry = MaybeEntry.get(); - continue; + switch (Entry.Kind) { + case llvm::BitstreamEntry::SubBlock: + if (llvm::Error Err = Stream.SkipBlock()) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); + return true; + } + continue; - case llvm::BitstreamEntry::EndBlock: - DoneWithExtensionBlock = true; - continue; + case llvm::BitstreamEntry::EndBlock: + DoneWithExtensionBlock = true; + continue; - case llvm::BitstreamEntry::Error: - return true; + case llvm::BitstreamEntry::Error: + return true; - case llvm::BitstreamEntry::Record: - break; - } + 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)) @@ -4968,8 +5306,8 @@ bool ASTReader::isAcceptableASTFile(Stri 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; } @@ -4978,7 +5316,13 @@ ASTReader::ReadSubmoduleBlock(ModuleFile 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. @@ -4995,7 +5339,12 @@ ASTReader::ReadSubmoduleBlock(ModuleFile // 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"); @@ -5472,10 +5821,20 @@ PreprocessedEntity *ASTReader::ReadPrepr } 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; @@ -5485,10 +5844,13 @@ PreprocessedEntity *ASTReader::ReadPrepr 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; @@ -5897,10 +6259,24 @@ QualType ASTReader::readTypeRecord(unsig 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"); @@ -7205,13 +7581,26 @@ ASTReader::GetExternalCXXCtorInitializer 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; } @@ -7227,11 +7616,27 @@ CXXBaseSpecifier *ASTReader::GetExternal 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; @@ -7439,7 +7844,10 @@ Stmt *ASTReader::GetExternalDeclStmt(uin // 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); @@ -9269,8 +9677,14 @@ void ASTReader::ReadComments() { 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. @@ -9286,7 +9700,12 @@ void ASTReader::ReadComments() { // 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); @@ -11756,8 +12175,8 @@ IdentifierResolver &ASTReader::getIdReso 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); Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Jun 26 12:50:12 2019 @@ -3679,14 +3679,28 @@ Decl *ASTReader::ReadDeclRecord(DeclID I // 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"); @@ -4025,12 +4039,25 @@ void ASTReader::loadDeclUpdateRecords(Pe 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()); @@ -4094,13 +4121,25 @@ void ASTReader::loadPendingDeclChain(Dec 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? Modified: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderStmt.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp Wed Jun 26 12:50:12 2019 @@ -2392,7 +2392,13 @@ Stmt *ASTReader::ReadStmtFromStream(Modu 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. @@ -2410,7 +2416,12 @@ Stmt *ASTReader::ReadStmtFromStream(Modu 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; Modified: cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp (original) +++ cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp Wed Jun 26 12:50:12 2019 @@ -128,12 +128,21 @@ GlobalModuleIndex::GlobalModuleIndex(std 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: @@ -157,19 +166,23 @@ GlobalModuleIndex::GlobalModuleIndex(std 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) @@ -234,7 +247,7 @@ 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; @@ -244,22 +257,26 @@ GlobalModuleIndex::readIndex(StringRef P 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); + llvm::Error::success()); } void @@ -438,9 +455,7 @@ namespace { : 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. @@ -511,24 +526,25 @@ namespace { }; } -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). @@ -538,7 +554,11 @@ bool GlobalModuleIndexBuilder::loadModul 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; @@ -547,8 +567,10 @@ bool GlobalModuleIndexBuilder::loadModul case llvm::BitstreamEntry::Record: // In the 'other' state, just skip the record. We don't care. if (State == Other) { - InStream.skipRecord(Entry.ID); - continue; + if (llvm::Expected<unsigned> Skipped = InStream.skipRecord(Entry.ID)) + continue; + else + return Skipped.takeError(); } // Handle potentially-interesting records below. @@ -556,8 +578,8 @@ bool GlobalModuleIndexBuilder::loadModul 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; @@ -565,8 +587,8 @@ bool GlobalModuleIndexBuilder::loadModul } 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; @@ -574,16 +596,16 @@ bool GlobalModuleIndexBuilder::loadModul } 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; @@ -595,7 +617,10 @@ bool GlobalModuleIndexBuilder::loadModul // 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) { @@ -637,7 +662,9 @@ bool GlobalModuleIndexBuilder::loadModul /*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. @@ -682,7 +709,7 @@ bool GlobalModuleIndexBuilder::loadModul // We don't care about this record. } - return false; + return llvm::Error::success(); } namespace { @@ -820,7 +847,7 @@ bool GlobalModuleIndexBuilder::writeInde return false; } -GlobalModuleIndex::ErrorCode +llvm::Error GlobalModuleIndex::writeIndex(FileManager &FileMgr, const PCHContainerReader &PCHContainerRdr, StringRef Path) { @@ -833,7 +860,7 @@ GlobalModuleIndex::writeIndex(FileManage 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. @@ -842,7 +869,8 @@ GlobalModuleIndex::writeIndex(FileManage 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. @@ -859,7 +887,8 @@ GlobalModuleIndex::writeIndex(FileManage // 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; } @@ -870,8 +899,8 @@ GlobalModuleIndex::writeIndex(FileManage 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. @@ -879,7 +908,8 @@ GlobalModuleIndex::writeIndex(FileManage { 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. @@ -887,31 +917,32 @@ GlobalModuleIndex::writeIndex(FileManage 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 { Modified: cfe/trunk/test/Index/pch-from-libclang.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/pch-from-libclang.c?rev=364464&r1=364463&r2=364464&view=diff ============================================================================== --- cfe/trunk/test/Index/pch-from-libclang.c (original) +++ cfe/trunk/test/Index/pch-from-libclang.c Wed Jun 26 12:50:12 2019 @@ -7,6 +7,7 @@ // - 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits