jansvoboda11 created this revision.
jansvoboda11 added a reviewer: benlangmuir.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `ScanInstance` is a local variable in 
`DependencyScanningAction::runInvocation()` that is referenced by 
`ModuleDepCollector`. Since D132405 <https://reviews.llvm.org/D132405>, 
`ModuleDepCollector` can escape the function and can outlive its 
`ScanInstance`. This patch fixes that by reference-counting `ScanInstance`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133988

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -261,11 +261,11 @@
   // This has to be delayed as the context hash can change at the start of
   // `CompilerInstance::ExecuteAction`.
   if (MDC.ContextHash.empty()) {
-    MDC.ContextHash = MDC.ScanInstance.getInvocation().getModuleHash();
+    MDC.ContextHash = MDC.ScanInstance->getInvocation().getModuleHash();
     MDC.Consumer.handleContextHash(MDC.ContextHash);
   }
 
-  SourceManager &SM = MDC.ScanInstance.getSourceManager();
+  SourceManager &SM = MDC.ScanInstance->getSourceManager();
 
   // Dependency generation really does want to go all the way to the
   // file entry for a source location to find out what is depended on.
@@ -308,16 +308,16 @@
 }
 
 void ModuleDepCollectorPP::EndOfMainFile() {
-  FileID MainFileID = MDC.ScanInstance.getSourceManager().getMainFileID();
-  MDC.MainFile = std::string(MDC.ScanInstance.getSourceManager()
+  FileID MainFileID = MDC.ScanInstance->getSourceManager().getMainFileID();
+  MDC.MainFile = std::string(MDC.ScanInstance->getSourceManager()
                                  .getFileEntryForID(MainFileID)
                                  ->getName());
 
-  if (!MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude.empty())
-    MDC.addFileDep(MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude);
+  if (!MDC.ScanInstance->getPreprocessorOpts().ImplicitPCHInclude.empty())
+    MDC.addFileDep(MDC.ScanInstance->getPreprocessorOpts().ImplicitPCHInclude);
 
   for (const Module *M :
-       MDC.ScanInstance.getPreprocessor().getAffectingModules())
+       MDC.ScanInstance->getPreprocessor().getAffectingModules())
     if (!MDC.isPrebuiltModule(M))
       DirectModularDeps.insert(M);
 
@@ -359,7 +359,7 @@
   MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
   MD.IsSystem = M->IsSystem;
 
-  const FileEntry *ModuleMap = MDC.ScanInstance.getPreprocessor()
+  const FileEntry *ModuleMap = MDC.ScanInstance->getPreprocessor()
                                    .getHeaderSearchInfo()
                                    .getModuleMap()
                                    .getModuleMapFileForUniquing(M);
@@ -372,9 +372,9 @@
   }
 
   serialization::ModuleFile *MF =
-      MDC.ScanInstance.getASTReader()->getModuleManager().lookup(
+      MDC.ScanInstance->getASTReader()->getModuleManager().lookup(
           M->getASTFile());
-  MDC.ScanInstance.getASTReader()->visitInputFiles(
+  MDC.ScanInstance->getASTReader()->visitInputFiles(
       *MF, true, true, [&](const serialization::InputFile &IF, bool isSystem) {
         // __inferred_module.map is the result of the way in which an implicit
         // module build handles inferred modules. It adds an overlay VFS with
@@ -412,7 +412,7 @@
     // map). We simply specify all the module maps in the order they were loaded
     // during the implicit build during scan.
     // TODO: Resolve this by serializing and only using Module::UndeclaredUses.
-    MDC.ScanInstance.getASTReader()->visitTopLevelModuleMaps(
+    MDC.ScanInstance->getASTReader()->visitTopLevelModuleMaps(
         *MF, [&](const FileEntry *FE) {
           if (FE->getName().endswith("__inferred_module.map"))
             return;
@@ -439,7 +439,7 @@
       MD, [&](CompilerInvocation &BuildInvocation) {
         if (MDC.OptimizeArgs)
           optimizeHeaderSearchOpts(BuildInvocation.getHeaderSearchOpts(),
-                                   *MDC.ScanInstance.getASTReader(), *MF);
+                                   *MDC.ScanInstance->getASTReader(), *MF);
       });
 
   MDC.associateWithContextHash(CI, MD);
@@ -535,7 +535,7 @@
 
 ModuleDepCollector::ModuleDepCollector(
     std::unique_ptr<DependencyOutputOptions> Opts,
-    CompilerInstance &ScanInstance, DependencyConsumer &C,
+    std::shared_ptr<CompilerInstance> ScanInstance, DependencyConsumer &C,
     CompilerInvocation OriginalCI, bool OptimizeArgs, bool EagerLoadModules)
     : ScanInstance(ScanInstance), Consumer(C), Opts(std::move(Opts)),
       OriginalInvocation(std::move(OriginalCI)), OptimizeArgs(OptimizeArgs),
@@ -550,7 +550,7 @@
 bool ModuleDepCollector::isPrebuiltModule(const Module *M) {
   std::string Name(M->getTopLevelModuleName());
   const auto &PrebuiltModuleFiles =
-      ScanInstance.getHeaderSearchOpts().PrebuiltModuleFiles;
+      ScanInstance->getHeaderSearchOpts().PrebuiltModuleFiles;
   auto PrebuiltModuleFileIt = PrebuiltModuleFiles.find(Name);
   if (PrebuiltModuleFileIt == PrebuiltModuleFiles.end())
     return false;
@@ -570,12 +570,12 @@
 
 void ModuleDepCollector::addFileDep(StringRef Path) {
   llvm::SmallString<256> Storage;
-  Path = makeAbsolute(ScanInstance, Path, Storage);
+  Path = makeAbsolute(*ScanInstance, Path, Storage);
   FileDeps.push_back(std::string(Path));
 }
 
 void ModuleDepCollector::addFileDep(ModuleDeps &MD, StringRef Path) {
   llvm::SmallString<256> Storage;
-  Path = makeAbsolute(ScanInstance, Path, Storage);
+  Path = makeAbsolute(*ScanInstance, Path, Storage);
   MD.FileDeps.insert(Path);
 }
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -174,34 +174,35 @@
     Scanned = true;
 
     // Create a compiler instance to handle the actual work.
-    CompilerInstance ScanInstance(std::move(PCHContainerOps));
-    ScanInstance.setInvocation(std::move(Invocation));
+    auto ScanInstance =
+        std::make_shared<CompilerInstance>(std::move(PCHContainerOps));
+    ScanInstance->setInvocation(std::move(Invocation));
 
     // Create the compiler's actual diagnostics engine.
-    sanitizeDiagOpts(ScanInstance.getDiagnosticOpts());
-    ScanInstance.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
-    if (!ScanInstance.hasDiagnostics())
+    sanitizeDiagOpts(ScanInstance->getDiagnosticOpts());
+    ScanInstance->createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+    if (!ScanInstance->hasDiagnostics())
       return false;
 
-    ScanInstance.getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath =
+    ScanInstance->getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath =
         true;
 
-    ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false;
-    ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false;
-    ScanInstance.getFrontendOpts().ModulesShareFileManager = false;
+    ScanInstance->getFrontendOpts().GenerateGlobalModuleIndex = false;
+    ScanInstance->getFrontendOpts().UseGlobalModuleIndex = false;
+    ScanInstance->getFrontendOpts().ModulesShareFileManager = false;
 
     FileMgr->getFileSystemOpts().WorkingDir = std::string(WorkingDirectory);
-    ScanInstance.setFileManager(FileMgr);
-    ScanInstance.createSourceManager(*FileMgr);
+    ScanInstance->setFileManager(FileMgr);
+    ScanInstance->createSourceManager(*FileMgr);
 
     llvm::StringSet<> PrebuiltModulesInputFiles;
     // Store the list of prebuilt module files into header search options. This
     // will prevent the implicit build to create duplicate modules and will
     // force reuse of the existing prebuilt module files instead.
-    if (!ScanInstance.getPreprocessorOpts().ImplicitPCHInclude.empty())
+    if (!ScanInstance->getPreprocessorOpts().ImplicitPCHInclude.empty())
       visitPrebuiltModule(
-          ScanInstance.getPreprocessorOpts().ImplicitPCHInclude, ScanInstance,
-          ScanInstance.getHeaderSearchOpts().PrebuiltModuleFiles,
+          ScanInstance->getPreprocessorOpts().ImplicitPCHInclude, *ScanInstance,
+          ScanInstance->getHeaderSearchOpts().PrebuiltModuleFiles,
           PrebuiltModulesInputFiles, /*VisitInputFiles=*/DepFS != nullptr);
 
     // Use the dependency scanning optimized file system if requested to do so.
@@ -209,11 +210,11 @@
       // Support for virtual file system overlays on top of the caching
       // filesystem.
       FileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation(
-          ScanInstance.getInvocation(), ScanInstance.getDiagnostics(), DepFS));
+          ScanInstance->getInvocation(), ScanInstance->getDiagnostics(), DepFS));
 
       llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> LocalDepFS =
           DepFS;
-      ScanInstance.getPreprocessorOpts().DependencyDirectivesForFile =
+      ScanInstance->getPreprocessorOpts().DependencyDirectivesForFile =
           [LocalDepFS = std::move(LocalDepFS)](FileEntryRef File)
           -> Optional<ArrayRef<dependency_directives_scan::Directive>> {
         if (llvm::ErrorOr<EntryRef> Entry =
@@ -231,18 +232,18 @@
     // which ensures that the compiler won't create new dependency collectors,
     // and thus won't write out the extra '.d' files to disk.
     auto Opts = std::make_unique<DependencyOutputOptions>();
-    std::swap(*Opts, ScanInstance.getInvocation().getDependencyOutputOpts());
+    std::swap(*Opts, ScanInstance->getInvocation().getDependencyOutputOpts());
     // We need at least one -MT equivalent for the generator of make dependency
     // files to work.
     if (Opts->Targets.empty())
       Opts->Targets = {
-          deduceDepTarget(ScanInstance.getFrontendOpts().OutputFile,
-                          ScanInstance.getFrontendOpts().Inputs)};
+          deduceDepTarget(ScanInstance->getFrontendOpts().OutputFile,
+                          ScanInstance->getFrontendOpts().Inputs)};
     Opts->IncludeSystemHeaders = true;
 
     switch (Format) {
     case ScanningOutputFormat::Make:
-      ScanInstance.addDependencyCollector(
+      ScanInstance->addDependencyCollector(
           std::make_shared<DependencyConsumerForwarder>(
               std::move(Opts), WorkingDirectory, Consumer));
       break;
@@ -250,7 +251,7 @@
       MDC = std::make_shared<ModuleDepCollector>(
           std::move(Opts), ScanInstance, Consumer, OriginalInvocation,
           OptimizeArgs, EagerLoadModules);
-      ScanInstance.addDependencyCollector(MDC);
+      ScanInstance->addDependencyCollector(MDC);
       break;
     }
 
@@ -259,7 +260,7 @@
     //
     // TODO: Implement diagnostic bucketing to reduce the impact of strict
     // context hashing.
-    ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true;
+    ScanInstance->getHeaderSearchOpts().ModulesStrictContextHash = true;
 
     std::unique_ptr<FrontendAction> Action;
 
@@ -268,7 +269,7 @@
     else
       Action = std::make_unique<ReadPCHAndPreprocessAction>();
 
-    const bool Result = ScanInstance.ExecuteAction(*Action);
+    const bool Result = ScanInstance->ExecuteAction(*Action);
     if (!DepFS)
       FileMgr->clearStatCache();
 
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -180,9 +180,9 @@
 class ModuleDepCollector final : public DependencyCollector {
 public:
   ModuleDepCollector(std::unique_ptr<DependencyOutputOptions> Opts,
-                     CompilerInstance &ScanInstance, DependencyConsumer &C,
-                     CompilerInvocation OriginalCI, bool OptimizeArgs,
-                     bool EagerLoadModules);
+                     std::shared_ptr<CompilerInstance> ScanInstance,
+                     DependencyConsumer &C, CompilerInvocation OriginalCI,
+                     bool OptimizeArgs, bool EagerLoadModules);
 
   void attachToPreprocessor(Preprocessor &PP) override;
   void attachToASTReader(ASTReader &R) override;
@@ -195,7 +195,7 @@
   friend ModuleDepCollectorPP;
 
   /// The compiler instance for scanning the current translation unit.
-  CompilerInstance &ScanInstance;
+  std::shared_ptr<CompilerInstance> ScanInstance;
   /// The consumer of collected dependency information.
   DependencyConsumer &Consumer;
   /// Path to the main source file.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to