From 745e6f168d0276c15fb72f3d90e3d93d60282b1b Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Fri, 19 Oct 2012 00:38:02 +0000 Subject: [PATCH] Move the set of files to be validated in an AST file into the control block, so the input files are validated early on, before we've committed to loading the AST file. This (accidentally) fixed a but wherein the main file used to generate the AST file would *not* be validated by the existing validation logic. At the moment, this leads to some duplication of filenames between the source manager block and input-file blocks, as well as validation logic. This will be handled via an upcoming patch. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@166251 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Serialization/ASTBitCodes.h | 41 ++-- include/clang/Serialization/ASTReader.h | 5 +- include/clang/Serialization/ASTWriter.h | 1 + include/clang/Serialization/Module.h | 7 - lib/Serialization/ASTReader.cpp | 240 ++++++++++++---------- lib/Serialization/ASTWriter.cpp | 74 +++++-- lib/Serialization/Module.cpp | 2 +- test/PCH/missing-file.cpp | 12 +- 8 files changed, 219 insertions(+), 163 deletions(-) diff --git a/include/clang/Serialization/ASTBitCodes.h b/include/clang/Serialization/ASTBitCodes.h index a22cac523b..988cac20b3 100644 --- a/include/clang/Serialization/ASTBitCodes.h +++ b/include/clang/Serialization/ASTBitCodes.h @@ -221,7 +221,13 @@ namespace clang { /// \brief The control block, which contains all of the /// information that needs to be validated prior to committing /// to loading the AST file. - CONTROL_BLOCK_ID + CONTROL_BLOCK_ID, + + /// \brief The block of input files, which were used as inputs + /// to create this AST file. + /// + /// This block is part of the control block. + INPUT_FILES_BLOCK_ID }; /// \brief Record types that occur within the control block. @@ -254,6 +260,13 @@ namespace clang { ORIGINAL_PCH_DIR = 6 }; + /// \brief Record types that occur within the input-files block + /// inside the control block. + enum InputFileRecordTypes { + /// \brief An input file. + INPUT_FILE = 1 + }; + /// \brief Record types that occur within the AST block itself. enum ASTRecordTypes { /// \brief Record code for the offsets of each type. @@ -439,61 +452,57 @@ namespace clang { /// \brief The list of delegating constructor declarations. DELEGATING_CTORS = 37, - /// \brief Record code for the table of offsets into the block - /// of file source-location information. - FILE_SOURCE_LOCATION_OFFSETS = 38, - /// \brief Record code for the set of known namespaces, which are used /// for typo correction. - KNOWN_NAMESPACES = 39, + KNOWN_NAMESPACES = 38, /// \brief Record code for the remapping information used to relate /// loaded modules to the various offsets and IDs(e.g., source location /// offests, declaration and type IDs) that are used in that module to /// refer to other modules. - MODULE_OFFSET_MAP = 40, + MODULE_OFFSET_MAP = 39, /// \brief Record code for the source manager line table information, /// which stores information about \#line directives. - SOURCE_MANAGER_LINE_TABLE = 41, + SOURCE_MANAGER_LINE_TABLE = 40, /// \brief Record code for map of Objective-C class definition IDs to the /// ObjC categories in a module that are attached to that class. - OBJC_CATEGORIES_MAP = 42, + OBJC_CATEGORIES_MAP = 41, /// \brief Record code for a file sorted array of DeclIDs in a module. - FILE_SORTED_DECLS = 43, + FILE_SORTED_DECLS = 42, /// \brief Record code for an array of all of the (sub)modules that were /// imported by the AST file. - IMPORTED_MODULES = 44, + IMPORTED_MODULES = 43, /// \brief Record code for the set of merged declarations in an AST file. - MERGED_DECLARATIONS = 45, + MERGED_DECLARATIONS = 44, /// \brief Record code for the array of redeclaration chains. /// /// This array can only be interpreted properly using the local /// redeclarations map. - LOCAL_REDECLARATIONS = 46, + LOCAL_REDECLARATIONS = 45, /// \brief Record code for the array of Objective-C categories (including /// extensions). /// /// This array can only be interpreted properly using the Objective-C /// categories map. - OBJC_CATEGORIES = 47, + OBJC_CATEGORIES = 46, /// \brief Record code for the table of offsets of each macro ID. /// /// The offset table contains offsets into the blob stored in /// the preprocessor block. Each offset points to the corresponding /// macro definition. - MACRO_OFFSET = 48, + MACRO_OFFSET = 47, /// \brief Record of updates for a macro that was modified after /// being deserialized. - MACRO_UPDATES = 49 + MACRO_UPDATES = 48 }; /// \brief Record types used within a source manager block. diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index 7d9792e0fe..03ec200035 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/include/clang/Serialization/ASTReader.h @@ -871,6 +871,7 @@ private: llvm::SmallVectorImpl &Loaded); ASTReadResult ReadControlBlock(ModuleFile &F, llvm::SmallVectorImpl &Loaded); + ASTReadResult ReadInputFilesBlock(ModuleFile &F); ASTReadResult ReadASTBlock(ModuleFile &F); bool CheckPredefinesBuffers(); bool ParseLineTable(ModuleFile &F, SmallVectorImpl &Record); @@ -1041,10 +1042,6 @@ public: /// \brief Load the AST file designated by the given file name. ASTReadResult ReadAST(const std::string &FileName, ModuleKind Type); - /// \brief Checks that no file that is stored in PCH is out-of-sync with - /// the actual file in the file system. - ASTReadResult validateFileEntries(ModuleFile &M); - /// \brief Make the entities in the given module and any of its (non-explicit) /// submodules visible to name lookup. /// diff --git a/include/clang/Serialization/ASTWriter.h b/include/clang/Serialization/ASTWriter.h index 3fb754dca9..37cf1b3c36 100644 --- a/include/clang/Serialization/ASTWriter.h +++ b/include/clang/Serialization/ASTWriter.h @@ -411,6 +411,7 @@ private: void WriteBlockInfoBlock(); void WriteControlBlock(ASTContext &Context, StringRef isysroot, const std::string &OutputFile); + void WriteInputFiles(SourceManager &SourceMgr, StringRef isysroot); void WriteStatCache(MemorizeStatCalls &StatCalls); void WriteSourceManagerBlock(SourceManager &SourceMgr, const Preprocessor &PP, diff --git a/include/clang/Serialization/Module.h b/include/clang/Serialization/Module.h index 30bed03b8a..a1a47d8b04 100644 --- a/include/clang/Serialization/Module.h +++ b/include/clang/Serialization/Module.h @@ -149,13 +149,6 @@ public: /// \brief SLocEntries that we're going to preload. SmallVector PreloadSLocEntries; - /// \brief The number of source location file entries in this AST file. - unsigned LocalNumSLocFileEntries; - - /// \brief Offsets for all of the source location file entries in the - /// AST file. - const uint32_t *SLocFileOffsets; - /// \brief Remapping table for source locations in this module. ContinuousRangeMap SLocRemap; diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index fe2affb91e..a0bfa10b85 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -1772,9 +1772,28 @@ ASTReader::ASTReadResult ASTReader::ReadControlBlock(ModuleFile &F, } if (Code == llvm::bitc::ENTER_SUBBLOCK) { - Stream.ReadSubBlockID(); - if (!Stream.SkipBlock()) + switch (Stream.ReadSubBlockID()) { + case INPUT_FILES_BLOCK_ID: + switch (ReadInputFilesBlock(F)) { + case Success: + break; + + case Failure: + return Failure; + + case IgnorePCH: + // FIXME: We could consider reading through to the end of this + // AST block, skipping subblocks, to see if there are other + // AST blocks elsewhere. + return IgnorePCH; + } continue; + + default: + if (!Stream.SkipBlock()) + continue; + break; + } Error("malformed block record in AST file"); return Failure; @@ -1882,6 +1901,117 @@ ASTReader::ASTReadResult ASTReader::ReadControlBlock(ModuleFile &F, return Failure; } +ASTReader::ASTReadResult ASTReader::ReadInputFilesBlock(ModuleFile &F) { + llvm::BitstreamCursor &Stream = F.Stream; + + // If we're not doing any validation, there's no reason to read this + // input-files block. + if (DisableValidation) { + return Stream.SkipBlock()? Failure : Success; + } + + if (Stream.EnterSubBlock(INPUT_FILES_BLOCK_ID)) { + Error("malformed block record in AST file"); + return Failure; + } + + // Read all of the records and blocks for the AST file. + RecordData Record; + while (!Stream.AtEndOfStream()) { + unsigned Code = Stream.ReadCode(); + if (Code == llvm::bitc::END_BLOCK) { + if (Stream.ReadBlockEnd()) { + Error("error at end of module block in AST file"); + return Failure; + } + + DeclContext *DC = Context.getTranslationUnitDecl(); + if (!DC->hasExternalVisibleStorage() && DC->hasExternalLexicalStorage()) + DC->setMustBuildLookupTable(); + + return Success; + } + + if (Code == llvm::bitc::ENTER_SUBBLOCK) { + Stream.ReadSubBlockID(); + if (!Stream.SkipBlock()) + continue; + Error("malformed block record in AST file"); + return Failure; + } + + if (Code == llvm::bitc::DEFINE_ABBREV) { + Stream.ReadAbbrevRecord(); + continue; + } + + // Read and process a record. + Record.clear(); + const char *BlobStart = 0; + unsigned BlobLen = 0; + switch ((InputFileRecordTypes)Stream.ReadRecord(Code, Record, + &BlobStart, &BlobLen)) { + case INPUT_FILE: { + // Check that the file exists. + StringRef Filename(BlobStart, BlobLen); + const FileEntry *File = getFileEntry(Filename); + if (File == 0) { + std::string ErrorStr = "could not find file '"; + ErrorStr += Filename; + ErrorStr += "' referenced by AST file"; + Error(ErrorStr.c_str()); + return IgnorePCH; + } + + off_t StoredSize = (off_t)Record[0]; + time_t StoredTime = (time_t)Record[1]; + + // Check if there was a request to override the contents of the file + // that was part of the precompiled header. Overridding such a file + // can lead to problems when lexing using the source locations from the + // PCH. + SourceManager &SM = getSourceManager(); + if (SM.isFileOverridden(File)) { + Error(diag::err_fe_pch_file_overridden, Filename); + // After emitting the diagnostic, recover by disabling the override so + // that the original file will be used. + SM.disableFileContentsOverride(File); + // The FileEntry is a virtual file entry with the size of the contents + // that would override the original contents. Set it to the original's + // size/time. + FileMgr.modifyFileEntry(const_cast(File), + StoredSize, StoredTime); + } + + // The stat info from the FileEntry came from the cached stat + // info of the PCH, so we cannot trust it. + struct stat StatBuf; + if (::stat(File->getName(), &StatBuf) != 0) { + StatBuf.st_size = File->getSize(); + StatBuf.st_mtime = File->getModificationTime(); + } + + if ((StoredSize != StatBuf.st_size +#if !defined(LLVM_ON_WIN32) + // In our regression testing, the Windows file system seems to + // have inconsistent modification times that sometimes + // erroneously trigger this error-handling path. + || StoredTime != StatBuf.st_mtime +#endif + )) { + Error(diag::err_fe_pch_file_modified, Filename); + return IgnorePCH; + } + + break; + } + } + } + + Error("premature end of bitstream in AST file"); + return Failure; +} + ASTReader::ASTReadResult ASTReader::ReadASTBlock(ModuleFile &F) { llvm::BitstreamCursor &Stream = F.Stream; @@ -2356,11 +2486,6 @@ ASTReader::ReadASTBlock(ModuleFile &F) { return Failure; break; - case FILE_SOURCE_LOCATION_OFFSETS: - F.SLocFileOffsets = (const uint32_t *)BlobStart; - F.LocalNumSLocFileEntries = Record[0]; - 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. @@ -2665,96 +2790,6 @@ ASTReader::ReadASTBlock(ModuleFile &F) { return Failure; } -ASTReader::ASTReadResult ASTReader::validateFileEntries(ModuleFile &M) { - llvm::BitstreamCursor &SLocEntryCursor = M.SLocEntryCursor; - - for (unsigned i = 0, e = M.LocalNumSLocFileEntries; i != e; ++i) { - SLocEntryCursor.JumpToBit(M.SLocFileOffsets[i]); - unsigned Code = SLocEntryCursor.ReadCode(); - if (Code == llvm::bitc::END_BLOCK || - Code == llvm::bitc::ENTER_SUBBLOCK || - Code == llvm::bitc::DEFINE_ABBREV) { - Error("incorrectly-formatted source location entry in AST file"); - return Failure; - } - - RecordData Record; - const char *BlobStart; - unsigned BlobLen; - switch (SLocEntryCursor.ReadRecord(Code, Record, &BlobStart, &BlobLen)) { - default: - Error("incorrectly-formatted source location entry in AST file"); - return Failure; - - case SM_SLOC_FILE_ENTRY: { - // If the buffer was overridden, the file need not exist. - if (Record[6]) - break; - - StringRef Filename(BlobStart, BlobLen); - const FileEntry *File = getFileEntry(Filename); - - if (File == 0) { - std::string ErrorStr = "could not find file '"; - ErrorStr += Filename; - ErrorStr += "' referenced by AST file"; - Error(ErrorStr.c_str()); - return IgnorePCH; - } - - if (Record.size() < 7) { - Error("source location entry is incorrect"); - return Failure; - } - - off_t StoredSize = (off_t)Record[4]; - time_t StoredTime = (time_t)Record[5]; - - // Check if there was a request to override the contents of the file - // that was part of the precompiled header. Overridding such a file - // can lead to problems when lexing using the source locations from the - // PCH. - SourceManager &SM = getSourceManager(); - if (SM.isFileOverridden(File)) { - Error(diag::err_fe_pch_file_overridden, Filename); - // After emitting the diagnostic, recover by disabling the override so - // that the original file will be used. - SM.disableFileContentsOverride(File); - // The FileEntry is a virtual file entry with the size of the contents - // that would override the original contents. Set it to the original's - // size/time. - FileMgr.modifyFileEntry(const_cast(File), - StoredSize, StoredTime); - } - - // The stat info from the FileEntry came from the cached stat - // info of the PCH, so we cannot trust it. - struct stat StatBuf; - if (::stat(File->getName(), &StatBuf) != 0) { - StatBuf.st_size = File->getSize(); - StatBuf.st_mtime = File->getModificationTime(); - } - - if ((StoredSize != StatBuf.st_size -#if !defined(LLVM_ON_WIN32) - // In our regression testing, the Windows file system seems to - // have inconsistent modification times that sometimes - // erroneously trigger this error-handling path. - || StoredTime != StatBuf.st_mtime -#endif - )) { - Error(diag::err_fe_pch_file_modified, Filename); - return IgnorePCH; - } - - break; - } - } - } - - return Success; -} - void ASTReader::makeNamesVisible(const HiddenNames &Names) { for (unsigned I = 0, N = Names.size(); I != N; ++I) { switch (Names[I].getKind()) { @@ -2917,17 +2952,6 @@ ASTReader::ASTReadResult ASTReader::ReadAST(const std::string &FileName, TotalModulesSizeInBits += F.SizeInBits; GlobalBitOffsetsMap.insert(std::make_pair(F.GlobalBitOffset, &F)); - // Make sure that the files this module was built against are - // still available. - // FIXME: Move this validation into ReadControlBlock. - if (!DisableValidation) { - switch(validateFileEntries(F)) { - case Failure: return Failure; - case IgnorePCH: return IgnorePCH; - case Success: break; - } - } - // Preload SLocEntries. for (unsigned I = 0, N = F.PreloadSLocEntries.size(); I != N; ++I) { int Index = int(F.PreloadSLocEntries[I] - 1) + F.SLocEntryBaseID; diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index ea3db882d7..adbca78d97 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -813,7 +813,6 @@ void ASTWriter::WriteBlockInfoBlock() { RECORD(FP_PRAGMA_OPTIONS); RECORD(OPENCL_EXTENSIONS); RECORD(DELEGATING_CTORS); - RECORD(FILE_SOURCE_LOCATION_OFFSETS); RECORD(KNOWN_NAMESPACES); RECORD(MODULE_OFFSET_MAP); RECORD(SOURCE_MANAGER_LINE_TABLE); @@ -1105,6 +1104,63 @@ void ASTWriter::WriteControlBlock(ASTContext &Context, StringRef isysroot, Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir); } + WriteInputFiles(Context.SourceMgr, isysroot); + Stream.ExitBlock(); +} + +void ASTWriter::WriteInputFiles(SourceManager &SourceMgr, StringRef isysroot) { + using namespace llvm; + Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4); + RecordData Record; + + // Create input-file abbreviation. + BitCodeAbbrev *IFAbbrev = new BitCodeAbbrev(); + IFAbbrev->Add(BitCodeAbbrevOp(INPUT_FILE)); + IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 12)); // Size + IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 32)); // Modification time + IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // File name + unsigned IFAbbrevCode = Stream.EmitAbbrev(IFAbbrev); + + // Write out all of the input files. + std::vector InputFileOffsets; + for (unsigned I = 1, N = SourceMgr.local_sloc_entry_size(); I != N; ++I) { + // Get this source location entry. + const SrcMgr::SLocEntry *SLoc = &SourceMgr.getLocalSLocEntry(I); + FileID FID = FileID::get(I); + assert(&SourceMgr.getSLocEntry(FID) == SLoc); + + // We only care about file entries that were not overridden. + if (!SLoc->isFile()) + continue; + const SrcMgr::ContentCache *Cache = SLoc->getFile().getContentCache(); + if (!Cache->OrigEntry || Cache->BufferOverridden) + continue; + + Record.clear(); + Record.push_back(INPUT_FILE); + + // Emit size/modification time for this file. + Record.push_back(Cache->OrigEntry->getSize()); + Record.push_back(Cache->OrigEntry->getModificationTime()); + + // Turn the file name into an absolute path, if it isn't already. + const char *Filename = Cache->OrigEntry->getName(); + SmallString<128> FilePath(Filename); + + // Ask the file manager to fixup the relative path for us. This will + // honor the working directory. + SourceMgr.getFileManager().FixupRelativePath(FilePath); + + // FIXME: This call to make_absolute shouldn't be necessary, the + // call to FixupRelativePath should always return an absolute path. + llvm::sys::fs::make_absolute(FilePath); + Filename = FilePath.c_str(); + + Filename = adjustFilenameForRelocatablePCH(Filename, isysroot); + + Stream.EmitRecordWithBlob(IFAbbrevCode, Record, Filename); + } + Stream.ExitBlock(); } @@ -1442,9 +1498,6 @@ void ASTWriter::WriteSourceManagerBlock(SourceManager &SourceMgr, // Write out the source location entry table. We skip the first // entry, which is always the same dummy entry. std::vector SLocEntryOffsets; - // Write out the offsets of only source location file entries. - // We will go through them in ASTReader::validateFileEntries(). - std::vector SLocFileEntryOffsets; RecordData PreloadSLocs; SLocEntryOffsets.reserve(SourceMgr.local_sloc_entry_size() - 1); for (unsigned I = 1, N = SourceMgr.local_sloc_entry_size(); @@ -1463,7 +1516,6 @@ void ASTWriter::WriteSourceManagerBlock(SourceManager &SourceMgr, const SrcMgr::ContentCache *Cache = SLoc->getFile().getContentCache(); if (Cache->OrigEntry) { Code = SM_SLOC_FILE_ENTRY; - SLocFileEntryOffsets.push_back(Stream.GetCurrentBitNo()); } else Code = SM_SLOC_BUFFER_ENTRY; } else @@ -1587,18 +1639,6 @@ void ASTWriter::WriteSourceManagerBlock(SourceManager &SourceMgr, Record.push_back(SourceMgr.getNextLocalOffset() - 1); // skip dummy Stream.EmitRecordWithBlob(SLocOffsetsAbbrev, Record, data(SLocEntryOffsets)); - Abbrev = new BitCodeAbbrev(); - Abbrev->Add(BitCodeAbbrevOp(FILE_SOURCE_LOCATION_OFFSETS)); - Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 16)); // # of slocs - Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // offsets - unsigned SLocFileOffsetsAbbrev = Stream.EmitAbbrev(Abbrev); - - Record.clear(); - Record.push_back(FILE_SOURCE_LOCATION_OFFSETS); - Record.push_back(SLocFileEntryOffsets.size()); - Stream.EmitRecordWithBlob(SLocFileOffsetsAbbrev, Record, - data(SLocFileEntryOffsets)); - // Write the source location entry preloads array, telling the AST // reader which source locations entries it should load eagerly. Stream.EmitRecord(SOURCE_LOCATION_PRELOADS, PreloadSLocs); diff --git a/lib/Serialization/Module.cpp b/lib/Serialization/Module.cpp index b6204f9478..09ea754d44 100644 --- a/lib/Serialization/Module.cpp +++ b/lib/Serialization/Module.cpp @@ -25,7 +25,7 @@ ModuleFile::ModuleFile(ModuleKind Kind, unsigned Generation) Generation(Generation), SizeInBits(0), LocalNumSLocEntries(0), SLocEntryBaseID(0), SLocEntryBaseOffset(0), SLocEntryOffsets(0), - SLocFileOffsets(0), LocalNumIdentifiers(0), + LocalNumIdentifiers(0), IdentifierOffsets(0), BaseIdentifierID(0), IdentifierTableData(0), IdentifierLookupTable(0), LocalNumMacros(0), MacroOffsets(0), diff --git a/test/PCH/missing-file.cpp b/test/PCH/missing-file.cpp index 8edaaea79d..7dd11d4561 100644 --- a/test/PCH/missing-file.cpp +++ b/test/PCH/missing-file.cpp @@ -10,16 +10,8 @@ // RUN: rm %t.h || rm %t.h || rm %t.h // Check diagnostic with location in original source: -// RUN: %clang_cc1 -include-pch %t.h.pch -Wpadded -emit-obj -o %t.o %s 2> %t.stderr -// RUN: grep 'bytes to align' %t.stderr - -// Check diagnostic with 2nd location in original source: -// RUN: not %clang_cc1 -DREDECL -include-pch %t.h.pch -emit-obj -o %t.o %s 2> %t.stderr -// RUN: grep 'previous definition is here' %t.stderr - -// Check diagnostic with instantiation location in original source: -// RUN: not %clang_cc1 -DINSTANTIATION -include-pch %t.h.pch -emit-obj -o %t.o %s 2> %t.stderr -// RUN: grep 'cannot be used prior to' %t.stderr +// RUN: not %clang_cc1 -include-pch %t.h.pch -emit-obj -o %t.o %s 2> %t.stderr +// RUN: grep 'could not find file' %t.stderr void qq(S*) {} -- 2.40.0