[clang] 80c0c63 - [clang][deps] Prevent unintended modifications of the original TU command-line
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 14 05:04:57 PDT 2021
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: }
More information about the cfe-commits
mailing list