From d54dff026b02303a35147224de72bb44cbb53c79 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Thu, 3 May 2012 21:50:39 +0000 Subject: [PATCH] [PCH] When validating that the files coming from PCH did not change, also validate that we didn't override the contents of any of such files. If this is detected, emit a diagnostic error and recover gracefully by using the contents of the original file that the PCH was built from. Part of rdar://11305263 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@156107 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Basic/DiagnosticSerializationKinds.td | 2 + include/clang/Basic/FileManager.h | 5 ++ include/clang/Basic/SourceManager.h | 38 +++++++++++++- lib/Basic/FileManager.cpp | 6 +++ lib/Basic/SourceManager.cpp | 51 ++++++++++++++----- lib/Serialization/ASTReader.cpp | 24 ++++++++- test/PCH/remap-file-from-pch.cpp | 10 ++++ test/PCH/remap-file-from-pch.cpp.h | 2 + test/PCH/remap-file-from-pch.cpp.remap.h | 4 ++ 9 files changed, 124 insertions(+), 18 deletions(-) create mode 100644 test/PCH/remap-file-from-pch.cpp create mode 100644 test/PCH/remap-file-from-pch.cpp.h create mode 100644 test/PCH/remap-file-from-pch.cpp.remap.h diff --git a/include/clang/Basic/DiagnosticSerializationKinds.td b/include/clang/Basic/DiagnosticSerializationKinds.td index 7f9fe262f7..6e27f3490f 100644 --- a/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/include/clang/Basic/DiagnosticSerializationKinds.td @@ -22,6 +22,8 @@ def err_fe_pch_error_at_end_block : Error< def err_fe_pch_file_modified : Error< "file '%0' has been modified since the precompiled header was built">, DefaultFatal; +def err_fe_pch_file_overridden : Error< + "file '%0' from the precompiled header has been overridden">; def warn_pch_target_triple : Error< "PCH file was compiled for the target '%0' but the current translation " diff --git a/include/clang/Basic/FileManager.h b/include/clang/Basic/FileManager.h index 5c7d9eba4a..94487f31bd 100644 --- a/include/clang/Basic/FileManager.h +++ b/include/clang/Basic/FileManager.h @@ -226,6 +226,11 @@ public: void GetUniqueIDMapping( SmallVectorImpl &UIDToFiles) const; + /// \brief Modifies the size and modification time of a previously created + /// FileEntry. Use with caution. + static void modifyFileEntry(FileEntry *File, off_t Size, + time_t ModificationTime); + void PrintStats() const; }; diff --git a/include/clang/Basic/SourceManager.h b/include/clang/Basic/SourceManager.h index bcb2d561a4..3164f874f1 100644 --- a/include/clang/Basic/SourceManager.h +++ b/include/clang/Basic/SourceManager.h @@ -21,7 +21,9 @@ #include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/OwningPtr.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/Support/MemoryBuffer.h" #include #include @@ -501,9 +503,24 @@ class SourceManager : public RefCountedBase { /// files, should report the original file name. Defaults to true. bool OverridenFilesKeepOriginalName; - /// \brief Files that have been overriden with the contents from another file. - llvm::DenseMap OverriddenFiles; + struct OverriddenFilesInfoTy { + /// \brief Files that have been overriden with the contents from another + /// file. + llvm::DenseMap OverriddenFiles; + /// \brief Files that were overridden with a memory buffer. + llvm::DenseSet OverriddenFilesWithBuffer; + }; + + /// \brief Lazily create the object keeping overridden files info, since + /// it is uncommonly used. + OwningPtr OverriddenFilesInfo; + OverriddenFilesInfoTy &getOverriddenFilesInfo() { + if (!OverriddenFilesInfo) + OverriddenFilesInfo.reset(new OverriddenFilesInfoTy); + return *OverriddenFilesInfo; + } + /// MemBufferInfos - Information about various memory buffers that we have /// read in. All FileEntry* within the stored ContentCache objects are NULL, /// as they do not refer to a file. @@ -715,6 +732,23 @@ public: void overrideFileContents(const FileEntry *SourceFile, const FileEntry *NewFile); + /// \brief Returns true if the file contents have been overridden. + bool isFileOverridden(const FileEntry *File) { + if (OverriddenFilesInfo) { + if (OverriddenFilesInfo->OverriddenFilesWithBuffer.count(File)) + return true; + if (OverriddenFilesInfo->OverriddenFiles.find(File) != + OverriddenFilesInfo->OverriddenFiles.end()) + return true; + } + return false; + } + + /// \brief Disable overridding the contents of a file, previously enabled + /// with \see overrideFileContents. + /// This should be called before parsing has begun. + void disableFileContentsOverride(const FileEntry *File); + //===--------------------------------------------------------------------===// // FileID manipulation methods. //===--------------------------------------------------------------------===// diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp index fd6d33463a..669bf7d4c7 100644 --- a/lib/Basic/FileManager.cpp +++ b/lib/Basic/FileManager.cpp @@ -584,6 +584,12 @@ void FileManager::GetUniqueIDMapping( UIDToFiles[(*VFE)->getUID()] = *VFE; } +void FileManager::modifyFileEntry(FileEntry *File, + off_t Size, time_t ModificationTime) { + File->Size = Size; + File->ModTime = ModificationTime; +} + void FileManager::PrintStats() const { llvm::errs() << "\n*** File Manager Stats:\n"; diff --git a/lib/Basic/SourceManager.cpp b/lib/Basic/SourceManager.cpp index cef091c598..ed920eb488 100644 --- a/lib/Basic/SourceManager.cpp +++ b/lib/Basic/SourceManager.cpp @@ -71,7 +71,7 @@ unsigned ContentCache::getSize() const { void ContentCache::replaceBuffer(const llvm::MemoryBuffer *B, bool DoNotFree) { - if (B == Buffer.getPointer()) { + if (B && B == Buffer.getPointer()) { assert(0 && "Replacing with the same buffer"); Buffer.setInt(DoNotFree? DoNotFreeFlag : 0); return; @@ -440,16 +440,20 @@ SourceManager::getOrCreateContentCache(const FileEntry *FileEnt) { EntryAlign = std::max(8U, EntryAlign); Entry = ContentCacheAlloc.Allocate(1, EntryAlign); - // If the file contents are overridden with contents from another file, - // pass that file to ContentCache. - llvm::DenseMap::iterator - overI = OverriddenFiles.find(FileEnt); - if (overI == OverriddenFiles.end()) + if (OverriddenFilesInfo) { + // If the file contents are overridden with contents from another file, + // pass that file to ContentCache. + llvm::DenseMap::iterator + overI = OverriddenFilesInfo->OverriddenFiles.find(FileEnt); + if (overI == OverriddenFilesInfo->OverriddenFiles.end()) + new (Entry) ContentCache(FileEnt); + else + new (Entry) ContentCache(OverridenFilesKeepOriginalName ? FileEnt + : overI->second, + overI->second); + } else { new (Entry) ContentCache(FileEnt); - else - new (Entry) ContentCache(OverridenFilesKeepOriginalName ? FileEnt - : overI->second, - overI->second); + } return Entry; } @@ -622,6 +626,8 @@ void SourceManager::overrideFileContents(const FileEntry *SourceFile, const_cast(IR)->replaceBuffer(Buffer, DoNotFree); const_cast(IR)->BufferOverridden = true; + + getOverriddenFilesInfo().OverriddenFilesWithBuffer.insert(SourceFile); } void SourceManager::overrideFileContents(const FileEntry *SourceFile, @@ -632,7 +638,20 @@ void SourceManager::overrideFileContents(const FileEntry *SourceFile, assert(FileInfos.count(SourceFile) == 0 && "This function should be called at the initialization stage, before " "any parsing occurs."); - OverriddenFiles[SourceFile] = NewFile; + getOverriddenFilesInfo().OverriddenFiles[SourceFile] = NewFile; +} + +void SourceManager::disableFileContentsOverride(const FileEntry *File) { + if (!isFileOverridden(File)) + return; + + const SrcMgr::ContentCache *IR = getOrCreateContentCache(File); + const_cast(IR)->replaceBuffer(0); + const_cast(IR)->ContentsEntry = IR->OrigEntry; + + assert(OverriddenFilesInfo); + OverriddenFilesInfo->OverriddenFiles.erase(File); + OverriddenFilesInfo->OverriddenFilesWithBuffer.erase(File); } StringRef SourceManager::getBufferData(FileID FID, bool *Invalid) const { @@ -1887,10 +1906,14 @@ SourceManager::MemoryBufferSizes SourceManager::getMemoryBufferSizes() const { } size_t SourceManager::getDataStructureSizes() const { - return llvm::capacity_in_bytes(MemBufferInfos) + size_t size = llvm::capacity_in_bytes(MemBufferInfos) + llvm::capacity_in_bytes(LocalSLocEntryTable) + llvm::capacity_in_bytes(LoadedSLocEntryTable) + llvm::capacity_in_bytes(SLocEntryLoaded) - + llvm::capacity_in_bytes(FileInfos) - + llvm::capacity_in_bytes(OverriddenFiles); + + llvm::capacity_in_bytes(FileInfos); + + if (OverriddenFilesInfo) + size += llvm::capacity_in_bytes(OverriddenFilesInfo->OverriddenFiles); + + return size; } diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index f2240561fa..f1ea091627 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -2473,6 +2473,26 @@ ASTReader::ASTReadResult ASTReader::validateFileEntries(ModuleFile &M) { 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. @@ -2482,12 +2502,12 @@ ASTReader::ASTReadResult ASTReader::validateFileEntries(ModuleFile &M) { StatBuf.st_mtime = File->getModificationTime(); } - if (((off_t)Record[4] != StatBuf.st_size + 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. - || (time_t)Record[5] != StatBuf.st_mtime + || StoredTime != StatBuf.st_mtime #endif )) { Error(diag::err_fe_pch_file_modified, Filename); diff --git a/test/PCH/remap-file-from-pch.cpp b/test/PCH/remap-file-from-pch.cpp new file mode 100644 index 0000000000..f8a4cbe5c6 --- /dev/null +++ b/test/PCH/remap-file-from-pch.cpp @@ -0,0 +1,10 @@ +// %clang_cc1 -remap-file "%s;%S/Inputs/remapped-file" -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-EXIST %s + +// RUN: %clang_cc1 -x c++-header %s.h -emit-pch -o %t.pch +// RUN: %clang_cc1 %s -include-pch %t.pch -remap-file "%s.h;%s.remap.h" -fsyntax-only 2>&1 | FileCheck %s + +const char *str = STR; +int ge = zool; + +// CHECK: file '{{.*}}/remap-file-from-pch.cpp.h' from the precompiled header has been overridden +// CHECK: use of undeclared identifier 'zool' diff --git a/test/PCH/remap-file-from-pch.cpp.h b/test/PCH/remap-file-from-pch.cpp.h new file mode 100644 index 0000000000..0edf6aa7cd --- /dev/null +++ b/test/PCH/remap-file-from-pch.cpp.h @@ -0,0 +1,2 @@ + +#define STR "nexus" diff --git a/test/PCH/remap-file-from-pch.cpp.remap.h b/test/PCH/remap-file-from-pch.cpp.remap.h new file mode 100644 index 0000000000..e8e8ab7e4b --- /dev/null +++ b/test/PCH/remap-file-from-pch.cpp.remap.h @@ -0,0 +1,4 @@ + +#define STR "nexus" + +int zool; -- 2.40.0