ASTReader: Bypass overridden files when reading PCHs

If contents of a file that is part of a PCM are overridden when reading
it, but weren't overridden when the PCM was being built, the ASTReader
will emit an error.  Now it creates a separate FileEntry for recovery,
bypassing the overridden content instead of discarding it.  The
pre-existing testcase clang/test/PCH/remap-file-from-pch.cpp confirms
that the new recovery method works correctly.

This resolves a long-standing FIXME to avoid hypothetically invalidating
another precompiled module that's already using the overridden contents.

This also removes ContentCache-related API that would be unsafe to use
across `CompilerInstance`s in an implicit modules build.  This helps to
unblock us sinking it from SourceManager into FileManager in the future,
which would allow us to delete `InMemoryModuleCache`.

https://reviews.llvm.org/D66710

llvm-svn: 370546

6 files changed, 116 insertions, 37 deletions

diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index a1d9bf7..a47ac12 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -116,6 +116,8 @@ public:
 
   const StringRef getName() const { return Name; }
 
+  bool isValid() const { return Entry->isValid(); }
+
   const FileEntry &getFileEntry() const { return *Entry; }
 
   off_t getSize() const { return Entry->getSize(); }
@@ -128,6 +130,13 @@ public:
 
   time_t getModificationTime() const { return Entry->getModificationTime(); }
 
+  friend bool operator==(const FileEntryRef &LHS, const FileEntryRef &RHS) {
+    return LHS.Entry == RHS.Entry && LHS.Name == RHS.Name;
+  }
+  friend bool operator!=(const FileEntryRef &LHS, const FileEntryRef &RHS) {
+    return !(LHS == RHS);
+  }
+
 private:
   StringRef Name;
   const FileEntry *Entry;
@@ -158,6 +167,10 @@ class FileManager : public RefCountedBase<FileManager> {
   /// The virtual files that we have allocated.
   SmallVector<std::unique_ptr<FileEntry>, 4> VirtualFileEntries;
 
+  /// A set of files that bypass the maps and uniquing.  They can have
+  /// conflicting filenames.
+  SmallVector<std::unique_ptr<FileEntry>, 0> BypassFileEntries;
+
   /// A cache that maps paths to directory entries (either real or
   /// virtual) we have looked up, or an error that occurred when we looked up
   /// the directory.
@@ -314,6 +327,16 @@ public:
   const FileEntry *getVirtualFile(StringRef Filename, off_t Size,
                                   time_t ModificationTime);
 
+  /// Retrieve a FileEntry that bypasses VFE, which is expected to be a virtual
+  /// file entry, to access the real file.  The returned FileEntry will have
+  /// the same filename as FE but a different identity and its own stat.
+  ///
+  /// This should be used only for rare error recovery paths because it
+  /// bypasses all mapping and uniquing, blindly creating a new FileEntry.
+  /// There is no attempt to deduplicate these; if you bypass the same file
+  /// twice, you get two new file entries.
+  llvm::Optional<FileEntryRef> getBypassFile(FileEntryRef VFE);
+
   /// Open the specified file as a MemoryBuffer, returning a new
   /// MemoryBuffer if successful, otherwise returning null.
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
@@ -353,11 +376,6 @@ public:
   void GetUniqueIDMapping(
                     SmallVectorImpl<const FileEntry *> &UIDToFiles) const;
 
-  /// 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);
-
   /// Retrieve the canonical name for a given directory.
   ///
   /// This is a very expensive operation, despite its results being cached,
diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index 3176f44..3185ca0 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -952,11 +952,12 @@ public:
     return false;
   }
 
-  /// Disable overridding the contents of a file, previously enabled
-  /// with #overrideFileContents.
+  /// Bypass the overridden contents of a file.  This creates a new FileEntry
+  /// and initializes the content cache for it.  Returns nullptr if there is no
+  /// such file in the filesystem.
   ///
   /// This should be called before parsing has begun.
-  void disableFileContentsOverride(const FileEntry *File);
+  const FileEntry *bypassFileContentsOverride(const FileEntry &File);
 
   /// Specify that a file is transient.
   void setFileIsTransient(const FileEntry *SourceFile);
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index 4330c7a..cc6c261 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -390,6 +390,25 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
   return UFE;
 }
 
+llvm::Optional<FileEntryRef> FileManager::getBypassFile(FileEntryRef VF) {
+  // Stat of the file and return nullptr if it doesn't exist.
+  llvm::vfs::Status Status;
+  if (getStatValue(VF.getName(), Status, /*isFile=*/true, /*F=*/nullptr))
+    return None;
+
+  // Fill it in from the stat.
+  BypassFileEntries.push_back(std::make_unique<FileEntry>());
+  const FileEntry &VFE = VF.getFileEntry();
+  FileEntry &BFE = *BypassFileEntries.back();
+  BFE.Name = VFE.getName();
+  BFE.Size = Status.getSize();
+  BFE.Dir = VFE.Dir;
+  BFE.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
+  BFE.UID = NextFileUID++;
+  BFE.IsValid = true;
+  return FileEntryRef(VF.getName(), BFE);
+}
+
 bool FileManager::FixupRelativePath(SmallVectorImpl<char> &path) const {
   StringRef pathRef(path.data(), path.size());
 
@@ -515,12 +534,6 @@ void FileManager::GetUniqueIDMapping(
     UIDToFiles[VFE->getUID()] = VFE.get();
 }
 
-void FileManager::modifyFileEntry(FileEntry *File,
-                                  off_t Size, time_t ModificationTime) {
-  File->Size = Size;
-  File->ModTime = ModificationTime;
-}
-
 StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
   // FIXME: use llvm::sys::fs::canonical() when it gets implemented
   llvm::DenseMap<const DirectoryEntry *, llvm::StringRef>::iterator Known
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index d49d594..58b9528 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -669,17 +669,19 @@ void SourceManager::overrideFileContents(const FileEntry *SourceFile,
   getOverriddenFilesInfo().OverriddenFiles[SourceFile] = NewFile;
 }
 
-void SourceManager::disableFileContentsOverride(const FileEntry *File) {
-  if (!isFileOverridden(File))
-    return;
-
-  const SrcMgr::ContentCache *IR = getOrCreateContentCache(File);
-  const_cast<SrcMgr::ContentCache *>(IR)->replaceBuffer(nullptr);
-  const_cast<SrcMgr::ContentCache *>(IR)->ContentsEntry = IR->OrigEntry;
+const FileEntry *
+SourceManager::bypassFileContentsOverride(const FileEntry &File) {
+  assert(isFileOverridden(&File));
+  llvm::Optional<FileEntryRef> BypassFile =
+      FileMgr.getBypassFile(FileEntryRef(File.getName(), File));
+
+  // If the file can't be found in the FS, give up.
+  if (!BypassFile)
+    return nullptr;
 
-  assert(OverriddenFilesInfo);
-  OverriddenFilesInfo->OverriddenFiles.erase(File);
-  OverriddenFilesInfo->OverriddenFilesWithBuffer.erase(File);
+  const FileEntry *FE = &BypassFile->getFileEntry();
+  (void)getOrCreateContentCache(FE);
+  return FE;
 }
 
 void SourceManager::setFileIsTransient(const FileEntry *File) {
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index aaa59fc..10b2a5c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2315,19 +2315,14 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
   if ((!Overridden && !Transient) && SM.isFileOverridden(File)) {
     if (Complain)
       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.
-    //
-    // FIXME: This recovery is just as broken as the original state; there may
-    // be another precompiled module that's using the overridden contents, or
-    // we might be half way through parsing it. Instead, we should treat the
-    // overridden contents as belonging to a separate FileEntry.
-    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<FileEntry*>(File),
-                            StoredSize, StoredTime);
+
+    // After emitting the diagnostic, bypass the overriding file to recover
+    // (this creates a separate FileEntry).
+    File = SM.bypassFileContentsOverride(*File);
+    if (!File) {
+      F.InputFilesLoaded[ID - 1] = InputFile::getNotFound();
+      return InputFile();
+    }
   }
 
   bool IsOutOfDate = false;
diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp
index c55403a..9195cc0 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -397,4 +397,54 @@ TEST_F(FileManagerTest, getFileDontOpenRealPath) {
   EXPECT_EQ((*file)->tryGetRealPathName(), ExpectedResult);
 }
 
+TEST_F(FileManagerTest, getBypassFile) {
+  SmallString<64> CustomWorkingDir;
+#ifdef _WIN32
+  CustomWorkingDir = "C:/";
+#else
+  CustomWorkingDir = "/";
+#endif
+
+  auto FS = IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>(
+      new llvm::vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
+  // Inject fake files into the file system.
+  auto Cache = std::make_unique<FakeStatCache>();
+  Cache->InjectDirectory("/tmp", 42);
+  Cache->InjectFile("/tmp/test", 43);
+  Manager.setStatCache(std::move(Cache));
+
+  // Set up a virtual file with a different size than FakeStatCache uses.
+  const FileEntry *File = Manager.getVirtualFile("/tmp/test", /*Size=*/10, 0);
+  ASSERT_TRUE(File);
+  FileEntryRef Ref("/tmp/test", *File);
+  EXPECT_TRUE(Ref.isValid());
+  EXPECT_EQ(Ref.getSize(), 10);
+
+  // Calling a second time should not affect the UID or size.
+  unsigned VirtualUID = Ref.getUID();
+  EXPECT_EQ(*expectedToOptional(Manager.getFileRef("/tmp/test")), Ref);
+  EXPECT_EQ(Ref.getUID(), VirtualUID);
+  EXPECT_EQ(Ref.getSize(), 10);
+
+  // Bypass the file.
+  llvm::Optional<FileEntryRef> BypassRef = Manager.getBypassFile(Ref);
+  ASSERT_TRUE(BypassRef);
+  EXPECT_TRUE(BypassRef->isValid());
+  EXPECT_EQ(BypassRef->getName(), Ref.getName());
+
+  // Check that it's different in the right ways.
+  EXPECT_NE(&BypassRef->getFileEntry(), File);
+  EXPECT_NE(BypassRef->getUID(), VirtualUID);
+  EXPECT_NE(BypassRef->getSize(), Ref.getSize());
+
+  // The virtual file should still be returned when searching.
+  EXPECT_EQ(*expectedToOptional(Manager.getFileRef("/tmp/test")), Ref);
+}
+
 } // anonymous namespace