[llvm] [SelectionDAG] Fix crash for salvaging with indirect debug values (PR #72645)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 17 04:33:10 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-debuginfo
Author: David Stenberg (dstenb)
<details>
<summary>Changes</summary>
This is a follow-up to #<!-- -->68981.
We may end up in SelectionDAG::salvageDebugInfo() with indirect debug
values, and attempting to salvage ADD nodes with non-constant RHS would
lead us to try to turn those indirect debug values variadic, which is
not allowed.
This triggered the following assert in the SDDbgValue constructor:
Assertion `!(IsVariadic && IsIndirect)' failed.
This also adds a lit test for salvaging when having an indirect debug
value and constant RHS, as there seems like there was no such lit test.
However, I am not sure if the use of the stack_value operation is
correct in that case (which is existing behavior before #<!-- -->68981), but
that at least documents the current behavior.
---
Full diff: https://github.com/llvm/llvm-project/pull/72645.diff
2 Files Affected:
- (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+4)
- (added) llvm/test/DebugInfo/X86/salvage-add-node-indirect.ll (+103)
``````````diff
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 1bcd85417eba260..cb79b7a73cd3c72 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -10852,6 +10852,10 @@ void SelectionDAG::salvageDebugInfo(SDNode &N) {
uint64_t Offset;
if (RHSConstant)
Offset = N.getConstantOperandVal(1);
+ // We are not allowed to turn indirect debug values variadic, so
+ // don't salvage those.
+ if (!RHSConstant && DV->isIndirect())
+ continue;
// Rewrite an ADD constant node into a DIExpression. Since we are
// performing arithmetic to compute the variable's *value* in the
diff --git a/llvm/test/DebugInfo/X86/salvage-add-node-indirect.ll b/llvm/test/DebugInfo/X86/salvage-add-node-indirect.ll
new file mode 100644
index 000000000000000..cae8a479a5ad9da
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/salvage-add-node-indirect.ll
@@ -0,0 +1,103 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
+; RUN: llc -mtriple=x86_64 %s -start-before=x86-isel -o - -stop-after=x86-isel | FileCheck %s
+
+; Verify that we don't crash due to attempting to turn the indirect debug value
+; in @test_non_constant variadic when salvaging the ADD node with non-constant
+; RHS (originating from the GEP). This means that the debug location is
+; dropped.
+;
+; We should salvage the debug information in @test_constant.
+; XXX: Is it actually correct to add a stack_value operation in that case?
+
+%struct.x = type { i64, i64, float, i64, double, ptr }
+
+define i64 @test_constant(ptr %rdata) {
+ ; CHECK-LABEL: name: test_constant
+ ; CHECK: bb.0.entry:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: liveins: $rdi
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: DBG_PHI $rdi, 1
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr64 = COPY $rdi
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1.for.body31:
+ ; CHECK-NEXT: successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: DBG_INSTR_REF !4, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_plus_uconst, 144, DW_OP_deref, DW_OP_stack_value), dbg-instr-ref(1, 0), debug-location !8
+ ; CHECK-NEXT: CMP64mi32 [[COPY]], 1, $noreg, 144, $noreg, 0, implicit-def $eflags :: (load (s64) from %ir.arrayidx33, align 1)
+ ; CHECK-NEXT: JCC_1 %bb.1, 5, implicit $eflags
+ ; CHECK-NEXT: JMP_1 %bb.2
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2.land.lhs.true.i:
+ ; CHECK-NEXT: [[MOV32r0_:%[0-9]+]]:gr32 = MOV32r0 implicit-def dead $eflags
+ ; CHECK-NEXT: [[SUBREG_TO_REG:%[0-9]+]]:gr64 = SUBREG_TO_REG 0, killed [[MOV32r0_]], %subreg.sub_32bit
+ ; CHECK-NEXT: $rax = COPY [[SUBREG_TO_REG]]
+ ; CHECK-NEXT: RET 0, $rax
+entry:
+ br label %for.body31
+
+for.body31: ; preds = %for.body31, %entry
+ %arrayidx33 = getelementptr [30 x %struct.x], ptr %rdata, i64 0, i64 3
+ call void @llvm.dbg.declare(metadata ptr %arrayidx33, metadata !9, metadata !DIExpression()), !dbg !11
+ %0 = load i64, ptr %arrayidx33, align 1
+ %cmp.i = icmp eq i64 %0, 0
+ br i1 %cmp.i, label %land.lhs.true.i, label %for.body31
+
+land.lhs.true.i: ; preds = %for.body31
+ ret i64 0
+}
+
+define i64 @test_non_constant(ptr %rdata, i64 %i.194) {
+ ; CHECK-LABEL: name: test_non_constant
+ ; CHECK: bb.0.entry:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: liveins: $rdi, $rsi
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr64_nosp = COPY $rsi
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gr64 = COPY $rdi
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1.for.body31:
+ ; CHECK-NEXT: successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[LEA64r:%[0-9]+]]:gr64 = LEA64r [[COPY]], 2, [[COPY]], 0, $noreg
+ ; CHECK-NEXT: [[SHL64ri:%[0-9]+]]:gr64_nosp = SHL64ri [[LEA64r]], 4, implicit-def dead $eflags
+ ; CHECK-NEXT: CMP64mi32 [[COPY1]], 1, killed [[SHL64ri]], 0, $noreg, 0, implicit-def $eflags :: (load (s64) from %ir.arrayidx33, align 1)
+ ; CHECK-NEXT: JCC_1 %bb.1, 5, implicit $eflags
+ ; CHECK-NEXT: JMP_1 %bb.2
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2.land.lhs.true.i:
+ ; CHECK-NEXT: [[MOV32r0_:%[0-9]+]]:gr32 = MOV32r0 implicit-def dead $eflags
+ ; CHECK-NEXT: [[SUBREG_TO_REG:%[0-9]+]]:gr64 = SUBREG_TO_REG 0, killed [[MOV32r0_]], %subreg.sub_32bit
+ ; CHECK-NEXT: $rax = COPY [[SUBREG_TO_REG]]
+ ; CHECK-NEXT: RET 0, $rax
+entry:
+ br label %for.body31
+
+for.body31: ; preds = %for.body31, %entry
+ %arrayidx33 = getelementptr [30 x %struct.x], ptr %rdata, i64 0, i64 %i.194
+ call void @llvm.dbg.declare(metadata ptr %arrayidx33, metadata !4, metadata !DIExpression()), !dbg !8
+ %0 = load i64, ptr %arrayidx33, align 1
+ %cmp.i = icmp eq i64 %0, 0
+ br i1 %cmp.i, label %land.lhs.true.i, label %for.body31
+
+land.lhs.true.i: ; preds = %for.body31
+ ret i64 0
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, retainedTypes: !2, globals: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "foo.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !DILocalVariable(name: "a", arg: 1, scope: !5, file: !1, line: 33, type: !7)
+!5 = distinct !DISubprogram(name: "test_non_constant", scope: !1, file: !1, line: 33, type: !6, scopeLine: 34, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!6 = distinct !DISubroutineType(types: !2)
+!7 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "x", file: !1, line: 17, size: 160, elements: !2)
+!8 = !DILocation(line: 33, column: 16, scope: !5)
+!9 = !DILocalVariable(name: "a", arg: 1, scope: !10, file: !1, line: 33, type: !7)
+!10 = distinct !DISubprogram(name: "test_constant", scope: !1, file: !1, line: 33, type: !6, scopeLine: 34, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!11 = !DILocation(line: 33, column: 16, scope: !10)
``````````
</details>
https://github.com/llvm/llvm-project/pull/72645
More information about the llvm-commits
mailing list