release/19.x: [clang] Don't add DWARF debug info when assembling .s with clang-cl /Z7 (#106686) #107146
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: None (llvmbot) ChangesBackport fcb7b39 Requested by: @mstorsjo Full diff: https://github.com/llvm/llvm-project/pull/107146.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 366b147a052bf2..8858c318aba7a1 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8561,6 +8561,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
|
|
@llvm/pr-subscribers-clang-driver Author: None (llvmbot) ChangesBackport fcb7b39 Requested by: @mstorsjo Full diff: https://github.com/llvm/llvm-project/pull/107146.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 366b147a052bf2..8858c318aba7a1 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8561,6 +8561,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
|
|
It seems like llvmbot didn't CC anyone to ack to backport this time (or yet), so I'll do it manually: @bogner and @zmodem, what do you think about backporting this to the 19.x release? Also sorry for filing the backport before sorting out buildbot breakage caused by the patch... @tru - this isn't a very urgent fix wrt the release (for me, I guess it could be for someone else though?), so if you want to keep 19.1.0 extra safe and minimal, feel free to hold off of it for a while. But it's a fix for a clear regression, so I guess it qualifies for 19.x at some point at least. The main impact that I know of, is that one of LLDB's tests on Windows fails, in MSVC configurations, until this issue is fixed. |
FWIW, it seems like all buildbots were happy after the two follow-up test fixes that I have included here. |
Sounds good to me, but if rc4 is the last release candidate, this would probably have to go in a future minor release. |
|
With 19.1.0 out of the door, I think this one should be safe to merge now - it has brewed in git main for a couple of weeks without any reported breakage (other than the test fixups that are included). |
…Z7 (llvm#106686) 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, llvm#101710. (cherry picked from commit fcb7b39)
|
@mstorsjo (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport fcb7b39
Requested by: @mstorsjo