[CLANG][DWARF] Do not emit -ggnu-pubnames for split dwarf version 5.

[ayermolo]

When -gsplit-dwarf is passed in clang emmmits -ggnu-pubnames which results in
.debug_gnu_pubnames/..debug_gnu_pubtypes being generated. For DWARF5 we have
functional .debug_names.

TBH not sure what the right behavior should be. Should we not generate anything
at all by default or generate .debug_names? Doing some archeological digging
initially -gnu-pubnames was always generated to be in line with what gcc does.. It was then
changed so that it was generated when split-dwarf was enabled:
6586452#diff-bac41c71569f27df21a843bcd74d2e604ed508afbdf141777761dfb545c5d228

For LLDB these gnu sections are not useful and just waste space. Maybe a better
check is to be based on tunning?

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

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. For DWARF5 we have
functional .debug_names.

TBH not sure what the right behavior should be. Should we not generate anything
at all by default or generate .debug_names? Doing some archeological digging
initially -gnu-pubnames was always generated to be in line with what gcc does.. It was then
changed so that it was generated when split-dwarf was enabled:
6586452#diff-bac41c71569f27df21a843bcd74d2e604ed508afbdf141777761dfb545c5d228

For LLDB these gnu sections are not useful and just waste space. Maybe a better
check is to be based on tunning?

---

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

2 Files Affected:

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

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 6e1b7e8657d0dc..27a5aef17c8d71 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4479,9 +4479,10 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
                       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)))
+    if (EffectiveDWARFVersion < 5 &&
+        (!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"
diff --git a/clang/test/Driver/split-debug.c b/clang/test/Driver/split-debug.c
index 968f33b4cc035c..1d5f0fa42fdeea 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

[dwblaikie]

If you're debugging with lldb you should probably be using -glldb - if you're doing that, certainly gnu-pubnames should not be enabled by default or implicitly by gsplit-dwarf.

I'd say -gsplit-dwarf -glldb probably doesn't need any names/accelerator table by default (any more than the non-split dwarf should default enable names in -glldb - which, maybe it should be, since it is the default on apple platforms, I think?) - split dwarf for gcc needed to (& I think should still be the default, under -ggdb (the default on Linux etc)) enable gnuoubnames by default for correctness (gdb would assume it was present/accurate and fail lookups, rather than falling back to slow performance/exhaustive search)

[clayborg]

I am fine with -glldb doing the right thing for LLDB.

That being said, I don't believe that LLDB or GDB use .debug_pubnames or .debug_pubtypes as they are incomplete and can't be relied upon. So I would say we should never emit these tables unless these older GNU tables unless the user explicitly asks for them.

[clayborg]

I just noticed the naming difference. I am thinking of the standard older .debug_pubnames and .debug_pubtypes. Are the .debug_gnu_pubnames and .debug_gnu_pubtypes more complete and usable by debuggers? If so, then we should use the -dlldb to disable these GNU specific pubnames and pubtypes sections. And ensure that -ggdb enables these if GDB still uses them and if it doesn't support .debug_names very well.

[ayermolo]

Changed to tunning.

[nico]

Looks this breaks tests on macOS: http://45.33.8.238/macm1/79435/step_7.txt

Please take a look and revert for now if it takes a while to fix.

[ayermolo]

Looks this breaks tests on macOS: http://45.33.8.238/macm1/79435/step_7.txt

Please take a look and revert for now if it takes a while to fix.

Looks this breaks tests on macOS: http://45.33.8.238/macm1/79435/step_7.txt

Please take a look and revert for now if it takes a while to fix.

Looks like it's just one test that checks for -ggnu-pubnames. Let me take a look.

[nico]

It's been a few hours. Time to revert and analyze offline?

[ayermolo]

It's been a few hours. Time to revert and analyze offline?

I think I have a fix. Just takes forever to setup and build on my laptop to test it.
Basically I think mac defaults to LLDB tuning, and test that fails explicitly passes the flag in.

[ayermolo]

fix: #83206