[clang][Toolchains][Gnu] pass -g through to assembler

[nickdesaulniers]

Summary

We've been working around this for a long time in the Linux kernel; we
bend over backwards to continue to support CC=clang (w/
-fno-integrated-as) for architectures where clang can't yet be used to
assemble the kernel's assembler sources. Supporting debug info for the
combination of CC=clang w/ GNU binutils as "GAS" has been painful.

Fix this in clang so that we can work towards dropping complexity in the
Linux kernel's build system, Kbuild, for supporting this combination of
tools.

GAS added support for -g in 2004 2.16 release via
commit 329e276daf98 ("Add support for a -g switch to GAS")

[nickdesaulniers]

I'm probably going to do something similar for -gdwarf-N as a follow up.

[MaskRay]

LGTM.

clang/lib/Driver/ToolChains/Gnu.cpp

973

Use Args.AddLastArg (see Clang.cpp)

clang/test/Driver/as-options.s

120

-g can be tested along with other pass-through options. This way we test the relative order (though usually it doesn't matter but it improves the check) and decreases the number of clang invocations.

[probinson]

clang/test/Driver/as-options.s

121

Please use an explicit triple here. I believe not all targets support -fno-integrated-as.

[nickdesaulniers]

• add triple, use Arg::AddLastArg

[nickdesaulniers]

@MaskRay do you think I should make it so that -g -g0 disables passing through -g?

clang/test/Driver/as-options.s

120

Is the suggestion to:

1. combine this test with a pre-existing RUN line? or
2. Test all of the pass through options in this run line.

If 1, then I'm not sure which test best to combine this with.

[nickdesaulniers]

In D136309#3871599, @nickdesaulniers wrote:

@MaskRay do you think I should make it so that -g -g0 disables passing through -g?

Looks like this pattern can be used:

if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
  if (!A->getOption().matches(options::OPT_g0))

[MaskRay]

Many -g options unfortunately enable/disable debug info emission: -g, -g[0123], -ggdb[0123], etc. The full rules are very complex. I think it makes sense to support a subset which is mostly likely used, i.e. -g, -g[0123]. So you may check OPT_g_Group and possibly reuse DebugLevelToInfoKind (if you want to support -ggdb0) or just hard code OPT_g0 if you just want to support -g0.

[nickdesaulniers]

handle -g0 positionally

[MaskRay]

clang/test/Driver/as-options.s

120

If there are many independent pass-through options and one invocation suffices to test them all, you may try that. But I'll not insist on that if you think separate invocations is clearer.

If you add more debug info tests, consider whether other files may be more suitable (e.g. debug-options.c, clang-g-opts.c). For now I think as-options.s is fine.

[MaskRay]

(Note that assembler synthesized debug info for assembly files is a very minor feature. I don't know why the Linux kernel is so fond of it..).

[nickdesaulniers]

In D136309#3871769, @MaskRay wrote:

(Note that assembler synthesized debug info for assembly files is a very minor feature. I don't know why the Linux kernel is so fond of it..).

This is _required_ for symbolicating symbols defined in assembler for stack traces. The primary use is for system wide profiling. See also b/249023842 and b/236733195.

[nickdesaulniers]

Hmm... looks like this regresses 3db3a3b0d5c56dc7da843236e244307c4ac64860; I wonder what PR it was referring to?

[nickdesaulniers]

• delete test from clang/test/Driver/gcc_forward.c

[nickdesaulniers]

Looks like binutils-gdb commit 329e276daf98fb4c8b3770a6c2f02fd22472a638 ("Add support for a -g switch to GAS") added support for -g. (I think the 2.15 release)

But that was in 2004; llvm commit 3db3a3b0d5c56dc7da843236e244307c4ac64860 was in 2016...

[nickdesaulniers]

Updating the description to note support for -g in gas looks like 2.16 (not 2.15) based on:

$ git describe --contains  329e276daf98  --match 'bin*'
binutils-2_16-branchpoint~2248

clang/lib/Driver/ToolChains/Gnu.cpp

void tools::gnutools::Assembler::ConstructJob(Compilation &C,
   Args.AddAllArgValues(CmdArgs, options::OPT_Wa_COMMA, options::OPT_Xassembler);
 
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
   for (const auto &II : Inputs)
     CmdArgs.push_back(II.getFilename());
 
+  if (Arg *A = Args.getLastArg(options::OPT_g_Flag, options::OPT_gN_Group))
+    if (!A->getOption().matches(options::OPT_g0))
+      Args.AddLastArg(CmdArgs, options::OPT_g_Flag);
+
   const char *Exec =
       Args.MakeArgString(getToolChain().GetProgramPath(DefaultAssembler));
   C.addCommand(std::make_unique<Command>(JA, *this,
                                          ResponseFileSupport::AtFileCurCP(),
                                          Exec, CmdArgs, Inputs, Output));
 
   // Handle the debug info splitting at object creation time if we're
   // creating an object.

clang/test/Driver/as-options.s

 
 // But -m flags for the integrated assembler _should_ warn if the integrated
 // assembler is not in use.
 // RUN: %clang -mrelax-all -fintegrated-as -x c++ %s -S -o /dev/null 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
 // RUN: %clang -mrelax-all -fno-integrated-as -x c++ %s -S -o /dev/null 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
 // WARN: unused
+
+// Test that -g is passed through to GAS.
+// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -g %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -g0 -g %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEBUG %s
+// DEBUG: "-g"
+// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -g -g0 %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=NODEBUG %s
+// NODEBUG-NOT: "-g"

clang/test/Driver/gcc_forward.c

 // CHECK-NOT: "-Wdocumentation"
 // CHECK-NOT: -march
 // CHECK-NOT: "-mlinker-version=10"
 // CHECK-NOT: "-Xclang"
 // CHECK-NOT: "foo-bar"
 // CHECK-NOT: "-Wall"
 // CHECK-NOT: "-Wdocumentation"
 // CHECK: "-o" "a.out"
-
-// Check that we're not forwarding -g options to the assembler
-// RUN: %clang -g -target x86_64-unknown-linux-gnu -no-integrated-as -c %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
-// CHECK-ASM: as
-// CHECK-ASM-NOT: "-g"