[clang] 9867259 - [clang][deps] Don't share in-memory VFS across scans

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2022-10-12T18:40:34-07:00
New Revision: 98672596626862e4b21319da7c01737bf7e20ae4

URL: https://github.com/llvm/llvm-project/commit/98672596626862e4b21319da7c01737bf7e20ae4
DIFF: https://github.com/llvm/llvm-project/commit/98672596626862e4b21319da7c01737bf7e20ae4.diff

LOG: [clang][deps] Don't share in-memory VFS across scans

The dependency scanning worker may create an in-memory VFS and inject that into its `FileManager`. (Implemented in D109485.) This VFS currently persists between scans, which is not correct: in-memory files created in one scan could affect subsequent scans. This patch changes things so that the in-memory VFS is local to `DependencyScanningWorker::computeDependencies()`.

To test this, one would first scan for dependencies of a named module and then run second scan (on the same worker) for something unrelated, which would pick up the in-memory file created in the first scan. This set up is impossible to achieve with `clang-scan-deps`. We could set this up in `ToolingTests`, but the scanner needs to access the physical filesystem to store `.pcm` files. AFAIK we don't have a good way to propagate something like `%t` into unit tests to make that feasible. (This could be achieved with something like the virtualized `OutputManager`.)

Reviewed By: Bigcheese

Differential Revision: https://reviews.llvm.org/D135414

Added: 
    

Modified: 
    clang/include/clang/Basic/FileManager.h
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
    clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index d3dc54a6be2ac..ec62e0091e0c0 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -243,6 +243,10 @@ class FileManager : public RefCountedBase<FileManager> {
   const FileSystemOptions &getFileSystemOpts() const { return FileSystemOpts; }
 
   llvm::vfs::FileSystem &getVirtualFileSystem() const { return *FS; }
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+  getVirtualFileSystemPtr() const {
+    return FS;
+  }
 
   void setVirtualFileSystem(IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
     this->FS = std::move(FS);

diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index c5776acadcb93..134ac919bf1e7 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -91,14 +91,13 @@ class DependencyScanningWorker {
 
 private:
   std::shared_ptr<PCHContainerOperations> PCHContainerOps;
-
-  /// The physical filesystem overlaid by `InMemoryFS`.
-  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> RealFS;
-  /// The in-memory filesystem laid on top the physical filesystem in `RealFS`.
-  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFS;
-  /// The file system that is used by each worker when scanning for
-  /// dependencies. This filesystem persists across multiple compiler
-  /// invocations.
+  /// The file system to be used during the scan.
+  /// This is either \c FS passed in the constructor (when performing canonical
+  /// preprocessing), or \c DepFS (when performing dependency directives scan).
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS;
+  /// When performing dependency directives scan, this is the caching (and
+  /// dependency-directives-extracting) filesystem overlaid on top of \c FS
+  /// (passed in the constructor).
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
   ScanningOutputFormat Format;
   /// Whether to optimize the modules' command-line arguments.

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 2da3de34a1850..1810d8f28f0a7 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -209,7 +209,8 @@ class DependencyScanningAction : public tooling::ToolAction {
       // Support for virtual file system overlays on top of the caching
       // filesystem.
       FileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation(
-          ScanInstance.getInvocation(), ScanInstance.getDiagnostics(), DepFS));
+          ScanInstance.getInvocation(), ScanInstance.getDiagnostics(),
+          FileMgr->getVirtualFileSystemPtr()));
 
       llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> LocalDepFS =
           DepFS;
@@ -324,15 +325,17 @@ DependencyScanningWorker::DependencyScanningWorker(
   PCHContainerOps->registerWriter(
       std::make_unique<ObjectFilePCHContainerWriter>());
 
-  auto OverlayFS =
-      llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>(std::move(FS));
-  InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
-  OverlayFS->pushOverlay(InMemoryFS);
-  RealFS = OverlayFS;
-
-  if (Service.getMode() == ScanningMode::DependencyDirectivesScan)
-    DepFS = new DependencyScanningWorkerFilesystem(Service.getSharedCache(),
-                                                   RealFS);
+  switch (Service.getMode()) {
+  case ScanningMode::DependencyDirectivesScan:
+    DepFS =
+        new DependencyScanningWorkerFilesystem(Service.getSharedCache(), FS);
+    BaseFS = DepFS;
+    break;
+  case ScanningMode::CanonicalPreprocessing:
+    DepFS = nullptr;
+    BaseFS = FS;
+    break;
+  }
 }
 
 llvm::Error DependencyScanningWorker::computeDependencies(
@@ -386,22 +389,32 @@ bool DependencyScanningWorker::computeDependencies(
     DependencyConsumer &Consumer, DiagnosticConsumer &DC,
     llvm::Optional<StringRef> ModuleName) {
   // Reset what might have been modified in the previous worker invocation.
-  RealFS->setCurrentWorkingDirectory(WorkingDirectory);
-
-  FileSystemOptions FSOpts;
-  FSOpts.WorkingDir = WorkingDirectory.str();
-  auto FileMgr = llvm::makeIntrusiveRefCnt<FileManager>(FSOpts, RealFS);
+  BaseFS->setCurrentWorkingDirectory(WorkingDirectory);
 
   Optional<std::vector<std::string>> ModifiedCommandLine;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> ModifiedFS;
   if (ModuleName) {
     ModifiedCommandLine = CommandLine;
-    InMemoryFS->addFile(*ModuleName, 0, llvm::MemoryBuffer::getMemBuffer(""));
     ModifiedCommandLine->emplace_back(*ModuleName);
+
+    auto OverlayFS = llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>(
+        std::move(BaseFS));
+    auto InMemoryFS =
+        llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+    InMemoryFS->setCurrentWorkingDirectory(WorkingDirectory);
+    InMemoryFS->addFile(*ModuleName, 0, llvm::MemoryBuffer::getMemBuffer(""));
+    OverlayFS->pushOverlay(InMemoryFS);
+    ModifiedFS = OverlayFS;
   }
 
   const std::vector<std::string> &FinalCommandLine =
       ModifiedCommandLine ? *ModifiedCommandLine : CommandLine;
 
+  FileSystemOptions FSOpts;
+  FSOpts.WorkingDir = WorkingDirectory.str();
+  auto FileMgr = llvm::makeIntrusiveRefCnt<FileManager>(
+      FSOpts, ModifiedFS ? ModifiedFS : BaseFS);
+
   std::vector<const char *> FinalCCommandLine(CommandLine.size(), nullptr);
   llvm::transform(CommandLine, FinalCCommandLine.begin(),
                   [](const std::string &Str) { return Str.c_str(); });