benlangmuir created this revision.
benlangmuir added reviewers: jansvoboda11, Bigcheese.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Sharing the FileManager across implicit module builds currently leaks
paths looked up in an importer into the built module itself. This can
cause non-deterministic results across scans. It is especially bad for
modules since the path can be saved into the pcm file itself, leading to
stateful behaviour if the cache is shared.
This should not impact the number of real filesystem accesses in the
scanner, since it is already caching in the
DependencyScanningWorkerFilesystem.
Note: this change does not affect whether or not the FileManager is
shared across TUs in the scanner, which is a separate issue.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D131412
Files:
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/test/ClangScanDeps/modules-file-path-isolation.c
Index: clang/test/ClangScanDeps/modules-file-path-isolation.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-file-path-isolation.c
@@ -0,0 +1,44 @@
+// Ensure that the spelling of a path seen outside a module (e.g. header via
+// symlink) does not leak into the compilation of that module unnecessarily.
+// Note: the spelling of the modulemap path still depends on the includer,
since
+// that is the only source of information about it.
+
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+// RUN: ln -s A.h %t/Z.h
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 -format
experimental-full \
+// RUN: -mode preprocess-dependency-directives -generate-modules-path-args >
%t/output
+// RUN: FileCheck %s < %t/output
+
+// CHECK: "modules": [
+// CHECK-NEXT: {
+// CHECK: "file-deps": [
+// CHECK-NEXT: "{{.*}}A.h",
+// CHECK-NEXT: "{{.*}}module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "A"
+// CHECK-NEXT: }
+
+//--- cdb.json.in
+[{
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/tu.c -fmodules
-fmodules-cache-path=DIR/module-cache -fimplicit-modules
-fimplicit-module-maps",
+ "file": "DIR/tu.c"
+}]
+
+//--- module.modulemap
+module A { header "A.h" }
+module B { header "B.h" }
+module C { header "C.h" }
+
+//--- A.h
+
+//--- B.h
+#include "Z.h"
+
+//--- tu.c
+#include "B.h"
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -170,6 +170,7 @@
ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false;
ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false;
+ ScanInstance.getFrontendOpts().ModulesShareFileManager = false;
FileMgr->getFileSystemOpts().WorkingDir = std::string(WorkingDirectory);
ScanInstance.setFileManager(FileMgr);
Index: clang/test/ClangScanDeps/modules-file-path-isolation.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-file-path-isolation.c
@@ -0,0 +1,44 @@
+// Ensure that the spelling of a path seen outside a module (e.g. header via
+// symlink) does not leak into the compilation of that module unnecessarily.
+// Note: the spelling of the modulemap path still depends on the includer, since
+// that is the only source of information about it.
+
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+// RUN: ln -s A.h %t/Z.h
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 -format experimental-full \
+// RUN: -mode preprocess-dependency-directives -generate-modules-path-args > %t/output
+// RUN: FileCheck %s < %t/output
+
+// CHECK: "modules": [
+// CHECK-NEXT: {
+// CHECK: "file-deps": [
+// CHECK-NEXT: "{{.*}}A.h",
+// CHECK-NEXT: "{{.*}}module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "A"
+// CHECK-NEXT: }
+
+//--- cdb.json.in
+[{
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/tu.c -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps",
+ "file": "DIR/tu.c"
+}]
+
+//--- module.modulemap
+module A { header "A.h" }
+module B { header "B.h" }
+module C { header "C.h" }
+
+//--- A.h
+
+//--- B.h
+#include "Z.h"
+
+//--- tu.c
+#include "B.h"
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -170,6 +170,7 @@
ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false;
ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false;
+ ScanInstance.getFrontendOpts().ModulesShareFileManager = false;
FileMgr->getFileSystemOpts().WorkingDir = std::string(WorkingDirectory);
ScanInstance.setFileManager(FileMgr);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
