-
Notifications
You must be signed in to change notification settings - Fork 15.2k
CodeGen: Fix CodeView crashes with empty llvm.dbg.cu #163286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CodeGen: Fix CodeView crashes with empty llvm.dbg.cu #163286
Conversation
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-debuginfo Author: Matt Arsenault (arsenm) ChangesFull diff: https://github.com/llvm/llvm-project/pull/163286.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index e57ed24a45065..6cbed50e14ea9 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -625,10 +625,13 @@ void CodeViewDebug::beginModule(Module *M) {
if (Asm->hasDebugInfo()) {
Node = *M->debug_compile_units_begin();
} else {
+ auto DebugCompileUnits = MMI->getModule()->debug_compile_units();
+ if (DebugCompileUnits.empty())
+ return;
+
// When emitting only compiler information, we may have only NoDebug CUs,
// which would be skipped by debug_compile_units_begin.
- NamedMDNode *CUs = MMI->getModule()->getNamedMetadata("llvm.dbg.cu");
- Node = *CUs->operands().begin();
+ Node = *DebugCompileUnits.begin();
}
const auto *CU = cast<DICompileUnit>(Node);
DISourceLanguageName Lang = CU->getSourceLanguage();
@@ -900,8 +903,11 @@ void CodeViewDebug::emitCompilerInformation() {
OS.AddComment("CPUType");
OS.emitInt16(static_cast<uint64_t>(TheCPU));
- NamedMDNode *CUs = MMI->getModule()->getNamedMetadata("llvm.dbg.cu");
- const MDNode *Node = *CUs->operands().begin();
+ auto CUs = MMI->getModule()->debug_compile_units();
+ if (CUs.empty())
+ return;
+
+ const MDNode *Node = *CUs.begin();
const auto *CU = cast<DICompileUnit>(Node);
StringRef CompilerVersion = CU->getProducer();
@@ -948,8 +954,11 @@ void CodeViewDebug::emitBuildInfo() {
// not clear if the compiler path should refer to the executable for the
// frontend or the backend. Leave it blank for now.
TypeIndex BuildInfoArgs[BuildInfoRecord::MaxArgs] = {};
- NamedMDNode *CUs = MMI->getModule()->getNamedMetadata("llvm.dbg.cu");
- const MDNode *Node = *CUs->operands().begin(); // FIXME: Multiple CUs.
+ auto CUs = MMI->getModule()->debug_compile_units();
+ if (CUs.empty())
+ return;
+
+ const MDNode *Node = *CUs.begin(); // FIXME: Multiple CUs.
const auto *CU = cast<DICompileUnit>(Node);
const DIFile *MainSourceFile = CU->getFile();
BuildInfoArgs[BuildInfoRecord::CurrentDirectory] =
diff --git a/llvm/test/DebugInfo/X86/codeview-empty-dbg-cu-crash.ll b/llvm/test/DebugInfo/X86/codeview-empty-dbg-cu-crash.ll
new file mode 100644
index 0000000000000..395b99e7f90b6
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/codeview-empty-dbg-cu-crash.ll
@@ -0,0 +1,30 @@
+; RUN: llc -mtriple=x86_64-pc-windows-msvc < %s | FileCheck %s
+
+; CHECK: .file "<stdin>"
+; CHECK-NEXT: .section .debug$S,"dr"
+; CHECK-NEXT: .p2align 2, 0x0
+; CHECK-NEXT: .long 4 # Debug section magic
+; CHECK-NEXT: .long 241
+; CHECK-NEXT: .long .Ltmp1-.Ltmp0 # Subsection size
+; CHECK-NEXT: .Ltmp0:
+; CHECK-NEXT: .short .Ltmp3-.Ltmp2 # Record length
+; CHECK-NEXT: .Ltmp2:
+; CHECK-NEXT: .short 4353 # Record kind: S_OBJNAME
+; CHECK-NEXT: .long 0 # Signature
+; CHECK-NEXT: .byte 0 # Object name
+; CHECK-NEXT: .p2align 2, 0x0
+; CHECK-NEXT: .Ltmp3:
+; CHECK-NEXT: .short .Ltmp5-.Ltmp4 # Record length
+; CHECK-NEXT: .Ltmp4:
+; CHECK-NEXT: .short 4412 # Record kind: S_COMPILE3
+; CHECK-NEXT: .long 3 # Flags and language
+; CHECK-NEXT: .short 208 # CPUType
+; CHECK-NEXT: .Ltmp1:
+; CHECK-NEXT: .p2align 2, 0x0
+; CHECK-NEXT: .cv_filechecksums # File index to string table offset subsection
+; CHECK-NEXT: .cv_stringtable # String table
+
+!llvm.dbg.cu = !{}
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 2, !"Debug Info Version", i32 3}
|
| } else { | ||
| auto DebugCompileUnits = MMI->getModule()->debug_compile_units(); | ||
| if (DebugCompileUnits.empty()) | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we set Asm to null here like we do for nodebug CUs? This would skip much more code, including emitBuildInfo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure why this was doing that, or how to test it
| auto CUs = MMI->getModule()->debug_compile_units(); | ||
| if (CUs.empty()) | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about emitting incomplete information, maybe we could default to version 0 or something similar?
dwblaikie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this could follow a code path more like the DwarfDebug handling - up in AsmPrinter::doInitialization, the DwarfDebug pass is only added if hasDebugInfo is true, which is based on DbgInfoAvailable which is based on !debug_compile_units.empty - whereas CodeViewDebug is added if the raw llvm.dbg.cu node exists. Seems like Rather than checking the raw node exists, we should check hasDebugInfo in a broader conditional that covers both CV and DWARF.
Hmm, nope, guess not - read further and there's a comment that says "On Windows targets, emit minimal CodeView compiler info even when debug info is disabled"... Not sure what to do with that, in the context of there then being no CUs to base that debug info on. Maybe check what happens in the more normal case, where there's no |
fddd368 to
9c5ab4d
Compare
|
ping |
| Asm = nullptr; | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to code golf this, but I think this would be cleaner if we took the CU variable below and stored it in CodeViewDebug (call it TheCU or something) and reused it in the endModule invocation. It goes right next to the CompilerInfoAsm state.
Part of what's going on here is that we never came up with a proper strategy for handling full LTO, which is what usually creates a multi-CU situation. The emergent behavior is probably fine, and maybe the best we could do, which is just using the global module information from the first compilation unit in the LTO unit. People probably already rely on that.
9c5ab4d to
f4ee03f
Compare
cjacek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
|
The test added here seems to be failing on the stage2 green dragon bot: https://green.lab.llvm.org/job/llvm.org/job/clang-stage2-Rthinlto/1366/testReport/junit/LLVM/DebugInfo_X86/codeview_empty_dbg_cu_crash_ll/ |
|

No description provided.