[clang] [clang modules] Setting `DebugCompilationDir` when it is safe to ignore current working directory (PR #128446)
Qiongsi Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 25 23:06:59 PST 2025
https://github.com/qiongsiwu updated https://github.com/llvm/llvm-project/pull/128446
>From c8eda8b9192cf4bdad4121063336beeb14cbe689 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Sun, 23 Feb 2025 16:47:18 -0800
Subject: [PATCH 1/7] Initial commit
---
.../DependencyScanning/ModuleDepCollector.cpp | 17 +++++++++++++-
clang/test/ClangScanDeps/modules-debug-dir.c | 22 +++++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
create mode 100644 clang/test/ClangScanDeps/modules-debug-dir.c
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 1c5f4c4b50ab6..8a94535d3806c 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -492,8 +492,23 @@ static std::string getModuleContextHash(const ModuleDeps &MD,
auto &FSOpts = const_cast<FileSystemOptions &>(CI.getFileSystemOpts());
if (CWD && !IgnoreCWD)
HashBuilder.add(*CWD);
- else
+ else {
FSOpts.WorkingDir.clear();
+ auto &CGOpts = const_cast<CodeGenOptions &>(CI.getCodeGenOpts());
+ if (CGOpts.DwarfVersion && CWD) {
+ // It is necessary to explicitly set the DebugCompilationDir
+ // to a common directory (e.g. root) if IgnoreCWD is true.
+ // When IgnoreCWD is true, the module's content should not depend
+ // on the current working directory. However, if dwarf information
+ // is needed (when CGOpts.DwarfVersion is non-zero), and if
+ // CGOpts.DebugCompilationDir is not explicitly set,
+ // the current working directory will be automatically embedded
+ // in the dwarf information in the pcm, contradicting the assumption
+ // that it is safe to ignore the CWD. Thus in such cases,
+ // CGOpts.DebugCompilationDir is explicitly set to a common directory.
+ CGOpts.DebugCompilationDir = llvm::sys::path::root_path(*CWD);
+ }
+ }
// Hash the BuildInvocation without any input files.
SmallString<0> ArgVec;
diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c
new file mode 100644
index 0000000000000..580f72205e68f
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-debug-dir.c
@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format \
+// RUN: experimental-full > %t/result.json
+
+//--- cdb.json.in
+[{
+ "directory": "DIR",
+ "command": "clang -g -fdebug-info-for-profiling DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -o DIR/tu.o",
+ "file": "DIR/tu.c"
+}]
+
+//--- include/module.modulemap
+module mod {
+ header "mod.h"
+}
+
+//--- include/mod.h
+
+//--- tu.c
+#include "mod.h"
>From 4b29c19ebc360ec80a8c16e1ac485bbac78a9062 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Mon, 24 Feb 2025 09:34:56 -0800
Subject: [PATCH 2/7] Fix the test.
---
clang/test/ClangScanDeps/modules-debug-dir.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c
index 580f72205e68f..1ac518985471d 100644
--- a/clang/test/ClangScanDeps/modules-debug-dir.c
+++ b/clang/test/ClangScanDeps/modules-debug-dir.c
@@ -3,11 +3,12 @@
// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
// RUN: clang-scan-deps -compilation-database %t/cdb.json -format \
// RUN: experimental-full > %t/result.json
+// RUN: cat %t/result.json | FileCheck %s
//--- cdb.json.in
[{
"directory": "DIR",
- "command": "clang -g -fdebug-info-for-profiling DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -o DIR/tu.o",
+ "command": "clang -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o",
"file": "DIR/tu.c"
}]
@@ -20,3 +21,10 @@ module mod {
//--- tu.c
#include "mod.h"
+
+// Check the -fdebug-compilation-dir used for the module is the root
+// directory when current working directory optimization is in effect.
+// CHECK: "modules": [
+// CHECK: "command-line": [
+// CHECK: "-fdebug-compilation-dir={{\/|C:|\/\/net}}",
+// CHECK: "translation-units": [
>From 5c53c84d33ad4ceec4e79dc36638e827607709ca Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Mon, 24 Feb 2025 10:34:35 -0800
Subject: [PATCH 3/7] Try to fix the test on windows.
---
clang/test/ClangScanDeps/modules-debug-dir.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c
index 1ac518985471d..87f1bc141c73a 100644
--- a/clang/test/ClangScanDeps/modules-debug-dir.c
+++ b/clang/test/ClangScanDeps/modules-debug-dir.c
@@ -8,7 +8,7 @@
//--- cdb.json.in
[{
"directory": "DIR",
- "command": "clang -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o",
+ "command": "clang -target x86_64-apple-darwin -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o",
"file": "DIR/tu.c"
}]
@@ -26,5 +26,5 @@ module mod {
// directory when current working directory optimization is in effect.
// CHECK: "modules": [
// CHECK: "command-line": [
-// CHECK: "-fdebug-compilation-dir={{\/|C:|\/\/net}}",
+// CHECK: "-fdebug-compilation-dir=/",
// CHECK: "translation-units": [
>From 7021180bec1533009a84104023770ef18e7056ea Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Tue, 25 Feb 2025 12:23:49 -0800
Subject: [PATCH 4/7] Address code review and try fixing the test on windows.
---
.../DependencyScanning/ModuleDepCollector.h | 2 +-
.../DependencyScanning/ModuleDepCollector.cpp | 58 ++++++++++++-------
clang/test/ClangScanDeps/modules-debug-dir.c | 4 +-
3 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index c2071a6eadedd..b41efa254342e 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -317,7 +317,7 @@ class ModuleDepCollector final : public DependencyCollector {
/// Compute the context hash for \p Deps, and create the mapping
/// \c ModuleDepsByID[Deps.ID] = &Deps.
- void associateWithContextHash(const CowCompilerInvocation &CI,
+ void associateWithContextHash(const CowCompilerInvocation &CI, bool IgnoreCWD,
ModuleDeps &Deps);
};
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 8a94535d3806c..5f805998f61bd 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -129,6 +129,26 @@ static void optimizeDiagnosticOpts(DiagnosticOptions &Opts,
Opts.Remarks.clear();
}
+static void optimizeCWD(FileSystemOptions &FSOpts, CodeGenOptions &CGOpts,
+ const std::string &CWD) {
+ FSOpts.WorkingDir.clear();
+ if (CGOpts.DwarfVersion) {
+ // It is necessary to explicitly set the DebugCompilationDir
+ // to a common directory (e.g. root) if IgnoreCWD is true.
+ // When IgnoreCWD is true, the module's content should not
+ // depend on the current working directory. However, if dwarf
+ // information is needed (when CGOpts.DwarfVersion is
+ // non-zero), then CGOpts.DebugCompilationDir must be
+ // populated, because otherwise the current working directory
+ // will be automatically embedded in the dwarf information in
+ // the pcm, contradicting the assumption that it is safe to
+ // ignore the CWD. Thus in such cases,
+ // CGOpts.DebugCompilationDir is explicitly set to a common
+ // directory.
+ CGOpts.DebugCompilationDir = llvm::sys::path::root_path(CWD);
+ }
+}
+
static std::vector<std::string> splitString(std::string S, char Separator) {
SmallVector<StringRef> Segments;
StringRef(S).split(Segments, Separator, /*MaxSplit=*/-1, /*KeepEmpty=*/false);
@@ -489,26 +509,8 @@ static std::string getModuleContextHash(const ModuleDeps &MD,
HashBuilder.add(getClangFullRepositoryVersion());
HashBuilder.add(serialization::VERSION_MAJOR, serialization::VERSION_MINOR);
llvm::ErrorOr<std::string> CWD = VFS.getCurrentWorkingDirectory();
- auto &FSOpts = const_cast<FileSystemOptions &>(CI.getFileSystemOpts());
if (CWD && !IgnoreCWD)
HashBuilder.add(*CWD);
- else {
- FSOpts.WorkingDir.clear();
- auto &CGOpts = const_cast<CodeGenOptions &>(CI.getCodeGenOpts());
- if (CGOpts.DwarfVersion && CWD) {
- // It is necessary to explicitly set the DebugCompilationDir
- // to a common directory (e.g. root) if IgnoreCWD is true.
- // When IgnoreCWD is true, the module's content should not depend
- // on the current working directory. However, if dwarf information
- // is needed (when CGOpts.DwarfVersion is non-zero), and if
- // CGOpts.DebugCompilationDir is not explicitly set,
- // the current working directory will be automatically embedded
- // in the dwarf information in the pcm, contradicting the assumption
- // that it is safe to ignore the CWD. Thus in such cases,
- // CGOpts.DebugCompilationDir is explicitly set to a common directory.
- CGOpts.DebugCompilationDir = llvm::sys::path::root_path(*CWD);
- }
- }
// Hash the BuildInvocation without any input files.
SmallString<0> ArgVec;
@@ -539,9 +541,7 @@ static std::string getModuleContextHash(const ModuleDeps &MD,
}
void ModuleDepCollector::associateWithContextHash(
- const CowCompilerInvocation &CI, ModuleDeps &Deps) {
- bool IgnoreCWD = any(OptimizeArgs & ScanningOptimizations::IgnoreCWD) &&
- isSafeToIgnoreCWD(CI);
+ const CowCompilerInvocation &CI, bool IgnoreCWD, ModuleDeps &Deps) {
Deps.ID.ContextHash =
getModuleContextHash(Deps, CI, EagerLoadModules, IgnoreCWD,
ScanInstance.getVirtualFileSystem());
@@ -741,6 +741,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
MD.ModuleMapFileDeps.emplace_back(*ResolvedFilenameAsRequested);
});
+ bool IgnoreCWD = false;
CowCompilerInvocation CI =
MDC.getInvocationAdjustedForModuleBuildWithoutOutputs(
MD, [&](CowCompilerInvocation &BuildInvocation) {
@@ -750,13 +751,26 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
*MDC.ScanInstance.getASTReader(), *MF,
MDC.PrebuiltModuleVFSMap,
MDC.OptimizeArgs);
+
if (any(MDC.OptimizeArgs & ScanningOptimizations::SystemWarnings))
optimizeDiagnosticOpts(
BuildInvocation.getMutDiagnosticOpts(),
BuildInvocation.getFrontendOpts().IsSystemModule);
+
+ IgnoreCWD =
+ any(MDC.OptimizeArgs & ScanningOptimizations::IgnoreCWD) &&
+ isSafeToIgnoreCWD(BuildInvocation);
+ if (IgnoreCWD) {
+ llvm::ErrorOr<std::string> CWD =
+ MDC.ScanInstance.getVirtualFileSystem()
+ .getCurrentWorkingDirectory();
+ if (CWD)
+ optimizeCWD(BuildInvocation.getMutFileSystemOpts(),
+ BuildInvocation.getMutCodeGenOpts(), *CWD);
+ }
});
- MDC.associateWithContextHash(CI, MD);
+ MDC.associateWithContextHash(CI, IgnoreCWD, MD);
// Finish the compiler invocation. Requires dependencies and the context hash.
MDC.addOutputPaths(CI, MD);
diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c
index 87f1bc141c73a..4edd30c0d91b7 100644
--- a/clang/test/ClangScanDeps/modules-debug-dir.c
+++ b/clang/test/ClangScanDeps/modules-debug-dir.c
@@ -8,7 +8,7 @@
//--- cdb.json.in
[{
"directory": "DIR",
- "command": "clang -target x86_64-apple-darwin -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o",
+ "command": "clang -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o",
"file": "DIR/tu.c"
}]
@@ -26,5 +26,5 @@ module mod {
// directory when current working directory optimization is in effect.
// CHECK: "modules": [
// CHECK: "command-line": [
-// CHECK: "-fdebug-compilation-dir=/",
+// CHECK: "-fdebug-compilation-dir={{\/|.*:}}",
// CHECK: "translation-units": [
>From 75b4ff6b36a305891bb707748c82357bf9ecc48f Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Tue, 25 Feb 2025 15:56:17 -0800
Subject: [PATCH 5/7] Address code review and try fixing the test on windows.
---
.../DependencyScanning/ModuleDepCollector.cpp | 21 ++++++++++++-------
clang/test/ClangScanDeps/modules-debug-dir.c | 6 ++++--
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 5f805998f61bd..7da3ec71f2da4 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -129,10 +129,9 @@ static void optimizeDiagnosticOpts(DiagnosticOptions &Opts,
Opts.Remarks.clear();
}
-static void optimizeCWD(FileSystemOptions &FSOpts, CodeGenOptions &CGOpts,
- const std::string &CWD) {
- FSOpts.WorkingDir.clear();
- if (CGOpts.DwarfVersion) {
+static void optimizeCWD(CowCompilerInvocation &BuildInvocation, StringRef CWD) {
+ BuildInvocation.getMutFileSystemOpts().WorkingDir.clear();
+ if (BuildInvocation.getCodeGenOpts().DwarfVersion) {
// It is necessary to explicitly set the DebugCompilationDir
// to a common directory (e.g. root) if IgnoreCWD is true.
// When IgnoreCWD is true, the module's content should not
@@ -145,7 +144,16 @@ static void optimizeCWD(FileSystemOptions &FSOpts, CodeGenOptions &CGOpts,
// ignore the CWD. Thus in such cases,
// CGOpts.DebugCompilationDir is explicitly set to a common
// directory.
- CGOpts.DebugCompilationDir = llvm::sys::path::root_path(CWD);
+ // FIXME: It is still excessive to create a copy of
+ // CodeGenOpts for each module. Since we do not modify the
+ // CodeGenOpts otherwise per module, the following code
+ // ends up generating identical CodeGenOpts for each module
+ // with DebugCompilationDir pointing to the root directory.
+ // We can optimize this away by creating a _single_ copy of
+ // CodeGenOpts whose DebugCompilationDir points to the root
+ // directory and reuse it across modules.
+ BuildInvocation.getMutCodeGenOpts().DebugCompilationDir =
+ llvm::sys::path::root_path(CWD);
}
}
@@ -765,8 +773,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
MDC.ScanInstance.getVirtualFileSystem()
.getCurrentWorkingDirectory();
if (CWD)
- optimizeCWD(BuildInvocation.getMutFileSystemOpts(),
- BuildInvocation.getMutCodeGenOpts(), *CWD);
+ optimizeCWD(BuildInvocation, *CWD);
}
});
diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c
index 4edd30c0d91b7..fadec1ad52e35 100644
--- a/clang/test/ClangScanDeps/modules-debug-dir.c
+++ b/clang/test/ClangScanDeps/modules-debug-dir.c
@@ -1,9 +1,11 @@
+// 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: clang-scan-deps -compilation-database %t/cdb.json -format \
// RUN: experimental-full > %t/result.json
-// RUN: cat %t/result.json | FileCheck %s
+// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s
//--- cdb.json.in
[{
@@ -26,5 +28,5 @@ module mod {
// directory when current working directory optimization is in effect.
// CHECK: "modules": [
// CHECK: "command-line": [
-// CHECK: "-fdebug-compilation-dir={{\/|.*:}}",
+// CHECK: "-fdebug-compilation-dir={{\/|.*:(\\)?}}",
// CHECK: "translation-units": [
>From a9274ea7ab4af0d5f3201e9db168f6ed165fa4fd Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Tue, 25 Feb 2025 19:45:04 -0800
Subject: [PATCH 6/7] Try one more time to fix the test on windows.
---
clang/test/ClangScanDeps/modules-debug-dir.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c
index fadec1ad52e35..7d0f529d6259c 100644
--- a/clang/test/ClangScanDeps/modules-debug-dir.c
+++ b/clang/test/ClangScanDeps/modules-debug-dir.c
@@ -1,5 +1,3 @@
-// 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
>From 6b4435c2dd20ca0c85a3ec7fbf9fda570449ee60 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Tue, 25 Feb 2025 23:06:27 -0800
Subject: [PATCH 7/7] Revert "Try one more time to fix the test on windows."
This reverts commit a9274ea7ab4af0d5f3201e9db168f6ed165fa4fd.
---
clang/test/ClangScanDeps/modules-debug-dir.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c
index 7d0f529d6259c..fadec1ad52e35 100644
--- a/clang/test/ClangScanDeps/modules-debug-dir.c
+++ b/clang/test/ClangScanDeps/modules-debug-dir.c
@@ -1,3 +1,5 @@
+// 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
More information about the cfe-commits
mailing list