From: Ben Barham Date: Sat, 28 Aug 2021 02:38:46 +0000 (-0700) Subject: [Modules] Change result of reading AST block to llvm::Error instead X-Git-Tag: upstream/15.0.7~32801 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a4a5c00b53d0638e2bd9011fa5cce7ef9739eea6;p=platform%2Fupstream%2Fllvm.git [Modules] Change result of reading AST block to llvm::Error instead Reading the AST block can never fail with a recoverable error as modules cannot be removed during this phase. Change the return type of these functions to return an llvm::Error instead, ie. either success or failure. NFC other than the wording of some of the errors. Differential Revision: https://reviews.llvm.org/D108268 --- diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 242b75b..e342a06 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1320,18 +1320,18 @@ private: ASTReaderListener *Listener, bool ValidateDiagnosticOptions); - ASTReadResult ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities); - ASTReadResult ReadExtensionBlock(ModuleFile &F); + llvm::Error ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities); + llvm::Error ReadExtensionBlock(ModuleFile &F); void ReadModuleOffsetMap(ModuleFile &F) const; - bool ParseLineTable(ModuleFile &F, const RecordData &Record); - bool ReadSourceManagerBlock(ModuleFile &F); + void ParseLineTable(ModuleFile &F, const RecordData &Record); + llvm::Error ReadSourceManagerBlock(ModuleFile &F); llvm::BitstreamCursor &SLocCursorForID(int ID); SourceLocation getImportLocation(ModuleFile *F); ASTReadResult ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, const ModuleFile *ImportedBy, unsigned ClientLoadCapabilities); - ASTReadResult ReadSubmoduleBlock(ModuleFile &F, - unsigned ClientLoadCapabilities); + llvm::Error ReadSubmoduleBlock(ModuleFile &F, + unsigned ClientLoadCapabilities); static bool ParseLanguageOptions(const RecordData &Record, bool Complain, ASTReaderListener &Listener, bool AllowCompatibleDifferences); @@ -1904,8 +1904,9 @@ public: /// 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. - static bool ReadBlockAbbrevs(llvm::BitstreamCursor &Cursor, unsigned BlockID, - uint64_t *StartOfBlockOffset = nullptr); + static llvm::Error ReadBlockAbbrevs(llvm::BitstreamCursor &Cursor, + unsigned BlockID, + uint64_t *StartOfBlockOffset = nullptr); /// Finds all the visible declarations with a given name. /// The current implementation of this method just loads the entire diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 376dbdf..128350c 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -10,15 +10,13 @@ // //===----------------------------------------------------------------------===// -#include "clang/Basic/OpenMPKinds.h" -#include "clang/Serialization/ASTRecordReader.h" #include "ASTCommon.h" #include "ASTReaderInternals.h" -#include "clang/AST/AbstractTypeReader.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTMutationListener.h" #include "clang/AST/ASTUnresolvedSet.h" +#include "clang/AST/AbstractTypeReader.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/AST/DeclCXX.h" @@ -31,8 +29,8 @@ #include "clang/AST/ExprCXX.h" #include "clang/AST/ExternalASTSource.h" #include "clang/AST/NestedNameSpecifier.h" -#include "clang/AST/OpenMPClause.h" #include "clang/AST/ODRHash.h" +#include "clang/AST/OpenMPClause.h" #include "clang/AST/RawCommentList.h" #include "clang/AST/TemplateBase.h" #include "clang/AST/TemplateName.h" @@ -42,6 +40,7 @@ #include "clang/AST/UnresolvedSet.h" #include "clang/Basic/CommentOptions.h" #include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticError.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/ExceptionSpecificationType.h" #include "clang/Basic/FileManager.h" @@ -51,6 +50,7 @@ #include "clang/Basic/LangOptions.h" #include "clang/Basic/Module.h" #include "clang/Basic/ObjCRuntime.h" +#include "clang/Basic/OpenMPKinds.h" #include "clang/Basic/OperatorKinds.h" #include "clang/Basic/PragmaKinds.h" #include "clang/Basic/Sanitizers.h" @@ -76,6 +76,7 @@ #include "clang/Sema/Weak.h" #include "clang/Serialization/ASTBitCodes.h" #include "clang/Serialization/ASTDeserializationListener.h" +#include "clang/Serialization/ASTRecordReader.h" #include "clang/Serialization/ContinuousRangeMap.h" #include "clang/Serialization/GlobalModuleIndex.h" #include "clang/Serialization/InMemoryModuleCache.h" @@ -1263,7 +1264,29 @@ void ASTReader::Error(unsigned DiagID, StringRef Arg1, StringRef Arg2, } void ASTReader::Error(llvm::Error &&Err) const { - Error(toString(std::move(Err))); + llvm::Error RemainingErr = + handleErrors(std::move(Err), [this](const DiagnosticError &E) { + auto Diag = E.getDiagnostic().second; + + // Ideally we'd just emit it, but have to handle a possible in-flight + // diagnostic. Note that the location is currently ignored as well. + auto NumArgs = Diag.getStorage()->NumDiagArgs; + assert(NumArgs <= 3 && "Can only have up to 3 arguments"); + StringRef Arg1, Arg2, Arg3; + switch (NumArgs) { + case 3: + Arg3 = Diag.getStringArg(2); + LLVM_FALLTHROUGH; + case 2: + Arg2 = Diag.getStringArg(1); + LLVM_FALLTHROUGH; + case 1: + Arg1 = Diag.getStringArg(0); + } + Error(Diag.getDiagID(), Arg1, Arg2, Arg3); + }); + if (RemainingErr) + Error(toString(std::move(RemainingErr))); } //===----------------------------------------------------------------------===// @@ -1271,9 +1294,7 @@ void ASTReader::Error(llvm::Error &&Err) const { //===----------------------------------------------------------------------===// /// Read the line table in the source manager block. -/// \returns true if there was an error. -bool ASTReader::ParseLineTable(ModuleFile &F, - const RecordData &Record) { +void ASTReader::ParseLineTable(ModuleFile &F, const RecordData &Record) { unsigned Idx = 0; LineTableInfo &LineTable = SourceMgr.getLineTable(); @@ -1312,12 +1333,10 @@ bool ASTReader::ParseLineTable(ModuleFile &F, } LineTable.AddEntry(FileID::get(FID), Entries); } - - return false; } /// Read a source manager block -bool ASTReader::ReadSourceManagerBlock(ModuleFile &F) { +llvm::Error ASTReader::ReadSourceManagerBlock(ModuleFile &F) { using namespace SrcMgr; BitstreamCursor &SLocEntryCursor = F.SLocEntryCursor; @@ -1329,36 +1348,29 @@ bool ASTReader::ReadSourceManagerBlock(ModuleFile &F) { SLocEntryCursor = F.Stream; // The stream itself is going to skip over the source manager block. - if (llvm::Error Err = F.Stream.SkipBlock()) { - Error(std::move(Err)); - return true; - } + if (llvm::Error Err = F.Stream.SkipBlock()) + return Err; // Enter the source manager block. - if (llvm::Error Err = - SLocEntryCursor.EnterSubBlock(SOURCE_MANAGER_BLOCK_ID)) { - Error(std::move(Err)); - return true; - } + if (llvm::Error Err = SLocEntryCursor.EnterSubBlock(SOURCE_MANAGER_BLOCK_ID)) + return Err; F.SourceManagerBlockStartOffset = SLocEntryCursor.GetCurrentBitNo(); RecordData Record; while (true) { Expected MaybeE = SLocEntryCursor.advanceSkippingSubblocks(); - if (!MaybeE) { - Error(MaybeE.takeError()); - return true; - } + if (!MaybeE) + return MaybeE.takeError(); llvm::BitstreamEntry E = MaybeE.get(); switch (E.Kind) { case llvm::BitstreamEntry::SubBlock: // Handled for us already. case llvm::BitstreamEntry::Error: - Error("malformed block record in AST file"); - return true; + return llvm::createStringError(std::errc::illegal_byte_sequence, + "malformed block record in AST file"); case llvm::BitstreamEntry::EndBlock: - return false; + return llvm::Error::success(); case llvm::BitstreamEntry::Record: // The interesting case. break; @@ -1369,10 +1381,8 @@ bool ASTReader::ReadSourceManagerBlock(ModuleFile &F) { StringRef Blob; Expected MaybeRecord = SLocEntryCursor.readRecord(E.ID, Record, &Blob); - if (!MaybeRecord) { - Error(MaybeRecord.takeError()); - return true; - } + if (!MaybeRecord) + return MaybeRecord.takeError(); switch (MaybeRecord.get()) { default: // Default behavior: ignore. break; @@ -1381,7 +1391,7 @@ bool ASTReader::ReadSourceManagerBlock(ModuleFile &F) { case SM_SLOC_BUFFER_ENTRY: case SM_SLOC_EXPANSION_ENTRY: // Once we hit one of the source location entries, we're done. - return false; + return llvm::Error::success(); } } } @@ -1632,13 +1642,11 @@ SourceLocation ASTReader::getImportLocation(ModuleFile *F) { /// 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, - uint64_t *StartOfBlockOffset) { - if (llvm::Error Err = Cursor.EnterSubBlock(BlockID)) { - // FIXME this drops errors on the floor. - consumeError(std::move(Err)); - return true; - } +llvm::Error ASTReader::ReadBlockAbbrevs(BitstreamCursor &Cursor, + unsigned BlockID, + uint64_t *StartOfBlockOffset) { + if (llvm::Error Err = Cursor.EnterSubBlock(BlockID)) + return Err; if (StartOfBlockOffset) *StartOfBlockOffset = Cursor.GetCurrentBitNo(); @@ -1646,27 +1654,18 @@ bool ASTReader::ReadBlockAbbrevs(BitstreamCursor &Cursor, unsigned BlockID, while (true) { uint64_t Offset = Cursor.GetCurrentBitNo(); Expected MaybeCode = Cursor.ReadCode(); - if (!MaybeCode) { - // FIXME this drops errors on the floor. - consumeError(MaybeCode.takeError()); - return true; - } + if (!MaybeCode) + return MaybeCode.takeError(); unsigned Code = MaybeCode.get(); // We expect all abbrevs to be at the start of the block. if (Code != llvm::bitc::DEFINE_ABBREV) { - if (llvm::Error Err = Cursor.JumpToBit(Offset)) { - // FIXME this drops errors on the floor. - consumeError(std::move(Err)); - return true; - } - return false; - } - if (llvm::Error Err = Cursor.ReadAbbrevRecord()) { - // FIXME this drops errors on the floor. - consumeError(std::move(Err)); - return true; + if (llvm::Error Err = Cursor.JumpToBit(Offset)) + return Err; + return llvm::Error::success(); } + if (llvm::Error Err = Cursor.ReadAbbrevRecord()) + return Err; } } @@ -2966,30 +2965,27 @@ ASTReader::ReadControlBlock(ModuleFile &F, } } -ASTReader::ASTReadResult -ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { +llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, + unsigned ClientLoadCapabilities) { BitstreamCursor &Stream = F.Stream; - if (llvm::Error Err = Stream.EnterSubBlock(AST_BLOCK_ID)) { - Error(std::move(Err)); - return Failure; - } + if (llvm::Error Err = Stream.EnterSubBlock(AST_BLOCK_ID)) + return Err; F.ASTBlockStartOffset = Stream.GetCurrentBitNo(); // Read all of the records and blocks for the AST file. RecordData Record; while (true) { Expected MaybeEntry = Stream.advance(); - if (!MaybeEntry) { - Error(MaybeEntry.takeError()); - return Failure; - } + if (!MaybeEntry) + return MaybeEntry.takeError(); llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::Error: - Error("error at end of module block in AST file"); - return Failure; + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "error at end of module block in AST file"); case llvm::BitstreamEntry::EndBlock: // Outside of C++, we do not store a lookup map for the translation unit. // Instead, mark it as needing a lookup map to be built if this module @@ -3002,7 +2998,7 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { DC->setMustBuildLookupTable(); } - return Success; + return llvm::Error::success(); case llvm::BitstreamEntry::SubBlock: switch (Entry.ID) { case DECLTYPES_BLOCK_ID: @@ -3011,15 +3007,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 (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } - if (ReadBlockAbbrevs(F.DeclsCursor, DECLTYPES_BLOCK_ID, - &F.DeclsBlockStartOffset)) { - Error("malformed block record in AST file"); - return Failure; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; + if (llvm::Error Err = ReadBlockAbbrevs( + F.DeclsCursor, DECLTYPES_BLOCK_ID, &F.DeclsBlockStartOffset)) + return Err; break; case PREPROCESSOR_BLOCK_ID: @@ -3027,14 +3019,11 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { if (!PP.getExternalSource()) PP.setExternalSource(this); - 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; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; + if (llvm::Error Err = + ReadBlockAbbrevs(F.MacroCursor, PREPROCESSOR_BLOCK_ID)) + return Err; F.MacroStartOffset = F.MacroCursor.GetCurrentBitNo(); break; @@ -3042,14 +3031,11 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { F.PreprocessorDetailCursor = Stream; 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; + return Err; } + if (llvm::Error Err = ReadBlockAbbrevs(F.PreprocessorDetailCursor, + PREPROCESSOR_DETAIL_BLOCK_ID)) + return Err; F.PreprocessorDetailStartOffset = F.PreprocessorDetailCursor.GetCurrentBitNo(); @@ -3060,36 +3046,29 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case SOURCE_MANAGER_BLOCK_ID: - if (ReadSourceManagerBlock(F)) - return Failure; + if (llvm::Error Err = ReadSourceManagerBlock(F)) + return Err; break; case SUBMODULE_BLOCK_ID: - if (ASTReadResult Result = - ReadSubmoduleBlock(F, ClientLoadCapabilities)) - return Result; + if (llvm::Error Err = ReadSubmoduleBlock(F, ClientLoadCapabilities)) + return Err; break; case COMMENTS_BLOCK_ID: { BitstreamCursor C = Stream; - 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; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; + if (llvm::Error Err = ReadBlockAbbrevs(C, COMMENTS_BLOCK_ID)) + return Err; CommentsCursors.push_back(std::make_pair(C, &F)); break; } default: - if (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; break; } continue; @@ -3104,10 +3083,8 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { StringRef Blob; Expected MaybeRecordType = Stream.readRecord(Entry.ID, Record, &Blob); - if (!MaybeRecordType) { - Error(MaybeRecordType.takeError()); - return Failure; - } + if (!MaybeRecordType) + return MaybeRecordType.takeError(); ASTRecordTypes RecordType = (ASTRecordTypes)MaybeRecordType.get(); // If we're not loading an AST context, we don't care about most records. @@ -3138,10 +3115,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case TYPE_OFFSET: { - if (F.LocalNumTypes != 0) { - Error("duplicate TYPE_OFFSET record in AST file"); - return Failure; - } + if (F.LocalNumTypes != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate TYPE_OFFSET record in AST file"); F.TypeOffsets = reinterpret_cast(Blob.data()); F.LocalNumTypes = Record[0]; unsigned LocalBaseTypeIndex = Record[1]; @@ -3162,10 +3139,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { } case DECL_OFFSET: { - if (F.LocalNumDecls != 0) { - Error("duplicate DECL_OFFSET record in AST file"); - return Failure; - } + if (F.LocalNumDecls != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate DECL_OFFSET record in AST file"); F.DeclOffsets = (const DeclOffset *)Blob.data(); F.LocalNumDecls = Record[0]; unsigned LocalBaseDeclID = Record[1]; @@ -3230,10 +3207,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case IDENTIFIER_OFFSET: { - if (F.LocalNumIdentifiers != 0) { - Error("duplicate IDENTIFIER_OFFSET record in AST file"); - return Failure; - } + if (F.LocalNumIdentifiers != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate IDENTIFIER_OFFSET record in AST file"); F.IdentifierOffsets = (const uint32_t *)Blob.data(); F.LocalNumIdentifiers = Record[0]; unsigned LocalBaseIdentifierID = Record[1]; @@ -3284,10 +3261,9 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; } - if (SpecialTypes.size() != Record.size()) { - Error("invalid special-types record"); - return Failure; - } + if (SpecialTypes.size() != Record.size()) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid special-types record"); for (unsigned I = 0, N = Record.size(); I != N; ++I) { serialization::TypeID ID = getGlobalTypeID(F, Record[I]); @@ -3316,10 +3292,9 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case WEAK_UNDECLARED_IDENTIFIERS: - if (Record.size() % 4 != 0) { - Error("invalid weak identifiers record"); - return Failure; - } + if (Record.size() % 4 != 0) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid weak identifiers record"); // FIXME: Ignore weak undeclared identifiers from non-original PCH // files. This isn't the way to do it :) @@ -3426,10 +3401,9 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { std::tie(F.SLocEntryBaseID, F.SLocEntryBaseOffset) = SourceMgr.AllocateLoadedSLocEntries(F.LocalNumSLocEntries, SLocSpaceSize); - if (!F.SLocEntryBaseID) { - Error("ran out of source locations"); - break; - } + if (!F.SLocEntryBaseID) + return llvm::createStringError(std::errc::invalid_argument, + "ran out of source locations"); // Make our entry in the range map. BaseID is negative and growing, so // we invert it. Because we invert it, though, we need the other end of // the range. @@ -3460,19 +3434,16 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case SOURCE_MANAGER_LINE_TABLE: - if (ParseLineTable(F, Record)) { - Error("malformed SOURCE_MANAGER_LINE_TABLE in AST file"); - return Failure; - } + ParseLineTable(F, Record); break; case SOURCE_LOCATION_PRELOADS: { // Need to transform from the local view (1-based IDs) to the global view, // which is based off F.SLocEntryBaseID. - if (!F.PreloadSLocEntries.empty()) { - Error("Multiple SOURCE_LOCATION_PRELOADS records in AST file"); - return Failure; - } + if (!F.PreloadSLocEntries.empty()) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "Multiple SOURCE_LOCATION_PRELOADS records in AST file"); F.PreloadSLocEntries.swap(Record); break; @@ -3484,10 +3455,9 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case VTABLE_USES: - if (Record.size() % 3 != 0) { - Error("Invalid VTABLE_USES record"); - return Failure; - } + if (Record.size() % 3 != 0) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "Invalid VTABLE_USES record"); // Later tables overwrite earlier ones. // FIXME: Modules will have some trouble with this. This is clearly not @@ -3503,15 +3473,15 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case PENDING_IMPLICIT_INSTANTIATIONS: - if (PendingInstantiations.size() % 2 != 0) { - Error("Invalid existing PendingInstantiations"); - return Failure; - } + if (PendingInstantiations.size() % 2 != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "Invalid existing PendingInstantiations"); - if (Record.size() % 2 != 0) { - Error("Invalid PENDING_IMPLICIT_INSTANTIATIONS block"); - return Failure; - } + if (Record.size() % 2 != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "Invalid PENDING_IMPLICIT_INSTANTIATIONS block"); for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) { PendingInstantiations.push_back(getGlobalDeclID(F, Record[I++])); @@ -3521,10 +3491,9 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case SEMA_DECL_REFS: - if (Record.size() != 3) { - Error("Invalid SEMA_DECL_REFS block"); - return Failure; - } + if (Record.size() != 3) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "Invalid SEMA_DECL_REFS block"); for (unsigned I = 0, N = Record.size(); I != N; ++I) SemaDeclRefs.push_back(getGlobalDeclID(F, Record[I])); break; @@ -3580,10 +3549,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { } case DECL_UPDATE_OFFSETS: - if (Record.size() % 2 != 0) { - Error("invalid DECL_UPDATE_OFFSETS block in AST file"); - return Failure; - } + if (Record.size() % 2 != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "invalid DECL_UPDATE_OFFSETS block in AST file"); for (unsigned I = 0, N = Record.size(); I != N; I += 2) { GlobalDeclID ID = getGlobalDeclID(F, Record[I]); DeclUpdateOffsets[ID].push_back(std::make_pair(&F, Record[I + 1])); @@ -3597,10 +3566,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case OBJC_CATEGORIES_MAP: - if (F.LocalNumObjCCategoriesInMap != 0) { - Error("duplicate OBJC_CATEGORIES_MAP record in AST file"); - return Failure; - } + if (F.LocalNumObjCCategoriesInMap != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate OBJC_CATEGORIES_MAP record in AST file"); F.LocalNumObjCCategoriesInMap = Record[0]; F.ObjCCategoriesMap = (const ObjCCategoriesInfo *)Blob.data(); @@ -3665,15 +3634,13 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case UNDEFINED_BUT_USED: - if (UndefinedButUsed.size() % 2 != 0) { - Error("Invalid existing UndefinedButUsed"); - return Failure; - } + if (UndefinedButUsed.size() % 2 != 0) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "Invalid existing UndefinedButUsed"); - if (Record.size() % 2 != 0) { - Error("invalid undefined-but-used record"); - return Failure; - } + if (Record.size() % 2 != 0) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid undefined-but-used record"); for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) { UndefinedButUsed.push_back(getGlobalDeclID(F, Record[I++])); UndefinedButUsed.push_back( @@ -3712,10 +3679,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case MACRO_OFFSET: { - if (F.LocalNumMacros != 0) { - Error("duplicate MACRO_OFFSET record in AST file"); - return Failure; - } + if (F.LocalNumMacros != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate MACRO_OFFSET record in AST file"); F.MacroOffsets = (const uint32_t *)Blob.data(); F.LocalNumMacros = Record[0]; unsigned LocalBaseMacroID = Record[1]; @@ -3743,26 +3710,24 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case OPTIMIZE_PRAGMA_OPTIONS: - if (Record.size() != 1) { - Error("invalid pragma optimize record"); - return Failure; - } + if (Record.size() != 1) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid pragma optimize record"); OptimizeOffPragmaLocation = ReadSourceLocation(F, Record[0]); break; case MSSTRUCT_PRAGMA_OPTIONS: - if (Record.size() != 1) { - Error("invalid pragma ms_struct record"); - return Failure; - } + if (Record.size() != 1) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid pragma ms_struct record"); PragmaMSStructState = Record[0]; break; case POINTERS_TO_MEMBERS_PRAGMA_OPTIONS: - if (Record.size() != 2) { - Error("invalid pragma ms_struct record"); - return Failure; - } + if (Record.size() != 2) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "invalid pragma pointers to members record"); PragmaMSPointersToMembersState = Record[0]; PointersToMembersPragmaLocation = ReadSourceLocation(F, Record[1]); break; @@ -3774,18 +3739,16 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case CUDA_PRAGMA_FORCE_HOST_DEVICE_DEPTH: - if (Record.size() != 1) { - Error("invalid cuda pragma options record"); - return Failure; - } + if (Record.size() != 1) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid cuda pragma options record"); ForceCUDAHostDeviceDepth = Record[0]; break; case ALIGN_PACK_PRAGMA_OPTIONS: { - if (Record.size() < 3) { - Error("invalid pragma pack record"); - return Failure; - } + if (Record.size() < 3) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid pragma pack record"); PragmaAlignPackCurrentValue = ReadAlignPackInfo(Record[0]); PragmaAlignPackCurrentLocation = ReadSourceLocation(F, Record[1]); unsigned NumStackEntries = Record[2]; @@ -3805,10 +3768,9 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { } case FLOAT_CONTROL_PRAGMA_OPTIONS: { - if (Record.size() < 3) { - Error("invalid pragma pack record"); - return Failure; - } + if (Record.size() < 3) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid pragma float control record"); FpPragmaCurrentValue = FPOptionsOverride::getFromOpaqueInt(Record[0]); FpPragmaCurrentLocation = ReadSourceLocation(F, Record[1]); unsigned NumStackEntries = Record[2]; @@ -4269,16 +4231,23 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, return ReadResult; } - // Here comes stuff that we only do once the entire chain is loaded. + // Here comes stuff that we only do once the entire chain is loaded. Do *not* + // remove modules from this point. Various fields are updated during reading + // the AST block and removing the modules would result in dangling pointers. + // They are generally only incidentally dereferenced, ie. a binary search + // runs over `GlobalSLocEntryMap`, which could cause an invalid module to + // be dereferenced but it wouldn't actually be used. - // Load the AST blocks of all of the modules that we loaded. We can still + // Load the AST blocks of all of the modules that we loaded. We can still // hit errors parsing the ASTs at this point. for (ImportedModule &M : Loaded) { ModuleFile &F = *M.Mod; // Read the AST block. - if (ReadASTBlock(F, ClientLoadCapabilities)) + if (llvm::Error Err = ReadASTBlock(F, ClientLoadCapabilities)) { + Error(std::move(Err)); return Failure; + } // The AST block should always have a definition for the main module. if (F.isModule() && !F.DidReadTopLevelSubmodule) { @@ -4288,8 +4257,10 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, // Read the extension blocks. while (!SkipCursorToBlock(F.Stream, EXTENSION_BLOCK_ID)) { - if (ReadExtensionBlock(F)) + if (llvm::Error Err = ReadExtensionBlock(F)) { + Error(std::move(Err)); return Failure; + } } // Once read, set the ModuleFile bit base offset and update the size in @@ -4810,32 +4781,26 @@ static bool parseModuleFileExtensionMetadata( return false; } -ASTReader::ASTReadResult ASTReader::ReadExtensionBlock(ModuleFile &F) { +llvm::Error ASTReader::ReadExtensionBlock(ModuleFile &F) { BitstreamCursor &Stream = F.Stream; RecordData Record; while (true) { Expected MaybeEntry = Stream.advance(); - if (!MaybeEntry) { - Error(MaybeEntry.takeError()); - return Failure; - } + if (!MaybeEntry) + return MaybeEntry.takeError(); llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::SubBlock: - if (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; continue; - case llvm::BitstreamEntry::EndBlock: - return Success; - + return llvm::Error::success(); case llvm::BitstreamEntry::Error: - return HadErrors; - + return llvm::createStringError(std::errc::illegal_byte_sequence, + "malformed block record in AST file"); case llvm::BitstreamEntry::Record: break; } @@ -4844,17 +4809,15 @@ ASTReader::ASTReadResult ASTReader::ReadExtensionBlock(ModuleFile &F) { StringRef Blob; Expected MaybeRecCode = Stream.readRecord(Entry.ID, Record, &Blob); - if (!MaybeRecCode) { - Error(MaybeRecCode.takeError()); - return Failure; - } + if (!MaybeRecCode) + return MaybeRecCode.takeError(); switch (MaybeRecCode.get()) { case EXTENSION_METADATA: { ModuleFileExtensionMetadata Metadata; - if (parseModuleFileExtensionMetadata(Record, Blob, Metadata)) { - Error("malformed EXTENSION_METADATA in AST file"); - return Failure; - } + if (parseModuleFileExtensionMetadata(Record, Blob, Metadata)) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "malformed EXTENSION_METADATA in AST file"); // Find a module file extension with this block name. auto Known = ModuleFileExtensions.find(Metadata.BlockName); @@ -4871,7 +4834,7 @@ ASTReader::ASTReadResult ASTReader::ReadExtensionBlock(ModuleFile &F) { } } - return Success; + return llvm::Error::success(); } void ASTReader::InitializeContext() { @@ -5451,13 +5414,11 @@ bool ASTReader::isAcceptableASTFile(StringRef Filename, FileManager &FileMgr, /*ValidateDiagnosticOptions=*/true); } -ASTReader::ASTReadResult -ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { +llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, + unsigned ClientLoadCapabilities) { // Enter the submodule block. - if (llvm::Error Err = F.Stream.EnterSubBlock(SUBMODULE_BLOCK_ID)) { - Error(std::move(Err)); - return Failure; - } + if (llvm::Error Err = F.Stream.EnterSubBlock(SUBMODULE_BLOCK_ID)) + return Err; ModuleMap &ModMap = PP.getHeaderSearchInfo().getModuleMap(); bool First = true; @@ -5466,19 +5427,17 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { while (true) { Expected MaybeEntry = F.Stream.advanceSkippingSubblocks(); - if (!MaybeEntry) { - Error(MaybeEntry.takeError()); - return Failure; - } + if (!MaybeEntry) + return MaybeEntry.takeError(); llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::SubBlock: // Handled for us already. case llvm::BitstreamEntry::Error: - Error("malformed block record in AST file"); - return Failure; + return llvm::createStringError(std::errc::illegal_byte_sequence, + "malformed block record in AST file"); case llvm::BitstreamEntry::EndBlock: - return Success; + return llvm::Error::success(); case llvm::BitstreamEntry::Record: // The interesting case. break; @@ -5488,16 +5447,14 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { StringRef Blob; Record.clear(); Expected MaybeKind = F.Stream.readRecord(Entry.ID, Record, &Blob); - if (!MaybeKind) { - Error(MaybeKind.takeError()); - return Failure; - } + if (!MaybeKind) + return MaybeKind.takeError(); unsigned Kind = MaybeKind.get(); - if ((Kind == SUBMODULE_METADATA) != First) { - Error("submodule metadata record should be at beginning of block"); - return Failure; - } + if ((Kind == SUBMODULE_METADATA) != First) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "submodule metadata record should be at beginning of block"); First = false; // Submodule information is only valid if we have a current module. @@ -5511,10 +5468,9 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { break; case SUBMODULE_DEFINITION: { - if (Record.size() < 12) { - Error("malformed module definition"); - return Failure; - } + if (Record.size() < 12) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "malformed module definition"); StringRef Name = Blob; unsigned Idx = 0; @@ -5546,10 +5502,9 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { SubmoduleID GlobalIndex = GlobalID - NUM_PREDEF_SUBMODULE_IDS; if (GlobalIndex >= SubmodulesLoaded.size() || - SubmodulesLoaded[GlobalIndex]) { - Error("too many submodules"); - return Failure; - } + SubmodulesLoaded[GlobalIndex]) + return llvm::createStringError(std::errc::invalid_argument, + "too many submodules"); if (!ParentModule) { if (const FileEntry *CurFile = CurrentModule->getASTFile()) { @@ -5557,10 +5512,12 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { if (!bool(PP.getPreprocessorOpts().DisablePCHOrModuleValidation & DisableValidationForModuleKind::Module) && CurFile != F.File) { - Error(diag::err_module_file_conflict, - CurrentModule->getTopLevelModuleName(), CurFile->getName(), - F.File->getName()); - return Failure; + auto ConflictError = + PartialDiagnostic(diag::err_module_file_conflict, + ContextObj->DiagAllocator) + << CurrentModule->getTopLevelModuleName() << CurFile->getName() + << F.File->getName(); + return DiagnosticError::create(CurrentImportLoc, ConflictError); } }