[clang] Don't add DWARF debug info when assembling .s with clang-cl /Z7

[mstorsjo]

This fixes a regression from f58330c.

That commit changed the clang-cl options /Zi and /Z7 to be implemented as aliases of -g rather than having separate handling.

This had the unintended effect, that when assembling .s files with clang-cl, the /Z7 option (which implies using CodeView debug info) was treated as a -g option, which causes ClangAs::ConstructJob to pick up the option as part of Args.getLastArg(options::OPT_g_Group), which sets the WantDebug variable.

Within Clang::ConstructJob, we check for whether explicit -gdwarf or -gcodeview options have been set, and if not, we pick the default debug format for the current toolchain. However, in ClangAs, if debug info has been enabled, it always adds DWARF debug info.

Add similar logic in ClangAs - check if the user has explicitly requested either DWARF or CodeView, otherwise look up the toolchain default. If we (either implicitly or explicitly) should be producing CodeView, don't enable the default ClangAs DWARF generation.

This fixes the issue, where assembling a single .s file with clang-cl, with the /Z7 option, causes the file to contain some DWARF sections. This causes the output executable to contain DWARF, in addition to the separate intended main PDB file.

By having the output executable contain DWARF sections, LLDB only looks at the (very little) DWARF info in the executable, rather than looking for a separate standalone PDB file. This caused an issue with LLDB's tests, #101710.

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Martin Storsjö (mstorsjo)

Changes

This fixes a regression from f58330c.

That commit changed the clang-cl options /Zi and /Z7 to be implemented as aliases of -g rather than having separate handling.

This had the unintended effect, that when assembling .s files with clang-cl, the /Z7 option (which implies using CodeView debug info) was treated as a -g option, which causes ClangAs::ConstructJob to pick up the option as part of Args.getLastArg(options::OPT_g_Group), which sets the WantDebug variable.

Within Clang::ConstructJob, we check for whether explicit -gdwarf or -gcodeview options have been set, and if not, we pick the default debug format for the current toolchain. However, in ClangAs, if debug info has been enabled, it always adds DWARF debug info.

Add similar logic in ClangAs - check if the user has explicitly requested either DWARF or CodeView, otherwise look up the toolchain default. If we (either implicitly or explicitly) should be producing CodeView, don't enable the default ClangAs DWARF generation.

This fixes the issue, where assembling a single .s file with clang-cl, with the /Z7 option, causes the file to contain some DWARF sections. This causes the output executable to contain DWARF, in addition to the separate intended main PDB file.

By having the output executable contain DWARF sections, LLDB only looks at the (very little) DWARF info in the executable, rather than looking for a separate standalone PDB file. This caused an issue with LLDB's tests, #101710.

---

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

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+26)
• (modified) clang/test/Driver/debug-options-as.c (+16-1)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index df86941950e46e..baac1215215b91 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8641,6 +8641,32 @@ void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
     WantDebug = !A->getOption().matches(options::OPT_g0) &&
                 !A->getOption().matches(options::OPT_ggdb0);
 
+  // If a -gdwarf argument appeared, remember it.
+  bool EmitDwarf = false;
+  if (const Arg *A = getDwarfNArg(Args))
+    EmitDwarf = checkDebugInfoOption(A, Args, D, getToolChain());
+
+  bool EmitCodeView = false;
+  if (const Arg *A = Args.getLastArg(options::OPT_gcodeview))
+    EmitCodeView = checkDebugInfoOption(A, Args, D, getToolChain());
+
+  // If the user asked for debug info but did not explicitly specify -gcodeview
+  // or -gdwarf, ask the toolchain for the default format.
+  if (!EmitCodeView && !EmitDwarf && WantDebug) {
+    switch (getToolChain().getDefaultDebugFormat()) {
+    case llvm::codegenoptions::DIF_CodeView:
+      EmitCodeView = true;
+      break;
+    case llvm::codegenoptions::DIF_DWARF:
+      EmitDwarf = true;
+      break;
+    }
+  }
+
+  // If the arguments don't imply DWARF, don't emit any debug info here.
+  if (!EmitDwarf)
+    WantDebug = false;
+
   llvm::codegenoptions::DebugInfoKind DebugInfoKind =
       llvm::codegenoptions::NoDebugInfo;
 
diff --git a/clang/test/Driver/debug-options-as.c b/clang/test/Driver/debug-options-as.c
index c83c0cb90431d3..3e1ae109711003 100644
--- a/clang/test/Driver/debug-options-as.c
+++ b/clang/test/Driver/debug-options-as.c
@@ -19,12 +19,27 @@
 // GGDB0-NOT: -debug-info-kind=
 
 // Check to make sure clang with -g on a .s file gets passed.
-// RUN: %clang -### -c -integrated-as -g -x assembler %s 2>&1 \
+// This requires a target that defaults to DWARF.
+// RUN: %clang -### --target=x86_64-linux-gnu -c -integrated-as -g -x assembler %s 2>&1 \
 // RUN:   | FileCheck %s
 //
 // CHECK: "-cc1as"
 // CHECK: "-debug-info-kind=constructor"
 
+// Check that a plain -g, without any -gdwarf, for a MSVC target, doesn't
+// trigger producing DWARF output.
+// RUN: %clang -### --target=x86_64-windows-msvc -c -integrated-as -g -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=MSVC %s
+//
+// MSVC: "-cc1as"
+// MSVC-NOT: "-debug-info-kind=constructor"
+
+// Check that clang-cl with the -Z7 option works the same, not triggering
+// any DWARF output.
+//
+// RUN: %clang_cl -### -c -Z7 -x assembler %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=MSVC %s
+
 // Check to make sure clang with -g on a .s file gets passed -dwarf-debug-producer.
 // RUN: %clang -### -c -integrated-as -g -x assembler %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=P %s

[mstorsjo]

@MaskRay - IIRC you're on vacation, but in case you're lurking - any objection to merging this to fix the regression (and backporting it)? If not, I'll go ahead and merge it in a day or two. (It probably misses 19.1.0 RC 4, but can go into the later releases at least.) Also even in case we'd like to change exactly how the fix is implemented, it'd be good to get the functional aspect of it in place, to unblock #101710.

[zmodem]

Looks reasonable to me.

[mstorsjo]

/cherry-pick fcb7b39

[llvmbot]

/pull-request #107146

[mstorsjo]

/cherry-pick fcb7b39 70f3511

[mstorsjo]

/cherry-pick fcb7b39 70f3511 cbb5f03