[lld] [LLD] [COFF] Parse all /debug: options, like /opt: (PR #75178)

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 12 05:20:07 PST 2023


https://github.com/mstorsjo created https://github.com/llvm/llvm-project/pull/75178

Most option handling is like it was before; the last /debug: option takes effect.

However, the options /debug:dwarf or /debug:symtab don't reset all flags into the specific behaviour they chose before - e.g. if an earlier option enables writing a PDB, a later /debug:dwarf or /debug:symtab doesn't disable that. This allows combining these options with options for controlling PDB writing, for finetuning what is done.


>From ef5033fc3cfe63d3947b2664cb218b289087c2b0 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/3] [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.
---
 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 85bc16003c23f41edfe2e2e5f95bd63c74baebd5 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/3] [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.

This still only processes the last option on the command line though,
so the behaviour should still remain exactly as it was, in all
corner cases.
---
 lld/COFF/Driver.cpp | 99 +++++++++++++++++++--------------------------
 1 file changed, 42 insertions(+), 57 deletions(-)

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 2eab745ad3b23..1f26831a27278 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,47 @@ 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" || s == "ghash" || s == "noghash") {
+      config->debug = true;
+      config->incremental = true;
+      config->includeDwarfChunks = true;
+      if (s == "full" || s == "ghash")
+        config->debugGHashes = true;
+      shouldCreatePDB = true;
+      doGC = false;
+    } else if (s == "dwarf") {
+      config->debug = true;
+      config->incremental = true;
+      config->includeDwarfChunks = true;
+      config->writeSymtab = true;
+      config->warnLongSectionNames = false;
+      doGC = false;
+    } else if (s == "symtab") {
+      config->writeSymtab = true;
+      doGC = false;
+    } else {
+      error("/debug: unknown option: " + s);
+    }
   }
 
   // Handle /demangle
@@ -1673,9 +1667,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 +1829,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 +2021,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 +2037,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 19f0c2d02ac6734e968e7294aa97c9f47525a615 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/3] [LLD] [COFF] Parse all /debug: options, like /opt:

Most option handling is like it was before; the last /debug: option
takes effect.

However, the options /debug:dwarf or /debug:symtab don't reset
all flags into the specific behaviour they chose before - e.g. if
an earlier option enables writing a PDB, a later /debug:dwarf or
/debug:symtab doesn't disable that. This allows combining these
options with options for controlling PDB writing, for finetuning
what is done.
---
 lld/COFF/Driver.cpp            | 69 ++++++++++++++++++++--------------
 lld/test/COFF/debug-dwarf.test |  8 ++++
 lld/test/COFF/symtab.test      |  2 +
 3 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 1f26831a27278..978f556632c59 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1616,37 +1616,48 @@ 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" || s == "ghash" || s == "noghash") {
-      config->debug = true;
-      config->incremental = true;
-      config->includeDwarfChunks = true;
-      if (s == "full" || s == "ghash")
-        config->debugGHashes = true;
-      shouldCreatePDB = true;
-      doGC = false;
-    } else if (s == "dwarf") {
-      config->debug = true;
-      config->incremental = true;
-      config->includeDwarfChunks = true;
-      config->writeSymtab = true;
-      config->warnLongSectionNames = false;
-      doGC = false;
-    } else if (s == "symtab") {
-      config->writeSymtab = true;
-      doGC = false;
-    } else {
-      error("/debug: unknown option: " + s);
+      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;
+        config->writeSymtab = false;
+        shouldCreatePDB = false;
+        doGC = true;
+      } else if (s == "full" || s == "ghash" || s == "noghash") {
+        config->debug = true;
+        config->incremental = true;
+        config->includeDwarfChunks = true;
+        if (s == "full" || s == "ghash")
+          config->debugGHashes = true;
+        shouldCreatePDB = true;
+        doGC = false;
+      } else if (s == "dwarf") {
+        config->debug = true;
+        config->incremental = true;
+        config->includeDwarfChunks = true;
+        config->writeSymtab = true;
+        config->warnLongSectionNames = false;
+        doGC = false;
+      } else if (s == "symtab") {
+        config->writeSymtab = true;
+        doGC = false;
+      } else {
+        error("/debug: unknown option: " + s);
+      }
     }
   }
 
diff --git a/lld/test/COFF/debug-dwarf.test b/lld/test/COFF/debug-dwarf.test
index 156b2f58f64e3..eacf363b41af4 100644
--- a/lld/test/COFF/debug-dwarf.test
+++ b/lld/test/COFF/debug-dwarf.test
@@ -17,3 +17,11 @@
 # RUN: rm -f %t.pdb
 # RUN: lld-link /debug:dwarf /pdb:%t.pdb /entry:main /out:%t.exe %p/Inputs/ret42.obj
 # RUN: not ls %t.pdb
+
+# Check that /debug /debug:dwarf or /debug:full,dwarf creates %t.pdb.
+# RUN: rm -f %t.pdb
+# RUN: lld-link /debug /debug:dwarf /entry:main /out:%t.exe %p/Inputs/ret42.obj
+# RUN: ls %t.pdb
+# RUN: rm -f %t.pdb
+# RUN: lld-link /debug:full,dwarf /entry:main /out:%t.exe %p/Inputs/ret42.obj
+# RUN: ls %t.pdb
diff --git a/lld/test/COFF/symtab.test b/lld/test/COFF/symtab.test
index 48c749957a422..f65754335a97e 100644
--- a/lld/test/COFF/symtab.test
+++ b/lld/test/COFF/symtab.test
@@ -5,6 +5,8 @@
 # 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



More information about the llvm-commits mailing list