[CLANG][DWARF] Do not emit -ggnu-pubnames for LLDB tuning, unless -ggnu-pubnames is specified.

[ayermolo]

When -gsplit-dwarf is passed in clang emmmits -ggnu-pubnames which
results in
.debug_gnu_pubnames/..debug_gnu_pubtypes being generated.
This is used by GDB, but not by LLDB.
Changed so that these sections are not emitted for LLDB tuning, unless flag
is passed explicitly.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Alexander Yermolovich (ayermolo)

Changes

When -gsplit-dwarf is passed in clang emmmits -ggnu-pubnames which
results in
.debug_gnu_pubnames/..debug_gnu_pubtypes being generated.
This is used by GDB, but not by LLDB.
Changed so that these sections are not emitted for LLDB tuning.

---

Full diff: https://github.com/llvm/llvm-project/pull/83331.diff

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+10-4)
• (modified) clang/test/Driver/split-debug.c (+10-1)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 66c3a237c12117..2dc42119973caf 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4478,14 +4478,20 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
       Args.getLastArg(options::OPT_ggnu_pubnames, options::OPT_gno_gnu_pubnames,
                       options::OPT_gpubnames, options::OPT_gno_pubnames);
   if (DwarfFission != DwarfFissionKind::None ||
-      (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC)))
-    if (!PubnamesArg ||
-        (!PubnamesArg->getOption().matches(options::OPT_gno_gnu_pubnames) &&
-         !PubnamesArg->getOption().matches(options::OPT_gno_pubnames)))
+      (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC))) {
+    const bool OptionSet =
+        (PubnamesArg &&
+         (PubnamesArg->getOption().matches(options::OPT_gpubnames) ||
+          PubnamesArg->getOption().matches(options::OPT_ggnu_pubnames)));
+    if ((DebuggerTuning != llvm::DebuggerKind::LLDB || OptionSet) &&
+        (!PubnamesArg ||
+         (!PubnamesArg->getOption().matches(options::OPT_gno_gnu_pubnames) &&
+          !PubnamesArg->getOption().matches(options::OPT_gno_pubnames))))
       CmdArgs.push_back(PubnamesArg && PubnamesArg->getOption().matches(
                                            options::OPT_gpubnames)
                             ? "-gpubnames"
                             : "-ggnu-pubnames");
+  }
   const auto *SimpleTemplateNamesArg =
       Args.getLastArg(options::OPT_gsimple_template_names,
                       options::OPT_gno_simple_template_names);
diff --git a/clang/test/Driver/split-debug.c b/clang/test/Driver/split-debug.c
index 968f33b4cc035c..e297724931045c 100644
--- a/clang/test/Driver/split-debug.c
+++ b/clang/test/Driver/split-debug.c
@@ -11,7 +11,6 @@
 // NOINLINE-NOT: "-fsplit-dwarf-inlining"
 // SPLIT-NOT:  "-dumpdir"
 // SPLIT:      "-debug-info-kind=constructor"
-// SPLIT-SAME: "-ggnu-pubnames"
 // SPLIT-SAME: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo"
 
 // RUN: %clang -### -c -target wasm32 -gsplit-dwarf -g %s 2>&1 | FileCheck %s --check-prefix=SPLIT
@@ -124,3 +123,13 @@
 // G1_NOSPLIT: "-debug-info-kind=line-tables-only"
 // G1_NOSPLIT-NOT: "-split-dwarf-file"
 // G1_NOSPLIT-NOT: "-split-dwarf-output"
+
+/// Do not generate -ggnu-pubnames for -glldb
+// RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -glldb %s 2>&1 | FileCheck %s --check-prefixes=GLLDBSPLIT
+
+// GLLDBSPLIT-NOT: "-ggnu-pubnames"
+
+/// Generate -ggnu-pubnames for -glldb when it is explicitly enabled
+// RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -glldb -ggnu-pubnames %s 2>&1 | FileCheck %s --check-prefixes=GLLDBSPLIT2
+
+// GLLDBSPLIT2: "-ggnu-pubnames"

[ayermolo]

Another attempt. Had to revert original #82840 because it was breaking a test on mac.

[dwblaikie]

has this lost coverage for non -glldb configurations?

(like would we still have test failures if we had a bug where -ggnu-pubnames wasn't passed on Linux when using -gsplit-dwarf?)

[ayermolo]

Sorry not sure I follow.
As discussed in original PR, #82840 , the idea is if -gsplit-dwarf with -glldb is used the -ggnu-pubnames wouldn't be passed in. Since LLDB doesn't benefit from it, and it just uses space.
A fix from original is if someone does specify -ggnu-pubnames it will be generated. I added a test for it on linux side. There is also a test on mac side that used -gsplit-dwarf with -ggnu-pubnames. On mac LLDB tunning is the default. So equivalent of passing -glldb on linux.

[MaskRay]

I haven't checked closely, but you probably need a -NOT directive, and if the debugger tuning is different, you can set the target triple (where the default debugger tuning is derived from) or pass -gtuning option explicitly.

[dwblaikie]

As discussed in original PR, #82840 , the idea is if -gsplit-dwarf with -glldb is used the -ggnu-pubnames wouldn't be passed in. Since LLDB doesn't benefit from it, and it just uses space.

Yep, I'm with you on all of that.

A fix from original is if someone does specify -ggnu-pubnames it will be generated.

Sure.

I added a test for it on linux side.

Added a test for what on the linux side? And where is that test?

There is also a test on mac side that used -gsplit-dwarf with -ggnu-pubnames.

Right, I see that test here in this split-debug.c file at the end. Checking for the absence of -ggnu-pubnames if -gsplit-dwarf -g -glldb is passed, and the presence if -gsplit-dwarf -g -glldb -ggnu-pubnames is passed. So I'm with you there.

But what i don't understand is that, with the removal of the // SPLIT-SAME: "-ggnu-pubnames" line above - what tests that, for instance -gsplit-dwarf -g -ggdb does use -ggnu-pubnames? (said another way: If we removed the any code that added -ggnu-pubnames when -gsplit-dwarf is specified, which lit tests would fail?)

[ayermolo]

Oh I see!
Thanks for clarification.
That check should be there. I think it snuck in when I tried to re-apply a patch internally and somehow got merged with original version of the #82840 PR.

Should I also add a test that explicitly passes -ggdb?

[dwblaikie]

Nah, I think that's probably fine without an explicit -ggdb test - though with the test as-is, wouldn't it fail on macos machines, where they'll implicitly default to -glldb and so -ggnu-pubnames won't get passed, so the check on line 14 of split-debug.c will fail? So maybe that has to be removed from there and only checked via explicit tuning flags, in which case adding a -ggdb test would be necessary (though it could share the CHECK lines from the -glldb -ggnu-pubnames test)

[ayermolo]

At least on my mac it passed. With that check.
I double checked, and this is what I see:
./bin/clang -### -c -target x86_64 -gsplit-dwarf -g /Users/ayermolo/local/llvm-project/clang/test/Driver/split-debug.c
"-dwarf-version=5" "-debugger-tuning=gdb" "-ggnu-pubnames"
./bin/clang -### -c -gsplit-dwarf -g /Users/ayermolo/local/llvm-project/clang/test/Driver/split-debug.c
"-dwarf-version=4" "-debugger-tuning=lldb"

[clayborg]

LGTM, but I will let the code owners here accept

[ayermolo]

@dwblaikie ping

[dwblaikie]

Yeah, still not sure why something like %clang -### -c -target x86_64 -g -gsplit-dwarf %s wouldn't change behavior on a darwin host and cause the check for gnu-pubnames to fail there.... but if you say that works, guess that's OK & if it doesn't, buildbots will tell us.

[ayermolo]

It's been 24 hours with no broken build bots. So I guess this one got it right. :)