[cfe-commits] r132389 - in /cfe/trunk: include/clang/Serialization/ASTReader.h lib/Serialization/ASTReader.cpp test/PCH/modified-header-error.c
Ted Kremenek
kremenek at apple.com
Wed Jun 1 14:02:43 PDT 2011
I think we've seen these kinds of problems before with the Windows tester (race conditions involving the filesystem). I'd be fine with XFAILing it on Windows.
On Jun 1, 2011, at 1:33 PM, Eli Friedman wrote:
> On Tue, May 31, 2011 at 10:43 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>> Author: akirtzidis
>> Date: Wed Jun 1 00:43:53 2011
>> New Revision: 132389
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=132389&view=rev
>> Log:
>> [PCH] Be conservative and check all the files the PCH references to see if
>> a file was modified since the time the PCH was created.
>>
>> The parser is not fit to deal with stale PCHs, too many invariants do not hold up. rdar://9530587.
>>
>> Added:
>> cfe/trunk/test/PCH/modified-header-error.c
>> Modified:
>> cfe/trunk/include/clang/Serialization/ASTReader.h
>> cfe/trunk/lib/Serialization/ASTReader.cpp
>>
>> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=132389&r1=132388&r2=132389&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
>> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Jun 1 00:43:53 2011
>> @@ -789,6 +789,10 @@
>> /// \brief Reads a statement from the specified cursor.
>> Stmt *ReadStmtFromStream(PerFileData &F);
>>
>> + /// \brief Get a FileEntry out of stored-in-PCH filename, making sure we take
>> + /// into account all the necessary relocations.
>> + const FileEntry *getFileEntry(llvm::StringRef filename);
>> +
>> void MaybeAddSystemRootToFilename(std::string &Filename);
>>
>> ASTReadResult ReadASTCore(llvm::StringRef FileName, ASTFileType Type);
>> @@ -888,6 +892,10 @@
>> /// name.
>> ASTReadResult ReadAST(const std::string &FileName, ASTFileType 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();
>> +
>> /// \brief Set the AST callbacks listener.
>> void setListener(ASTReaderListener *listener) {
>> Listener.reset(listener);
>>
>> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=132389&r1=132388&r2=132389&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
>> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Jun 1 00:43:53 2011
>> @@ -1842,6 +1842,22 @@
>> return MacroDefinitionsLoaded[ID - 1];
>> }
>>
>> +const FileEntry *ASTReader::getFileEntry(llvm::StringRef filenameStrRef) {
>> + std::string Filename = filenameStrRef;
>> + MaybeAddSystemRootToFilename(Filename);
>> + const FileEntry *File = FileMgr.getFile(Filename);
>> + if (File == 0 && !OriginalDir.empty() && !CurrentDir.empty() &&
>> + OriginalDir != CurrentDir) {
>> + std::string resolved = resolveFileRelativeToOriginalDir(Filename,
>> + OriginalDir,
>> + CurrentDir);
>> + if (!resolved.empty())
>> + File = FileMgr.getFile(resolved);
>> + }
>> +
>> + return File;
>> +}
>> +
>> /// \brief If we are loading a relocatable PCH file, and the filename is
>> /// not an absolute path, add the system root to the beginning of the file
>> /// name.
>> @@ -2351,6 +2367,79 @@
>> return Failure;
>> }
>>
>> +ASTReader::ASTReadResult ASTReader::validateFileEntries() {
>> + for (unsigned CI = 0, CN = Chain.size(); CI != CN; ++CI) {
>> + PerFileData *F = Chain[CI];
>> + llvm::BitstreamCursor &SLocEntryCursor = F->SLocEntryCursor;
>> +
>> + for (unsigned i = 0, e = F->LocalNumSLocEntries; i != e; ++i) {
>> + SLocEntryCursor.JumpToBit(F->SLocOffsets[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: {
>> + llvm::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() < 6) {
>> + Error("source location entry is incorrect");
>> + return Failure;
>> + }
>> +
>> + // 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 (((off_t)Record[4] != 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.
>> + || (time_t)Record[5] != StatBuf.st_mtime
>> +#endif
>> + )) {
>> + Error(diag::err_fe_pch_file_modified, Filename);
>> + return IgnorePCH;
>> + }
>> +
>> + break;
>> + }
>> +
>> + case SM_SLOC_BUFFER_ENTRY:
>> + case SM_SLOC_INSTANTIATION_ENTRY:
>> + break;
>> + }
>> + }
>> + }
>> +
>> + return Success;
>> +}
>> +
>> ASTReader::ASTReadResult ASTReader::ReadAST(const std::string &FileName,
>> ASTFileType Type) {
>> switch(ReadASTCore(FileName, Type)) {
>> @@ -2361,6 +2450,14 @@
>>
>> // Here comes stuff that we only do once the entire chain is loaded.
>>
>> + if (!DisableValidation) {
>> + switch(validateFileEntries()) {
>> + case Failure: return Failure;
>> + case IgnorePCH: return IgnorePCH;
>> + case Success: break;
>> + }
>> + }
>> +
>> // Allocate space for loaded slocentries, identifiers, decls and types.
>> unsigned TotalNumIdentifiers = 0, TotalNumTypes = 0, TotalNumDecls = 0,
>> TotalNumPreallocatedPreprocessingEntities = 0, TotalNumMacroDefs = 0,
>>
>> Added: cfe/trunk/test/PCH/modified-header-error.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/modified-header-error.c?rev=132389&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/PCH/modified-header-error.c (added)
>> +++ cfe/trunk/test/PCH/modified-header-error.c Wed Jun 1 00:43:53 2011
>> @@ -0,0 +1,11 @@
>> +// RUN: mkdir -p %t.dir
>> +// RUN: echo '#include "header2.h"' > %t.dir/header1.h
>> +// RUN: echo > %t.dir/header2.h
>> +// RUN: cp %s %t.dir/t.c
>> +// RUN: %clang_cc1 -x c-header %t.dir/header1.h -emit-pch -o %t.pch
>> +// RUN: echo >> %t.dir/header2.h
>> +// RUN: %clang_cc1 %t.dir/t.c -include-pch %t.pch -fsyntax-only 2>&1 | FileCheck %s
>> +
>> +#include "header2.h"
>> +
>> +// CHECK: fatal error: file {{.*}} has been modified since the precompiled header was built
>
> This test appears to be failing intermittently on Windows; see
> http://smooshlab.apple.com:8013/builders/rewriter_i386-pc-win32/builds/1126/steps/buildit.clang/logs/stdio
> .
>
> -Eli
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list