diff options
author | David Stenberg <david.stenberg@ericsson.com> | 2023-11-18 17:01:27 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-11-18 17:01:27 +0100 |
commit | c093383ffadff8dfadfd6bc0ab7107a0e194aa7e (patch) | |
tree | 8ffc7abf98d870b32b5e113121182963c95000a9 | |
parent | b00e2f2a5f3e48fbbe2324b0b371e5657ccd969c (diff) |
[SelectionDAG] Fix crash for salvaging with indirect debug values (#72645)
This is a follow-up to #68981, and fix for #72630, #72447.
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.
-rw-r--r-- | llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 4 | ||||
-rw-r--r-- | llvm/test/DebugInfo/X86/salvage-add-node-indirect.ll | 103 |
2 files changed, 107 insertions, 0 deletions
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 1bcd85417eba..cb79b7a73cd3 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 000000000000..cae8a479a5ad --- /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) |