[lld] [LLD] [COFF] Rewrite handling of the /debug: option. NFC. (PR #75175)

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 12 04:50:30 PST 2023


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

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.


>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/2] [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 a2bf471bac86fab1122d733d95d9217347bb3b72 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/2] [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..2b0959199e545 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: " + Twine(arg->getValue()));
+    }
   }
 
   // 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;



More information about the llvm-commits mailing list