Author: Jan Svoboda Date: 2021-06-14T13:58:19+02:00 New Revision: 80c0c639687ef52f5c432ea059ff9cb53125d08e
URL: https://github.com/llvm/llvm-project/commit/80c0c639687ef52f5c432ea059ff9cb53125d08e DIFF: https://github.com/llvm/llvm-project/commit/80c0c639687ef52f5c432ea059ff9cb53125d08e.diff LOG: [clang][deps] Prevent unintended modifications of the original TU command-line One of the goals of the dependency scanner is to generate command-lines that can be used to explicitly build modular dependencies of a translation unit. The only modifications to these command-lines should be for the purposes of explicit modular build. However, the current version of dependency scanner leaks its implementation details into the command-lines. The first problem is that the `clang-scan-deps` tool adjusts the original textual command-line (adding `-Eonly -M -MT <target> -sys-header-deps -Wno-error -o /dev/null `, removing `--serialize-diagnostics`) in order to set up the `DependencyScanning` library. This has been addressed in D103461, D104012, D104030, D104031, D104033. With these patches, the `DependencyScanning` library receives the unmodified `CompilerInvocation`, sets it up and uses it for the implicit modular build. Finally, to prevent leaking the implementation details to the resulting command-lines, this patch generates them from the **original** unmodified `CompilerInvocation` rather than from the one that drives the implicit build. Reviewed By: dexonsmith Differential Revision: https://reviews.llvm.org/D104036 Added: clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template clang/test/ClangScanDeps/Inputs/preserved-args/mod.h clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap clang/test/ClangScanDeps/Inputs/preserved-args/tu.c clang/test/ClangScanDeps/preserved-args.c Modified: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index 73bc41d1f955..a9f2b4d0c6fc 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -191,8 +191,7 @@ class ModuleDepCollector final : public DependencyCollector { public: ModuleDepCollector(std::unique_ptr<DependencyOutputOptions> Opts, CompilerInstance &I, DependencyConsumer &C, - std::map<std::string, std::string, std::less<>> - OriginalPrebuiltModuleFiles); + CompilerInvocation &&OriginalCI); void attachToPreprocessor(Preprocessor &PP) override; void attachToASTReader(ASTReader &R) override; @@ -215,9 +214,8 @@ class ModuleDepCollector final : public DependencyCollector { std::unordered_map<const Module *, ModuleDeps> ModularDeps; /// Options that control the dependency output generation. std::unique_ptr<DependencyOutputOptions> Opts; - /// The mapping between prebuilt module names and module files that were - /// present in the original CompilerInvocation. - std::map<std::string, std::string, std::less<>> OriginalPrebuiltModuleFiles; + /// The original Clang invocation passed to dependency scanner. + CompilerInvocation OriginalInvocation; /// Checks whether the module is known as being prebuilt. bool isPrebuiltModule(const Module *M); diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index a2f9b1c0e074..bbe498d12f15 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -47,9 +47,11 @@ class DependencyConsumerForwarder : public DependencyFileGenerator { /// A listener that collects the names and paths to imported modules. class ImportCollectingListener : public ASTReaderListener { + using PrebuiltModuleFilesT = + decltype(HeaderSearchOptions::PrebuiltModuleFiles); + public: - ImportCollectingListener( - std::map<std::string, std::string> &PrebuiltModuleFiles) + ImportCollectingListener(PrebuiltModuleFilesT &PrebuiltModuleFiles) : PrebuiltModuleFiles(PrebuiltModuleFiles) {} bool needsImportVisitation() const override { return true; } @@ -59,7 +61,7 @@ class ImportCollectingListener : public ASTReaderListener { } private: - std::map<std::string, std::string> &PrebuiltModuleFiles; + PrebuiltModuleFilesT &PrebuiltModuleFiles; }; /// Transform arbitrary file name into an object-like file name. @@ -99,6 +101,9 @@ class DependencyScanningAction : public tooling::ToolAction { FileManager *FileMgr, std::shared_ptr<PCHContainerOperations> PCHContainerOps, DiagnosticConsumer *DiagConsumer) override { + // Make a deep copy of the original Clang invocation. + CompilerInvocation OriginalInvocation(*Invocation); + // Create a compiler instance to handle the actual work. CompilerInstance Compiler(std::move(PCHContainerOps)); Compiler.setInvocation(std::move(Invocation)); @@ -144,24 +149,19 @@ class DependencyScanningAction : public tooling::ToolAction { Compiler.setFileManager(FileMgr); Compiler.createSourceManager(*FileMgr); - std::map<std::string, std::string> PrebuiltModuleFiles; if (!Compiler.getPreprocessorOpts().ImplicitPCHInclude.empty()) { - /// Collect the modules that were prebuilt as part of the PCH. - ImportCollectingListener Listener(PrebuiltModuleFiles); + // Collect the modules that were prebuilt as part of the PCH and pass them + // to the compiler. This will prevent the implicit build to create + // duplicate modules and force reuse of existing prebuilt module files + // instead. + ImportCollectingListener Listener( + Compiler.getHeaderSearchOpts().PrebuiltModuleFiles); ASTReader::readASTFileControlBlock( Compiler.getPreprocessorOpts().ImplicitPCHInclude, Compiler.getFileManager(), Compiler.getPCHContainerReader(), /*FindModuleFileExtensions=*/false, Listener, /*ValidateDiagnosticOptions=*/false); } - /// Make a backup of the original prebuilt module file arguments. - std::map<std::string, std::string, std::less<>> OrigPrebuiltModuleFiles = - Compiler.getHeaderSearchOpts().PrebuiltModuleFiles; - /// Configure the compiler with discovered prebuilt modules. This will - /// prevent the implicit build of duplicate modules and force reuse of - /// existing prebuilt module files instead. - Compiler.getHeaderSearchOpts().PrebuiltModuleFiles.insert( - PrebuiltModuleFiles.begin(), PrebuiltModuleFiles.end()); // Create the dependency collector that will collect the produced // dependencies. @@ -187,8 +187,7 @@ class DependencyScanningAction : public tooling::ToolAction { break; case ScanningOutputFormat::Full: Compiler.addDependencyCollector(std::make_shared<ModuleDepCollector>( - std::move(Opts), Compiler, Consumer, - std::move(OrigPrebuiltModuleFiles))); + std::move(Opts), Compiler, Consumer, std::move(OriginalInvocation))); break; } diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index fc6ba6956b34..21770540c100 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -20,8 +20,8 @@ using namespace dependencies; CompilerInvocation ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths( const ModuleDeps &Deps) const { - // Make a deep copy of the invocation. - CompilerInvocation CI(Instance.getInvocation()); + // Make a deep copy of the original Clang invocation. + CompilerInvocation CI(OriginalInvocation); // Remove options incompatible with explicit module build. CI.getFrontendOpts().Inputs.clear(); @@ -39,9 +39,6 @@ CompilerInvocation ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths( CI.getFrontendOpts().ModuleMapFiles.push_back(PrebuiltModule.ModuleMapFile); } - // Restore the original set of prebuilt module files. - CI.getHeaderSearchOpts().PrebuiltModuleFiles = OriginalPrebuiltModuleFiles; - CI.getPreprocessorOpts().ImplicitPCHInclude.clear(); return CI; @@ -269,10 +266,9 @@ void ModuleDepCollectorPP::addModuleDep( ModuleDepCollector::ModuleDepCollector( std::unique_ptr<DependencyOutputOptions> Opts, CompilerInstance &I, - DependencyConsumer &C, - std::map<std::string, std::string, std::less<>> OriginalPrebuiltModuleFiles) + DependencyConsumer &C, CompilerInvocation &&OriginalCI) : Instance(I), Consumer(C), Opts(std::move(Opts)), - OriginalPrebuiltModuleFiles(std::move(OriginalPrebuiltModuleFiles)) {} + OriginalInvocation(std::move(OriginalCI)) {} void ModuleDepCollector::attachToPreprocessor(Preprocessor &PP) { PP.addPPCallbacks(std::make_unique<ModuleDepCollectorPP>(Instance, *this)); diff --git a/clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template b/clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template new file mode 100644 index 000000000000..331b0d5008b2 --- /dev/null +++ b/clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template @@ -0,0 +1,7 @@ +[ + { + "directory": "DIR", + "command": "clang -MD -MT my_target -serialize-diagnostics DIR/tu.dia -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -fmodule-file=Foo=DIR/foo.pcm -o DIR/tu.o", + "file": "DIR/tu.c" + } +] diff --git a/clang/test/ClangScanDeps/Inputs/preserved-args/mod.h b/clang/test/ClangScanDeps/Inputs/preserved-args/mod.h new file mode 100644 index 000000000000..51e37a532a3a --- /dev/null +++ b/clang/test/ClangScanDeps/Inputs/preserved-args/mod.h @@ -0,0 +1 @@ +// mod.h diff --git a/clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap b/clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap new file mode 100644 index 000000000000..99e4ee8f6363 --- /dev/null +++ b/clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap @@ -0,0 +1,3 @@ +module Mod { + header "mod.h" +} diff --git a/clang/test/ClangScanDeps/Inputs/preserved-args/tu.c b/clang/test/ClangScanDeps/Inputs/preserved-args/tu.c new file mode 100644 index 000000000000..01f145835c76 --- /dev/null +++ b/clang/test/ClangScanDeps/Inputs/preserved-args/tu.c @@ -0,0 +1 @@ +#include "mod.h" diff --git a/clang/test/ClangScanDeps/preserved-args.c b/clang/test/ClangScanDeps/preserved-args.c new file mode 100644 index 000000000000..740cbc35fdfd --- /dev/null +++ b/clang/test/ClangScanDeps/preserved-args.c @@ -0,0 +1,26 @@ +// RUN: rm -rf %t && mkdir %t +// RUN: cp -r %S/Inputs/preserved-args/* %t +// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json + +// RUN: echo -%t > %t/result.json +// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full >> %t/result.json +// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s + +// CHECK: -[[PREFIX:.*]] +// CHECK-NEXT: { +// CHECK-NEXT: "modules": [ +// CHECK-NEXT: { +// CHECK: "command-line": [ +// CHECK-NEXT: "-cc1" +// CHECK: "-serialize-diagnostic-file" +// CHECK-NEXT: "[[PREFIX]]/tu.dia" +// CHECK: "-fmodule-file=Foo=[[PREFIX]]/foo.pcm" +// CHECK: "-MT" +// CHECK-NEXT: "my_target" +// CHECK: "-dependency-file" +// CHECK-NEXT: "[[PREFIX]]/tu.d" +// CHECK: ], +// CHECK: "name": "Mod" +// CHECK-NEXT: } +// CHECK-NEXT: ] +// CHECK: } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
