[DebugInfo][DWARF] Don't emit bogus DW_AT_call_target for complex calls #151378
+91
−38
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
thejh
force-pushed
the
dwarf-mcall-fix
branch
2 times, most recently
from
338d386 to
d58d5c7
Compare
|
For context: I ran into this when @dwblaikie asked me about test coverage in DWARF callsite info generation. |
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-debuginfo Author: Jann (thejh) ChangesOn X86-64, LLVM currently generates the same DWARF debug info for To fix it, change DwarfDebug::constructCallSiteEntryDIEs() to validate the callee operand's semantics ( This fix will result in less DW_TAG_call_site and DW_AT_call_target entries being generated. There is an existing test in dwarf-callsite-related-attrs.ll that asserts the broken behavior; remove the broken check, and instead add a new test dwarf-callsite-related-attrs-indirect.ll that checks behavior for indirect calls. The existing test xray-custom-log.ll is validating something even more broken: It checks the debug info generated by a PATCHABLE_EVENT_CALL. Full diff: https://github.com/llvm/llvm-project/pull/151378.diff 4 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 71888332a6620..5ae2d2a3958bd 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -940,14 +940,23 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
// In the case of an indirect call find the register that holds
// the callee.
const MachineOperand &CalleeOp = TII->getCalleeOperand(MI);
- if (!CalleeOp.isGlobal() &&
- (!CalleeOp.isReg() || !CalleeOp.getReg().isPhysical()))
+ bool PhysRegCalleeOperand =
+ CalleeOp.isReg() && CalleeOp.getReg().isPhysical();
+ // Hack: WebAssembly CALL instructions have MCInstrDesc that does not
+ // describe the call target operand.
+ if (CalleeOp.getOperandNo() < MI.getDesc().operands().size()) {
+ const MCOperandInfo &MCOI =
+ MI.getDesc().operands()[CalleeOp.getOperandNo()];
+ PhysRegCalleeOperand =
+ PhysRegCalleeOperand && MCOI.OperandType == MCOI::OPERAND_REGISTER;
+ }
+ if (!CalleeOp.isGlobal() && !PhysRegCalleeOperand)
continue;
unsigned CallReg = 0;
const DISubprogram *CalleeSP = nullptr;
const Function *CalleeDecl = nullptr;
- if (CalleeOp.isReg()) {
+ if (PhysRegCalleeOperand) {
CallReg = CalleeOp.getReg();
if (!CallReg)
continue;
diff --git a/llvm/test/CodeGen/X86/xray-custom-log.ll b/llvm/test/CodeGen/X86/xray-custom-log.ll
index 8f23055aca97c..f4cdc23687919 100644
--- a/llvm/test/CodeGen/X86/xray-custom-log.ll
+++ b/llvm/test/CodeGen/X86/xray-custom-log.ll
@@ -1,9 +1,6 @@
; RUN: llc -mtriple=x86_64 < %s | FileCheck %s
; RUN: llc -mtriple=x86_64 -relocation-model=pic < %s | FileCheck %s --check-prefix=PIC
-; RUN: llc -mtriple=x86_64 -filetype=obj %s -o %t
-; RUN: llvm-dwarfdump %t | FileCheck %s --check-prefix=DBG
-
define i32 @customevent() nounwind "function-instrument"="xray-always" !dbg !1 {
%eventptr = alloca i8
%eventsize = alloca i64
@@ -93,17 +90,6 @@ define void @leaf_func() "function-instrument"="xray-always" "frame-pointer"="no
declare void @llvm.xray.customevent(ptr, i64)
declare void @llvm.xray.typedevent(i64, ptr, i64)
-;; Construct call site entries for PATCHABLE_EVENT_CALL.
-; DBG: DW_TAG_subprogram
-; DBG: DW_TAG_call_site
-; DBG-NEXT: DW_AT_call_target (DW_OP_reg{{.*}})
-; DBG-NEXT: DW_AT_call_return_pc
-
-; DBG: DW_TAG_subprogram
-; DBG: DW_TAG_call_site
-; DBG-NEXT: DW_AT_call_target (DW_OP_reg{{.*}})
-; DBG-NEXT: DW_AT_call_return_pc
-
!llvm.dbg.cu = !{!7}
!llvm.module.flags = !{!10, !11}
diff --git a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs-indirect.ll b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs-indirect.ll
new file mode 100644
index 0000000000000..6c81e2e72d93c
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs-indirect.ll
@@ -0,0 +1,78 @@
+; $ clang -O2 -S -emit-llvm indir.c -gdwarf-5
+; __attribute__((disable_tail_calls)) void call_reg(void (*f)()) { f(); }
+; __attribute__((disable_tail_calls)) void call_mem(void (**f)()) { (*f)(); }
+
+; RUN: llc -mtriple=x86_64 -debugger-tune=lldb < %s -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump %t.o -o - | FileCheck %s -check-prefix=OBJ -implicit-check-not=DW_TAG_call_site -implicit-check-not=DW_AT_call_target
+; RUN: llvm-dwarfdump -verify %t.o 2>&1 | FileCheck %s -check-prefix=VERIFY
+; RUN: llvm-dwarfdump -statistics %t.o | FileCheck %s -check-prefix=STATS
+
+; VERIFY: No errors.
+; STATS: "#call site DIEs": 1,
+
+; OBJ: DW_TAG_subprogram
+; OBJ: DW_AT_name ("call_reg")
+; Function Attrs: nounwind uwtable
+define dso_local void @call_reg(ptr noundef readonly captures(none) %f) local_unnamed_addr #0 !dbg !10 {
+entry:
+ #dbg_value(ptr %f, !17, !DIExpression(), !18)
+
+; OBJ: DW_TAG_call_site
+; OBJ: DW_AT_call_target
+; OBJ: DW_AT_call_return_pc
+ call void (...) %f() #1, !dbg !19
+ ret void, !dbg !20
+}
+
+; OBJ: DW_TAG_subprogram
+; OBJ: DW_AT_name ("call_mem")
+; Function Attrs: nounwind uwtable
+define dso_local void @call_mem(ptr noundef readonly captures(none) %f) local_unnamed_addr #0 !dbg !21 {
+entry:
+ #dbg_value(ptr %f, !26, !DIExpression(), !27)
+ %0 = load ptr, ptr %f, align 8, !dbg !28, !tbaa !29
+ call void (...) %0() #1, !dbg !28
+ ret void, !dbg !33
+}
+
+attributes #0 = { nounwind uwtable "disable-tail-calls"="true" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 22.0.0git (https://github.com/llvm/llvm-project 74e4a8645da91247dc8dc502771c2cc4d46f1f91)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "indir.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "4a7538b13e2edbec44f43ed5154be38c")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
+!9 = !{!"clang version 22.0.0git (https://github.com/llvm/llvm-project 74e4a8645da91247dc8dc502771c2cc4d46f1f91)"}
+!10 = distinct !DISubprogram(name: "call_reg", scope: !1, file: !1, line: 1, type: !11, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !16)
+!11 = !DISubroutineType(types: !12)
+!12 = !{null, !13}
+!13 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !14, size: 64)
+!14 = !DISubroutineType(types: !15)
+!15 = !{null, null}
+!16 = !{!17}
+!17 = !DILocalVariable(name: "f", arg: 1, scope: !10, file: !1, line: 1, type: !13)
+!18 = !DILocation(line: 0, scope: !10)
+!19 = !DILocation(line: 1, column: 66, scope: !10)
+!20 = !DILocation(line: 1, column: 71, scope: !10)
+!21 = distinct !DISubprogram(name: "call_mem", scope: !1, file: !1, line: 2, type: !22, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !25)
+!22 = !DISubroutineType(types: !23)
+!23 = !{null, !24}
+!24 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !13, size: 64)
+!25 = !{!26}
+!26 = !DILocalVariable(name: "f", arg: 1, scope: !21, file: !1, line: 2, type: !24)
+!27 = !DILocation(line: 0, scope: !21)
+!28 = !DILocation(line: 2, column: 67, scope: !21)
+!29 = !{!30, !30, i64 0}
+!30 = !{!"any pointer", !31, i64 0}
+!31 = !{!"omnipotent char", !32, i64 0}
+!32 = !{!"Simple C/C++ TBAA"}
+!33 = !DILocation(line: 2, column: 75, scope: !21)
diff --git a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
index c927ff21e0191..8ed247d436d8d 100644
--- a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
+++ b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
@@ -20,7 +20,7 @@
; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis -o /dev/null
; VERIFY: No errors.
-; STATS: "#call site DIEs": 6,
+; STATS: "#call site DIEs": 5,
@sink = global i32 0, align 4, !dbg !0
@@ -94,16 +94,10 @@ entry:
; OBJ: DW_TAG_call_site
; OBJ: DW_AT_call_origin ([[foo_sp]] "_Z3foov")
; OBJ: DW_AT_call_return_pc
-; OBJ: DW_TAG_call_site
-; OBJ: DW_AT_call_target
-; OBJ: DW_AT_call_return_pc
define i32 @main() !dbg !29 {
entry:
call void @_Z3foov(), !dbg !32
- %indirect_target = load ptr, ptr undef
- call void %indirect_target()
-
call void asm sideeffect "", "~{dirflag},~{fpsr},~{flags}"()
ret i32 0, !dbg !33
|
On X86-64, LLVM currently generates the same DWARF debug info for `call rax` and `call [rax]`; in both cases, the generated DWARF claims that the call goes to address RAX. This bug occurs because the X86 machine instructions CALL64r and CALL64m both receive register operands, but those register operands have different semantics. To fix it, change DwarfDebug::constructCallSiteEntryDIEs() to validate the callee operand's semantics (`OperandType`) and make sure it is not semantically describing a memory location. This fix will result in less DW_TAG_call_site and DW_AT_call_target entries being generated. There is an existing test in dwarf-callsite-related-attrs.ll that asserts the broken behavior; remove the broken check, and instead add a new test dwarf-callsite-related-attrs-indirect.ll that checks behavior for indirect calls. The existing test xray-custom-log.ll is validating something even more broken: It checks the debug info generated by a PATCHABLE_EVENT_CALL. `TII->getCalleeOperand()` assumes that the first argument of a call instruction is always the destination, but the first argument of PATCHABLE_EVENT_CALL is instead the event structure; and so we were emitting debug info claiming the callee was stored in a register that actually contains some kind of xray event descriptor, and the test validates that this happens. I am breaking and deleting this test. I guess the intent there might have been to validate that we emit debuginfo referencing the target of the direct call that LLVM emits (which we don't do)? But I'm not sure. WebAssembly's CALL instruction somehow has the call target in operand 1, despite the MCInstrDesc only describing a single operand; in such cases, assume that the operand is meant to be a register operand.
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.
On X86-64, LLVM currently generates the same DWARF debug info for
call raxandcall [rax]; in both cases, the generated DWARF claims that the call goes to address RAX. This bug occurs because the X86 machine instructions CALL64r and CALL64m both receive register operands, but those register operands have different semantics.To fix it, change DwarfDebug::constructCallSiteEntryDIEs() to validate the callee operand's semantics (
OperandType) and make sure it is not semantically describing a memory location.This fix will result in less DW_TAG_call_site and DW_AT_call_target entries being generated.
There is an existing test in dwarf-callsite-related-attrs.ll that asserts the broken behavior; remove the broken check, and instead add a new test dwarf-callsite-related-attrs-indirect.ll that checks behavior for indirect calls.
The existing test xray-custom-log.ll is validating something even more broken: It checks the debug info generated by a PATCHABLE_EVENT_CALL.
TII->getCalleeOperand()assumes that the first argument of a call instruction is always the destination, but the first argument of PATCHABLE_EVENT_CALL is instead the event structure; and so we were emitting debug info claiming the callee was stored in a register that actually contains some kind of xray event descriptor, and the test validates that this happens.I am breaking and deleting this test.
I guess the intent there might have been to validate that we emit debuginfo referencing the target of the direct call that LLVM emits (which we don't do)? But I'm not sure.