[lld] [RFC] [LLD] [COFF] Restructure /debug: option handling, allow controlling features separately (PR #74820)
Martin Storsjö via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 8 02:03:50 PST 2023
https://github.com/mstorsjo created https://github.com/llvm/llvm-project/pull/74820
This is an RFC for a restructuring of the handling of the `/debug:` options in LLD/COFF. This consists of a series of small bitesize refactoring commits, to clearly show the procession. For actual review, if there's agreement on the general direction, I probably would split them up into individual PRs for all the steps.
The motive of the refactoring, is to allow controlling individual debug info features. Currently, if producing a PDB, one passes `/debug` to lld-link. However, if any of the input files also contain DWARF debug info, the output executable will also contain those bits of DWARF - `/debug` includes any DWARF debug info, which is omitted if `/debug` is left out. It would be good to be able to control things more granularly, like create a PDB, but skip any DWARF, but still retain a symbol table.
We do have a bunch of `/debug:<foo>` options, like `/debug:dwarf` and `/debug:symtab`, plus a bunch of variants like `/debug:ghash` and `/debug:noghash`.
(For reference, MS link.exe has none of these options, it only has `/debug`, and it passes DWARF sections into the output regardless of whther the `/debug` option was set.)
The main inflexibility with our current `/debug:` option handling, is that it is treated as an enum - we inspect the last option provided, and it selects one behaviour out of a number profiles. But if one want to be able to control all of these aspects individually, we're in a combinatorial nightmare, we'd need almost 4x4 potential combinations - PDB on/off, GHash on/off, DWARF on/off, symtab on/off, etc.
This branch tries to refactor the handling of the `/debug:` options so that instead of just inspecting the last, we iterate over all of them, progressively applying the settings from each of them. This mostly tries to keep the effect of the last flag dictating the chosen behaviour, like before, but e.g. `/debug:dwarf` or `/debug:symtab` doesn't disable PDB writing, even if there was an earlier `/debug:full` parsed before. On top of these, I add new flags `/debug:nodwarf` and `/debug:nosymtab`. This allows doing things like `/debug:full,nodwarf,symtab` to achieve the particular combination I mentioned before.
This allows handling the `-s` and `-S` options properly in the MinGW linker frontend, to omit the DWARF debug info (but possibly keep a symbol table) while creating a PDB.
>From 1c53455e83bd34cff651386b5e3496cbf8308a9f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Sat, 4 Nov 2023 01:02:00 +0200
Subject: [PATCH 1/5] [LLD] [COFF] Rewrite the config flags for dwarf debug
info or symtab. NFC.
This shouldn't have any user visible effect, but makes the logic
within the linker implementation more explicit.
Note how DWARF debug info sections were retained even if enabling
a link with PDB info only; that behaviour is preserved for now.
---
lld/COFF/Config.h | 4 ++--
lld/COFF/Driver.cpp | 6 +++---
lld/COFF/InputFiles.cpp | 4 ++--
lld/COFF/Writer.cpp | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 24126f635a06f..48c48cefe91d8 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -130,9 +130,9 @@ struct Configuration {
bool forceMultipleRes = false;
bool forceUnresolved = false;
bool debug = false;
- bool debugDwarf = false;
+ bool includeDwarfChunks = false;
bool debugGHashes = false;
- bool debugSymtab = false;
+ bool writeSymtab = false;
bool driver = false;
bool driverUponly = false;
bool driverWdm = false;
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 99c1a60735adc..2eab745ad3b23 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1653,6 +1653,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
debug == DebugKind::GHash || debug == DebugKind::NoGHash) {
config->debug = true;
config->incremental = true;
+ config->includeDwarfChunks = true;
}
// Handle /demangle
@@ -2028,9 +2029,8 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
parseSwaprun(arg->getValue());
config->terminalServerAware =
!config->dll && args.hasFlag(OPT_tsaware, OPT_tsaware_no, true);
- config->debugDwarf = debug == DebugKind::Dwarf;
config->debugGHashes = debug == DebugKind::GHash || debug == DebugKind::Full;
- config->debugSymtab = debug == DebugKind::Symtab;
+ config->writeSymtab = debug == DebugKind::Dwarf || debug == DebugKind::Symtab;
config->autoImport =
args.hasFlag(OPT_auto_import, OPT_auto_import_no, config->mingw);
config->pseudoRelocs = args.hasFlag(
@@ -2049,7 +2049,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
// Don't warn about long section names, such as .debug_info, for mingw or
// when -debug:dwarf is requested.
- if (config->mingw || config->debugDwarf)
+ if (config->mingw || debug == DebugKind::Dwarf)
config->warnLongSectionNames = false;
if (config->incremental && args.hasArg(OPT_profile)) {
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index dd2e1419bb10a..cd744800cb0de 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -236,8 +236,8 @@ SectionChunk *ObjFile::readSection(uint32_t sectionNumber,
// CodeView needs linker support. We need to interpret debug info,
// and then write it to a separate .pdb file.
- // Ignore DWARF debug info unless /debug is given.
- if (!ctx.config.debug && name.starts_with(".debug_"))
+ // Ignore DWARF debug info unless requested to be included.
+ if (!ctx.config.includeDwarfChunks && name.starts_with(".debug_"))
return nullptr;
if (sec->Characteristics & llvm::COFF::IMAGE_SCN_LNK_REMOVE)
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 7b1ff8071e2e3..ec5d10ed99032 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1376,7 +1376,7 @@ void Writer::createSymbolAndStringTable() {
sec->setStringTableOff(addEntryToStringTable(sec->name));
}
- if (ctx.config.debugDwarf || ctx.config.debugSymtab) {
+ if (ctx.config.writeSymtab) {
for (ObjFile *file : ctx.objFileInstances) {
for (Symbol *b : file->getSymbols()) {
auto *d = dyn_cast_or_null<Defined>(b);
>From a39d5acf8f43257db23a6aec412a7455306b850e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Mon, 6 Nov 2023 17:34:29 +0200
Subject: [PATCH 2/5] [LLD] [COFF] Rewrite handling of the /debug: option. NFC.
Don't treat the options as unique enum items, but more as flags that
can be composed, like the /opt: options.
---
lld/COFF/Driver.cpp | 110 +++++++++++++++++++++-----------------------
1 file changed, 53 insertions(+), 57 deletions(-)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 2eab745ad3b23..b7cbf2f572585 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -858,46 +858,6 @@ static std::string createResponseFile(const opt::InputArgList &args,
return std::string(data.str());
}
-enum class DebugKind {
- Unknown,
- None,
- Full,
- FastLink,
- GHash,
- NoGHash,
- Dwarf,
- Symtab
-};
-
-static DebugKind parseDebugKind(const opt::InputArgList &args) {
- auto *a = args.getLastArg(OPT_debug, OPT_debug_opt);
- if (!a)
- return DebugKind::None;
- if (a->getNumValues() == 0)
- return DebugKind::Full;
-
- DebugKind debug = StringSwitch<DebugKind>(a->getValue())
- .CaseLower("none", DebugKind::None)
- .CaseLower("full", DebugKind::Full)
- .CaseLower("fastlink", DebugKind::FastLink)
- // LLD extensions
- .CaseLower("ghash", DebugKind::GHash)
- .CaseLower("noghash", DebugKind::NoGHash)
- .CaseLower("dwarf", DebugKind::Dwarf)
- .CaseLower("symtab", DebugKind::Symtab)
- .Default(DebugKind::Unknown);
-
- if (debug == DebugKind::FastLink) {
- warn("/debug:fastlink unsupported; using /debug:full");
- return DebugKind::Full;
- }
- if (debug == DebugKind::Unknown) {
- error("/debug: unknown option: " + Twine(a->getValue()));
- return DebugKind::None;
- }
- return debug;
-}
-
static unsigned parseDebugTypes(const opt::InputArgList &args) {
unsigned debugTypes = static_cast<unsigned>(DebugType::None);
@@ -1647,13 +1607,58 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
if (args.hasArg(OPT_force, OPT_force_multipleres))
config->forceMultipleRes = true;
+ // Don't warn about long section names, such as .debug_info, for mingw (or
+ // when -debug:dwarf is requested, handled below).
+ if (config->mingw)
+ config->warnLongSectionNames = false;
+
+ bool doGC = true;
+
// Handle /debug
- DebugKind debug = parseDebugKind(args);
- if (debug == DebugKind::Full || debug == DebugKind::Dwarf ||
- debug == DebugKind::GHash || debug == DebugKind::NoGHash) {
- config->debug = true;
- config->incremental = true;
- config->includeDwarfChunks = true;
+ bool shouldCreatePDB = false;
+ if (auto *arg = args.getLastArg(OPT_debug, OPT_debug_opt)) {
+ std::string s;
+ if (arg->getOption().getID() == OPT_debug)
+ s = "full";
+ else
+ s = StringRef(arg->getValue()).lower();
+ if (s == "fastlink") {
+ warn("/debug:fastlink unsupported; using /debug:full");
+ s = "full";
+ }
+ if (s == "none") {
+ } else if (s == "full") {
+ config->debug = true;
+ config->incremental = true;
+ config->includeDwarfChunks = true;
+ config->debugGHashes = true;
+ shouldCreatePDB = true;
+ doGC = false;
+ } else if (s == "ghash") { // LLD extensions
+ config->debug = true;
+ config->incremental = true;
+ config->includeDwarfChunks = true;
+ config->debugGHashes = true;
+ shouldCreatePDB = true;
+ doGC = false;
+ } else if (s == "noghash") {
+ config->debug = true;
+ config->incremental = true;
+ config->includeDwarfChunks = true;
+ shouldCreatePDB = true;
+ doGC = false;
+ } else if (s == "dwarf") {
+ config->debug = true;
+ config->incremental = true;
+ config->includeDwarfChunks = true;
+ config->writeSymtab = true;
+ config->warnLongSectionNames = false;
+ } else if (s == "symtab") {
+ doGC = false;
+ config->writeSymtab = true;
+ } else {
+ error("/debug: unknown option: " + Twine(arg->getValue()));
+ }
}
// Handle /demangle
@@ -1673,9 +1678,6 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
config->driverUponly || config->driverWdm || args.hasArg(OPT_driver);
// Handle /pdb
- bool shouldCreatePDB =
- (debug == DebugKind::Full || debug == DebugKind::GHash ||
- debug == DebugKind::NoGHash);
if (shouldCreatePDB) {
if (auto *arg = args.getLastArg(OPT_pdb))
config->pdbPath = arg->getValue();
@@ -1838,8 +1840,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
config->noimplib = args.hasArg(OPT_noimplib);
+ if (args.hasArg(OPT_profile))
+ doGC = true;
// Handle /opt.
- bool doGC = debug == DebugKind::None || args.hasArg(OPT_profile);
std::optional<ICFLevel> icfLevel;
if (args.hasArg(OPT_profile))
icfLevel = ICFLevel::None;
@@ -2029,8 +2032,6 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
parseSwaprun(arg->getValue());
config->terminalServerAware =
!config->dll && args.hasFlag(OPT_tsaware, OPT_tsaware_no, true);
- config->debugGHashes = debug == DebugKind::GHash || debug == DebugKind::Full;
- config->writeSymtab = debug == DebugKind::Dwarf || debug == DebugKind::Symtab;
config->autoImport =
args.hasFlag(OPT_auto_import, OPT_auto_import_no, config->mingw);
config->pseudoRelocs = args.hasFlag(
@@ -2047,11 +2048,6 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
if (args.hasFlag(OPT_inferasanlibs, OPT_inferasanlibs_no, false))
warn("ignoring '/inferasanlibs', this flag is not supported");
- // Don't warn about long section names, such as .debug_info, for mingw or
- // when -debug:dwarf is requested.
- if (config->mingw || debug == DebugKind::Dwarf)
- config->warnLongSectionNames = false;
-
if (config->incremental && args.hasArg(OPT_profile)) {
warn("ignoring '/incremental' due to '/profile' specification");
config->incremental = false;
>From 3d5b6164224cf7247dae1d2a1f3e6a2edb19b63d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Mon, 6 Nov 2023 17:48:29 +0200
Subject: [PATCH 3/5] [LLD] [COFF] Parse all /debug: options, like /opt:.
This is almost an NFC.
Most option handling is like it was before; the last /debug: option
takes effect, however options like /debug:dwarf or /debug:symtab
don't disable writing a PDB.
---
lld/COFF/Driver.cpp | 90 +++++++++++++++++++++++++--------------------
1 file changed, 50 insertions(+), 40 deletions(-)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index b7cbf2f572585..89059cf3bd523 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1616,48 +1616,58 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
// Handle /debug
bool shouldCreatePDB = false;
- if (auto *arg = args.getLastArg(OPT_debug, OPT_debug_opt)) {
- std::string s;
+ for (auto *arg : args.filtered(OPT_debug, OPT_debug_opt)) {
+ std::string str;
if (arg->getOption().getID() == OPT_debug)
- s = "full";
+ str = "full";
else
- s = StringRef(arg->getValue()).lower();
- if (s == "fastlink") {
- warn("/debug:fastlink unsupported; using /debug:full");
- s = "full";
- }
- if (s == "none") {
- } else if (s == "full") {
- config->debug = true;
- config->incremental = true;
- config->includeDwarfChunks = true;
- config->debugGHashes = true;
- shouldCreatePDB = true;
- doGC = false;
- } else if (s == "ghash") { // LLD extensions
- config->debug = true;
- config->incremental = true;
- config->includeDwarfChunks = true;
- config->debugGHashes = true;
- shouldCreatePDB = true;
- doGC = false;
- } else if (s == "noghash") {
- config->debug = true;
- config->incremental = true;
- config->includeDwarfChunks = true;
- shouldCreatePDB = true;
- doGC = false;
- } else if (s == "dwarf") {
- config->debug = true;
- config->incremental = true;
- config->includeDwarfChunks = true;
- config->writeSymtab = true;
- config->warnLongSectionNames = false;
- } else if (s == "symtab") {
- doGC = false;
- config->writeSymtab = true;
- } else {
- error("/debug: unknown option: " + Twine(arg->getValue()));
+ str = StringRef(arg->getValue()).lower();
+ SmallVector<StringRef, 1> vec;
+ StringRef(str).split(vec, ',');
+ for (StringRef s : vec) {
+ if (s == "fastlink") {
+ warn("/debug:fastlink unsupported; using /debug:full");
+ s = "full";
+ }
+ if (s == "none") {
+ config->debug = false;
+ config->incremental = false;
+ config->includeDwarfChunks = false;
+ config->debugGHashes = false;
+ shouldCreatePDB = false;
+ doGC = true;
+ } else if (s == "full") {
+ config->debug = true;
+ config->incremental = true;
+ config->includeDwarfChunks = true;
+ config->debugGHashes = true;
+ shouldCreatePDB = true;
+ doGC = false;
+ } else if (s == "ghash") { // LLD extensions
+ config->debug = true;
+ config->incremental = true;
+ config->includeDwarfChunks = true;
+ config->debugGHashes = true;
+ shouldCreatePDB = true;
+ doGC = false;
+ } else if (s == "noghash") {
+ config->debug = true;
+ config->incremental = true;
+ config->includeDwarfChunks = true;
+ shouldCreatePDB = true;
+ doGC = false;
+ } else if (s == "dwarf") {
+ config->debug = true;
+ config->incremental = true;
+ config->includeDwarfChunks = true;
+ config->writeSymtab = true;
+ config->warnLongSectionNames = false;
+ } else if (s == "symtab") {
+ doGC = false;
+ config->writeSymtab = true;
+ } else {
+ error("/debug: unknown option: " + Twine(arg->getValue()));
+ }
}
}
>From fe58b7793cb10fcd6c900f86d367b7e59ac4d7e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Tue, 7 Nov 2023 00:39:02 +0200
Subject: [PATCH 4/5] [LLD] [COFF] Add /debug: options nodwarf and nosymtab
These allow tweaking what gets implied by /debug and /debug:dwarf.
---
lld/COFF/Driver.cpp | 4 ++++
lld/test/COFF/sort-debug.test | 4 ++++
lld/test/COFF/symtab.test | 4 ++++
3 files changed, 12 insertions(+)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 89059cf3bd523..ab14817e635dd 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1662,9 +1662,13 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
config->includeDwarfChunks = true;
config->writeSymtab = true;
config->warnLongSectionNames = false;
+ } else if (s == "nodwarf") {
+ config->includeDwarfChunks = false;
} else if (s == "symtab") {
doGC = false;
config->writeSymtab = true;
+ } else if (s == "nosymtab") {
+ config->writeSymtab = false;
} else {
error("/debug: unknown option: " + Twine(arg->getValue()));
}
diff --git a/lld/test/COFF/sort-debug.test b/lld/test/COFF/sort-debug.test
index bbe2ecd0efd8d..30cad39466548 100644
--- a/lld/test/COFF/sort-debug.test
+++ b/lld/test/COFF/sort-debug.test
@@ -7,6 +7,10 @@
# RUN: llvm-readobj --sections %t.exe | FileCheck -check-prefix=NODEBUG %s
# RUN: lld-link /debug:symtab /out:%t.exe /entry:main %t.obj
# RUN: llvm-readobj --sections %t.exe | FileCheck -check-prefix=NODEBUG %s
+# RUN: lld-link /debug:full,nodwarf /out:%t.exe /entry:main %t.obj
+# RUN: llvm-readobj --sections %t.exe | FileCheck -check-prefix=NODEBUG %s
+# RUN: lld-link /debug:full /debug:nodwarf /out:%t.exe /entry:main %t.obj
+# RUN: llvm-readobj --sections %t.exe | FileCheck -check-prefix=NODEBUG %s
# CHECK: Name: .text
# CHECK: Name: .reloc
diff --git a/lld/test/COFF/symtab.test b/lld/test/COFF/symtab.test
index 48c749957a422..45e8ed39737a4 100644
--- a/lld/test/COFF/symtab.test
+++ b/lld/test/COFF/symtab.test
@@ -5,9 +5,13 @@
# RUN: llvm-readobj --symbols %t.exe | FileCheck %s
# RUN: lld-link /debug:symtab /opt:noref /out:%t.exe /entry:main %t.obj %p/Inputs/std64.lib
# RUN: llvm-readobj --symbols %t.exe | FileCheck %s
+# RUN: lld-link /debug:full,symtab /opt:noref /out:%t.exe /entry:main %t.obj %p/Inputs/std64.lib
+# RUN: llvm-readobj --symbols %t.exe | FileCheck %s
# RUN: lld-link /debug /out:%t.exe /entry:main %t.obj %p/Inputs/std64.lib
# RUN: llvm-readobj --symbols %t.exe | FileCheck -check-prefix=NO %s
+# RUN: lld-link /debug:dwarf,nosymtab /out:%t.exe /entry:main %t.obj %p/Inputs/std64.lib
+# RUN: llvm-readobj --symbols %t.exe | FileCheck -check-prefix=NO %s
# CHECK: Symbols [
# CHECK-NEXT: Symbol {
>From a175bf6bf65a8a6106ef42bc0ed337d639292d45 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Tue, 7 Nov 2023 00:49:29 +0200
Subject: [PATCH 5/5] [LLD] [MinGW] Respect the -S/-s options when writing PDB
files
This allows avoiding including some stray DWARF sections (e.g. from
toolchain provided files), when writing a PDB file.
While that probably could be considered reasonable default behaviour,
PDB writing and including DWARF sections are two entirely orthogonal
concepts, and GNU ld (which can generate PDB files these days)
does include DWARF unless -S/-s is passed, when creating a PDB.
---
lld/MinGW/Driver.cpp | 5 +++++
lld/test/MinGW/driver.test | 7 +++++++
2 files changed, 12 insertions(+)
diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index d22b617cf2f01..94f0ae7993e62 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -297,6 +297,11 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
StringRef v = a->getValue();
if (!v.empty())
add("-pdb:" + v);
+ if (args.hasArg(OPT_strip_all)) {
+ add("-debug:nodwarf,nosymtab");
+ } else if (args.hasArg(OPT_strip_debug)) {
+ add("-debug:nodwarf,symtab");
+ }
} else if (args.hasArg(OPT_strip_debug)) {
add("-debug:symtab");
} else if (!args.hasArg(OPT_strip_all)) {
diff --git a/lld/test/MinGW/driver.test b/lld/test/MinGW/driver.test
index d08c64258be89..74a21f20fc28f 100644
--- a/lld/test/MinGW/driver.test
+++ b/lld/test/MinGW/driver.test
@@ -143,6 +143,13 @@ PDB-NOT: -debug:dwarf
RUN: ld.lld -### -m i386pep foo.o -pdb= 2>&1 | FileCheck -check-prefix PDB-DEFAULT %s
PDB-DEFAULT: -debug
PDB-DEFAULT-NOT: -pdb:{{.*}}
+PDB-DEFAULT-NOT: -debug:
+
+RUN: ld.lld -### -m i386pep foo.o -pdb= -S 2>&1 | FileCheck -check-prefix PDB-NODWARF %s
+PDB-NODWARF: -debug -debug:nodwarf,symtab
+
+RUN: ld.lld -### -m i386pep foo.o -pdb= -s 2>&1 | FileCheck -check-prefix PDB-NODWARF-NOSYMTAB %s
+PDB-NODWARF-NOSYMTAB: -debug -debug:nodwarf,nosymtab
RUN: ld.lld -### -m i386pep foo.o --large-address-aware 2>&1 | FileCheck -check-prefix LARGE-ADDRESS-AWARE %s
LARGE-ADDRESS-AWARE: -largeaddressaware
More information about the llvm-commits
mailing list