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 <[email protected]> > 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
