[cfe-commits] r132389 - in /cfe/trunk: include/clang/Serialization/ASTReader.h lib/Serialization/ASTReader.cpp test/PCH/modified-header-error.c

[Eli Friedman]

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