From: JF Bastien Date: Wed, 26 Jun 2019 19:50:12 +0000 (+0000) Subject: BitStream reader: propagate errors X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0ad57c86b23bd29ec94af2ba29d5ff57582f3076;p=clang 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 Differential Revision: https://reviews.llvm.org/D63518 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@364464 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h index 2cd68ea152..5a707007e4 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -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 #include #include @@ -1303,6 +1304,12 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(SourceLocation Loc, 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); } diff --git a/include/clang/Frontend/FrontendAction.h b/include/clang/Frontend/FrontendAction.h index 3a107d54d5..e994e24cf5 100644 --- a/include/clang/Frontend/FrontendAction.h +++ b/include/clang/Frontend/FrontendAction.h @@ -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 #include #include @@ -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. diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index a03cf1c59a..b299947c18 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/include/clang/Serialization/ASTReader.h @@ -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 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: diff --git a/include/clang/Serialization/GlobalModuleIndex.h b/include/clang/Serialization/GlobalModuleIndex.h index 2f9a70dfa8..5f48126262 100644 --- a/include/clang/Serialization/GlobalModuleIndex.h +++ b/include/clang/Serialization/GlobalModuleIndex.h @@ -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 #include @@ -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 + /// the error. + static std::pair 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); }; } diff --git a/lib/Frontend/ASTUnit.cpp b/lib/Frontend/ASTUnit.cpp index 66e2c2bc4a..4e261d9ec2 100644 --- a/lib/Frontend/ASTUnit.cpp +++ b/lib/Frontend/ASTUnit.cpp @@ -1206,8 +1206,10 @@ bool ASTUnit::Parse(std::shared_ptr PCHContainerOps, 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::LoadFromCompilerInvocationAction( Clang->setASTConsumer( llvm::make_unique(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 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(); } } diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp index 9cbc9ed1f7..cf0267549e 100644 --- a/lib/Frontend/CompilerInstance.cpp +++ b/lib/Frontend/CompilerInstance.cpp @@ -941,7 +941,9 @@ bool CompilerInstance::ExecuteAction(FrontendAction &Act) { 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::loadGlobalModuleIndex( 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::loadGlobalModuleIndex( } } 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(); diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp index 2f4f5ef64c..d724bbce37 100644 --- a/lib/Frontend/FrontendAction.cpp +++ b/lib/Frontend/FrontendAction.cpp @@ -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() { diff --git a/lib/Frontend/PrecompiledPreamble.cpp b/lib/Frontend/PrecompiledPreamble.cpp index 1fe8bfcb74..276a9676ea 100644 --- a/lib/Frontend/PrecompiledPreamble.cpp +++ b/lib/Frontend/PrecompiledPreamble.cpp @@ -352,7 +352,8 @@ llvm::ErrorOr PrecompiledPreamble::Build( 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); diff --git a/lib/Frontend/Rewrite/FrontendActions.cpp b/lib/Frontend/Rewrite/FrontendActions.cpp index aaef44b79d..0f1a0584c7 100644 --- a/lib/Frontend/Rewrite/FrontendActions.cpp +++ b/lib/Frontend/Rewrite/FrontendActions.cpp @@ -129,7 +129,11 @@ bool FixItRecompile::BeginInvocation(CompilerInstance &CI) { 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); diff --git a/lib/Frontend/SerializedDiagnosticReader.cpp b/lib/Frontend/SerializedDiagnosticReader.cpp index bb82e9ad47..c60dd6c1ef 100644 --- a/lib/Frontend/SerializedDiagnosticReader.cpp +++ b/lib/Frontend/SerializedDiagnosticReader.cpp @@ -41,21 +41,47 @@ std::error_code SerializedDiagnosticReader::readDiagnostics(StringRef File) { 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 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 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 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> 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 SerializedDiagnosticReader::readDiagnostics(StringRef File) { 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::skipUntilRecordOrBlock( BlockOrRecordID = 0; while (!Stream.AtEndOfStream()) { - unsigned Code = Stream.ReadCode(); + unsigned Code; + if (Expected 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 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::skipUntilRecordOrBlock( 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::skipUntilRecordOrBlock( 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::readMetaBlock(llvm::BitstreamCursor &Stream) { 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::readMetaBlock(llvm::BitstreamCursor &Stream) { } SmallVector Record; - unsigned RecordID = Stream.readRecord(BlockOrCode, Record); + Expected 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::readMetaBlock(llvm::BitstreamCursor &Stream) { 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::readDiagnosticBlock(llvm::BitstreamCursor &Stream) { 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::readDiagnosticBlock(llvm::BitstreamCursor &Stream) { // Read the record. Record.clear(); StringRef Blob; - unsigned RecID = Stream.readRecord(BlockOrCode, Record, &Blob); + Expected 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) diff --git a/lib/Frontend/TestModuleFileExtension.cpp b/lib/Frontend/TestModuleFileExtension.cpp index 561fe1012a..f8ed341934 100644 --- a/lib/Frontend/TestModuleFileExtension.cpp +++ b/lib/Frontend/TestModuleFileExtension.cpp @@ -48,7 +48,12 @@ TestModuleFileExtension::Reader::Reader(ModuleFileExtension *Ext, // Read the extension block. SmallVector Record; while (true) { - llvm::BitstreamEntry Entry = Stream.advanceSkippingSubblocks(); + llvm::Expected 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(ModuleFileExtension *Ext, Record.clear(); StringRef Blob; - unsigned RecCode = Stream.readRecord(Entry.ID, Record, &Blob); - switch (RecCode) { + Expected 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", diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index fa4a4b38a0..d04db59f35 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -1148,12 +1148,26 @@ bool ASTReader::ReadLexicalDeclContextStorage(ModuleFile &M, 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 MaybeCode = Cursor.ReadCode(); + if (!MaybeCode) { + Error(MaybeCode.takeError()); + return true; + } + unsigned Code = MaybeCode.get(); + + Expected 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::ReadVisibleDeclContextStorage(ModuleFile &M, 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 MaybeCode = Cursor.ReadCode(); + if (!MaybeCode) { + Error(MaybeCode.takeError()); + return true; + } + unsigned Code = MaybeCode.get(); + + Expected 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(ModuleFile &F) { 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 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(ModuleFile &F) { // Read a record. Record.clear(); StringRef Blob; - switch (SLocEntryCursor.readRecord(E.ID, Record, &Blob)) { + Expected 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 { RecordData Record; StringRef Blob; - unsigned Code = SLocEntryCursor.ReadCode(); - unsigned RecCode = SLocEntryCursor.readRecord(Code, Record, &Blob); + Expected MaybeCode = SLocEntryCursor.ReadCode(); + if (!MaybeCode) { + Error(MaybeCode.takeError()); + return nullptr; + } + unsigned Code = MaybeCode.get(); + + Expected 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 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 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::getImportLocation(ModuleFile *F) { 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 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(ModuleFile &F, uint64_t Offset) { // 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 MacroParams; MacroInfo *Macro = nullptr; @@ -1588,7 +1683,13 @@ MacroInfo *ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset) { // 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 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(ModuleFile &F, uint64_t Offset) { // Read a record. Record.clear(); - PreprocessorRecordTypes RecType = - (PreprocessorRecordTypes)Stream.readRecord(Entry.ID, Record); + PreprocessorRecordTypes RecType; + if (Expected 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 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 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(IdentifierInfo *II, 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(IdentifierInfo *II, // macro histroy. RecordData Record; while (true) { - llvm::BitstreamEntry Entry = + Expected 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 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 &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. + consumeError(std::move(Err)); + } - unsigned Code = Cursor.ReadCode(); + Expected 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(Result) == INPUT_FILE && - "invalid record type for input file"); - (void)Result; + if (Expected Maybe = Cursor.readRecord(Code, Record, &Blob)) + assert(static_cast(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(ModuleFile &F, unsigned ID, bool Complain) { // 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::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. + 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 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::ReadOptionsBlock( // Read and process a record. Record.clear(); - switch ((OptionsRecordTypes)Stream.readRecord(Entry.ID, Record)) { + Expected 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 &F, 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 &F, unsigned NumUserInputs = 0; StringRef BaseDirectoryAsWritten; while (true) { - llvm::BitstreamEntry Entry = Stream.advance(); + Expected 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 &F, 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 &F, // 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 &F, // Read and process a record. Record.clear(); StringRef Blob; - switch ((ControlRecordTypes)Stream.readRecord(Entry.ID, Record, &Blob)) { + Expected 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 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, unsigned ClientLoadCapabilities) { // 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, unsigned ClientLoadCapabilities) { 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, unsigned ClientLoadCapabilities) { 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, unsigned ClientLoadCapabilities) { 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, unsigned ClientLoadCapabilities) { } 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, unsigned ClientLoadCapabilities) { // Read and process a record. Record.clear(); StringRef Blob; - auto RecordType = - (ASTRecordTypes)Stream.readRecord(Entry.ID, Record, &Blob); + Expected 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 Result - = GlobalModuleIndex::readIndex(ModuleCachePath); - if (!Result.first) + std::pair 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(ModuleFile &MF) { /// true on failure. static bool SkipCursorToBlock(BitstreamCursor &Cursor, unsigned BlockID) { while (true) { - llvm::BitstreamEntry Entry = Cursor.advance(); + Expected 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(BitstreamCursor &Cursor, unsigned BlockID) { case llvm::BitstreamEntry::Record: // Ignore top-level records. - Cursor.skipRecord(Entry.ID); - break; + if (Expected 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::ReadAST(StringRef FileName, 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 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 FileName, 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 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 FileName, 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::readUnhashedControlBlockImpl( 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::readUnhashedControlBlockImpl( RecordData Record; ASTReadResult Result = Success; while (true) { - llvm::BitstreamEntry Entry = Stream.advance(); + Expected 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::readUnhashedControlBlockImpl( // Read and process a record. Record.clear(); - switch ( - (UnhashedControlBlockRecordTypes)Stream.readRecord(Entry.ID, Record)) { + Expected 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::ReadExtensionBlock(ModuleFile &F) { RecordData Record; while (true) { - llvm::BitstreamEntry Entry = Stream.advance(); + Expected 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::ReadExtensionBlock(ModuleFile &F) { Record.clear(); StringRef Blob; - unsigned RecCode = Stream.readRecord(Entry.ID, Record, &Blob); - switch (RecCode) { + Expected 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 readASTFileSignature(StringRef PCH) { // Scan for SIGNATURE inside the diagnostic options block. ASTReader::RecordData Record; while (true) { - llvm::BitstreamEntry Entry = Stream.advanceSkippingSubblocks(); + Expected 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 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::getOriginalSourceFile( 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::getOriginalSourceFile( // Scan for ORIGINAL_FILE inside the control block. RecordData Record; while (true) { - llvm::BitstreamEntry Entry = Stream.advanceSkippingSubblocks(); + Expected 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::getOriginalSourceFile( Record.clear(); StringRef Blob; - if (Stream.readRecord(Entry.ID, Record, &Blob) == ORIGINAL_FILE) + Expected 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 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 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 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 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(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; - - continue; + Expected 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 (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 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(StringRef Filename, FileManager &FileMgr, 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 &F, unsigned ClientLoadCapabilities) { Module *CurrentModule = nullptr; RecordData Record; while (true) { - llvm::BitstreamEntry Entry = F.Stream.advanceSkippingSubblocks(); + Expected 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 &F, unsigned ClientLoadCapabilities) { // Read a record. StringRef Blob; Record.clear(); - auto Kind = F.Stream.readRecord(Entry.ID, Record, &Blob); + Expected 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::ReadPreprocessedEntity(unsigned Index) { } 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 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::ReadPreprocessedEntity(unsigned Index) { PreprocessingRecord &PPRec = *PP.getPreprocessingRecord(); StringRef Blob; RecordData Record; - PreprocessorDetailRecordTypes RecType = - (PreprocessorDetailRecordTypes)M.PreprocessorDetailCursor.readRecord( - Entry.ID, Record, &Blob); - switch (RecType) { + Expected 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(unsigned Index) { 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 MaybeCode = DeclsCursor.ReadCode(); + if (!MaybeCode) { + Error(MaybeCode.takeError()); + return QualType(); + } + unsigned Code = MaybeCode.get(); + + Expected 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::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 MaybeCode = Cursor.ReadCode(); + if (!MaybeCode) { + Error(MaybeCode.takeError()); + return nullptr; + } + unsigned Code = MaybeCode.get(); + + Expected 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::GetExternalCXXBaseSpecifiers(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); + + Expected MaybeCode = Cursor.ReadCode(); + if (!MaybeCode) { + Error(MaybeCode.takeError()); + return nullptr; + } + unsigned Code = MaybeCode.get(); + + Expected 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(uint64_t Offset) { // 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 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 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::getIdResolver() { return SemaObj ? SemaObj->IdResolver : DummyIdResolver; } -unsigned ASTRecordReader::readRecord(llvm::BitstreamCursor &Cursor, - unsigned AbbrevID) { +Expected ASTRecordReader::readRecord(llvm::BitstreamCursor &Cursor, + unsigned AbbrevID) { Idx = 0; Record.clear(); return Cursor.readRecord(AbbrevID, Record); diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 359c86d019..bd7880c339 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -3679,14 +3679,28 @@ Decl *ASTReader::ReadDeclRecord(DeclID ID) { // 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 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 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(PendingUpdateRecord &Record) { 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 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 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(Decl *FirstLocal, uint64_t LocalOffset) { 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 MaybeCode = Cursor.ReadCode(); + if (!MaybeCode) + llvm::report_fatal_error( + "ASTReader::loadPendingDeclChain failed reading code: " + + toString(MaybeCode.takeError())); + unsigned Code = MaybeCode.get(); + if (Expected 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? diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp index e94a9125b7..82f055c3ec 100644 --- a/lib/Serialization/ASTReaderStmt.cpp +++ b/lib/Serialization/ASTReaderStmt.cpp @@ -2392,7 +2392,13 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) { Stmt::EmptyShell Empty; while (true) { - llvm::BitstreamEntry Entry = Cursor.advanceSkippingSubblocks(); + llvm::Expected 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(ModuleFile &F) { Stmt *S = nullptr; bool Finished = false; bool IsStmtReference = false; - switch ((StmtCode)Record.readRecord(Cursor, Entry.ID)) { + Expected MaybeStmtCode = Record.readRecord(Cursor, Entry.ID); + if (!MaybeStmtCode) { + Error(toString(MaybeStmtCode.takeError())); + return nullptr; + } + switch ((StmtCode)MaybeStmtCode.get()) { case STMT_STOP: Finished = true; break; diff --git a/lib/Serialization/GlobalModuleIndex.cpp b/lib/Serialization/GlobalModuleIndex.cpp index ebcfa9f506..626eebbef9 100644 --- a/lib/Serialization/GlobalModuleIndex.cpp +++ b/lib/Serialization/GlobalModuleIndex.cpp @@ -128,12 +128,21 @@ GlobalModuleIndex::GlobalModuleIndex(std::unique_ptr 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 Res = Cursor.advance()) + Entry = Res.get(); + else + Fail(Res.takeError()); switch (Entry.Kind) { case llvm::BitstreamEntry::Error: @@ -157,19 +166,23 @@ GlobalModuleIndex::GlobalModuleIndex(std::unique_ptr Buffer, 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 Record; StringRef Blob; - switch ((IndexRecordTypes)Cursor.readRecord(Entry.ID, Record, &Blob)) { + Expected MaybeIndexRecord = + Cursor.readRecord(Entry.ID, Record, &Blob); + if (!MaybeIndexRecord) + Fail(MaybeIndexRecord.takeError()); + IndexRecordTypes IndexRecord = + static_cast(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(IdentifierIndex); } -std::pair +std::pair GlobalModuleIndex::readIndex(StringRef Path) { // Load the index file, if it's there. llvm::SmallString<128> IndexPath; @@ -244,22 +257,26 @@ GlobalModuleIndex::readIndex(StringRef Path) { llvm::ErrorOr> 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 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 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 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::loadModuleFile(const FileEntry *File) { enum { Other, ControlBlock, ASTBlock, DiagnosticOptionsBlock } State = Other; bool Done = false; while (!Done) { - llvm::BitstreamEntry Entry = InStream.advance(); + Expected 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::loadModuleFile(const FileEntry *File) { 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 Skipped = InStream.skipRecord(Entry.ID)) + continue; + else + return Skipped.takeError(); } // Handle potentially-interesting records below. @@ -556,8 +578,8 @@ bool GlobalModuleIndexBuilder::loadModuleFile(const FileEntry *File) { 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::loadModuleFile(const FileEntry *File) { } 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::loadModuleFile(const FileEntry *File) { } 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::loadModuleFile(const FileEntry *File) { // Read the given record. SmallVector Record; StringRef Blob; - unsigned Code = InStream.readRecord(Entry.ID, Record, &Blob); + Expected 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::loadModuleFile(const FileEntry *File) { /*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::loadModuleFile(const FileEntry *File) { // We don't care about this record. } - return false; + return llvm::Error::success(); } namespace { @@ -820,7 +847,7 @@ bool GlobalModuleIndexBuilder::writeIndex(llvm::BitstreamWriter &Stream) { return false; } -GlobalModuleIndex::ErrorCode +llvm::Error GlobalModuleIndex::writeIndex(FileManager &FileMgr, const PCHContainerReader &PCHContainerRdr, StringRef Path) { @@ -833,7 +860,7 @@ GlobalModuleIndex::writeIndex(FileManager &FileMgr, 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(FileManager &FileMgr, 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(FileManager &FileMgr, // 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(FileManager &FileMgr, 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(FileManager &FileMgr, { 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(FileManager &FileMgr, 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 { diff --git a/test/Index/pch-from-libclang.c b/test/Index/pch-from-libclang.c index f4dd0f046f..ec6286da2a 100644 --- a/test/Index/pch-from-libclang.c +++ b/test/Index/pch-from-libclang.c @@ -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