[llvm] 810e4c3 - [DebugInfo] Correctly update dbg.values with duplicated location ops
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 14 03:19:35 PDT 2021
Author: Stephen Tozer
Date: 2021-07-14T11:17:24+01:00
New Revision: 810e4c3c66ed69ba82af687fc2a184bae874ca08
URL: https://github.com/llvm/llvm-project/commit/810e4c3c66ed69ba82af687fc2a184bae874ca08
DIFF: https://github.com/llvm/llvm-project/commit/810e4c3c66ed69ba82af687fc2a184bae874ca08.diff
LOG: [DebugInfo] Correctly update dbg.values with duplicated location ops
This patch fixes code that incorrectly handled dbg.values with duplicate
location operands, i.e. !DIArgList(i32 %a, i32 %a). The errors in
question were caused by either applying an update to dbg.value multiple
times when the update is only valid once, or by updating the
DIExpression for only the first instance of a value that appears
multiple times.
Differential Revision: https://reviews.llvm.org/D105831
Added:
llvm/test/DebugInfo/salvage-duplicate-values.ll
Modified:
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/test/CodeGen/AArch64/stack-tagging-dbg.ll
llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
index 7008b188e1f1c..ae6da22d3a762 100644
--- a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -548,7 +548,9 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(I)) {
for (Value *V : DVI->location_ops())
if (auto *AI = dyn_cast_or_null<AllocaInst>(V))
- Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
+ if (Allocas[AI].DbgVariableIntrinsics.empty() ||
+ Allocas[AI].DbgVariableIntrinsics.back() != DVI)
+ Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
continue;
}
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 388a3cefba4f4..785ef9bc966df 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1201,10 +1201,10 @@ bool HWAddressSanitizer::instrumentStack(
// to put it at the beginning of the expression.
SmallVector<uint64_t, 8> NewOps = {dwarf::DW_OP_LLVM_tag_offset,
retagMask(N)};
- auto Locations = DDI->location_ops();
- unsigned LocNo = std::distance(Locations.begin(), find(Locations, AI));
- DDI->setExpression(
- DIExpression::appendOpsToArg(DDI->getExpression(), NewOps, LocNo));
+ for (size_t LocNo = 0; LocNo < DDI->getNumVariableLocationOps(); ++LocNo)
+ if (DDI->getVariableLocationOp(LocNo) == AI)
+ DDI->setExpression(DIExpression::appendOpsToArg(DDI->getExpression(),
+ NewOps, LocNo));
}
size_t Size = getAllocaSizeInBytes(*AI);
@@ -1266,10 +1266,14 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
isa<CleanupReturnInst>(Inst))
RetVec.push_back(&Inst);
- if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst))
- for (Value *V : DVI->location_ops())
+ if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst)) {
+ for (Value *V : DVI->location_ops()) {
if (auto *Alloca = dyn_cast_or_null<AllocaInst>(V))
- AllocaDbgMap[Alloca].push_back(DVI);
+ if (!AllocaDbgMap.count(Alloca) ||
+ AllocaDbgMap[Alloca].back() != DVI)
+ AllocaDbgMap[Alloca].push_back(DVI);
+ }
+ }
if (InstrumentLandingPads && isa<LandingPadInst>(Inst))
LandingPadVec.push_back(&Inst);
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 4bc3747b973b1..86d1ea8ca8f0f 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1737,11 +1737,19 @@ void llvm::salvageDebugInfoForDbgValues(
assert(
is_contained(DIILocation, &I) &&
"DbgVariableIntrinsic must use salvaged instruction as its location");
- unsigned LocNo = std::distance(DIILocation.begin(), find(DIILocation, &I));
SmallVector<Value *, 4> AdditionalValues;
- DIExpression *SalvagedExpr = salvageDebugInfoImpl(
- I, DII->getExpression(), StackValue, LocNo, AdditionalValues);
-
+ // `I` may appear more than once in DII's location ops, and each use of `I`
+ // must be updated in the DIExpression and potentially have additional
+ // values added; thus we call salvageDebugInfoImpl for each `I` instance in
+ // DIILocation.
+ DIExpression *SalvagedExpr = DII->getExpression();
+ auto LocItr = find(DIILocation, &I);
+ while (SalvagedExpr && LocItr != DIILocation.end()) {
+ unsigned LocNo = std::distance(DIILocation.begin(), LocItr);
+ SalvagedExpr = salvageDebugInfoImpl(I, SalvagedExpr, StackValue, LocNo,
+ AdditionalValues);
+ LocItr = std::find(++LocItr, DIILocation.end(), &I);
+ }
// salvageDebugInfoImpl should fail on examining the first element of
// DbgUsers, or none of them.
if (!SalvagedExpr)
diff --git a/llvm/test/CodeGen/AArch64/stack-tagging-dbg.ll b/llvm/test/CodeGen/AArch64/stack-tagging-dbg.ll
index 1525b2d3e3531..9de0456549922 100644
--- a/llvm/test/CodeGen/AArch64/stack-tagging-dbg.ll
+++ b/llvm/test/CodeGen/AArch64/stack-tagging-dbg.ll
@@ -5,12 +5,14 @@ target triple = "aarch64--linux-android"
declare void @use32(i32*)
declare void @llvm.dbg.declare(metadata, metadata, metadata) nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) nounwind readnone speculatable
; Debug intrinsics use the new alloca directly, not through a GEP or a tagp.
define void @DbgIntrinsics() sanitize_memtag {
entry:
%x = alloca i32, align 4
call void @llvm.dbg.declare(metadata i32* %x, metadata !6, metadata !DIExpression()), !dbg !10
+ call void @llvm.dbg.value(metadata !DIArgList(i32* %x, i32* %x), metadata !6, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus)), !dbg !10
store i32 42, i32* %x, align 4
call void @use32(i32* %x)
ret void
@@ -19,6 +21,7 @@ entry:
; CHECK-LABEL: define void @DbgIntrinsics(
; CHECK: [[X:%.*]] = alloca { i32, [12 x i8] }, align 16
; CHECK: call void @llvm.dbg.declare(metadata { i32, [12 x i8] }* [[X]],
+; CHECK: call void @llvm.dbg.value(metadata !DIArgList({ i32, [12 x i8] }* [[X]], { i32, [12 x i8] }* [[X]])
!llvm.dbg.cu = !{!0}
diff --git a/llvm/test/DebugInfo/salvage-duplicate-values.ll b/llvm/test/DebugInfo/salvage-duplicate-values.ll
new file mode 100644
index 0000000000000..116af46aef5d9
--- /dev/null
+++ b/llvm/test/DebugInfo/salvage-duplicate-values.ll
@@ -0,0 +1,50 @@
+; XFAIL: *
+; RUN: opt %s -dce -S | FileCheck %s
+
+; Tests the results of salvaging variadic dbg.values that use the same SSA value
+; multiple times.
+
+; CHECK: call void @llvm.dbg.value(metadata !DIArgList(i32 %a, i32 %a),
+; CHECK-SAME: ![[VAR_C:[0-9]+]],
+; CHECK-SAME: !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_plus_uconst, 5, DW_OP_LLVM_arg, 1, DW_OP_plus_uconst, 5, DW_OP_plus, DW_OP_stack_value))
+
+; CHECK: call void @llvm.dbg.value(metadata !DIArgList(i32 %a, i32 %a, i32 %b, i32 %b),
+; CHECK-SAME: ![[VAR_C]],
+; CHECK-SAME: !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 2, DW_OP_plus, DW_OP_LLVM_arg, 1, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_plus, DW_OP_stack_value))
+
+; CHECK: ![[VAR_C]] = !DILocalVariable(name: "c"
+
+define i32 @"?multiply@@YAHHH at Z"(i32 %a, i32 %b) !dbg !8 {
+entry:
+ %add1 = add nsw i32 %a, 5, !dbg !15
+ %add2 = add nsw i32 %a, %b, !dbg !15
+ call void @llvm.dbg.value(metadata !DIArgList(i32 %add1, i32 %add1), metadata !16, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_stack_value)), !dbg !13
+ call void @llvm.dbg.value(metadata !DIArgList(i32 %add2, i32 %add2), metadata !16, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_stack_value)), !dbg !13
+ %mul = mul nsw i32 %a, %b, !dbg !17
+ ret i32 %mul, !dbg !17
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 11.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "/")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 11.0.0"}
+!8 = distinct !DISubprogram(name: "multiply", linkageName: "?multiply@@YAHHH at Z", scope: !1, file: !1, line: 1, type: !9, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !11, !11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "b", arg: 2, scope: !8, file: !1, line: 1, type: !11)
+!13 = !DILocation(line: 0, scope: !8)
+!14 = !DILocalVariable(name: "a", arg: 1, scope: !8, file: !1, line: 1, type: !11)
+!15 = !DILocation(line: 2, scope: !8)
+!16 = !DILocalVariable(name: "c", scope: !8, file: !1, line: 2, type: !11)
+!17 = !DILocation(line: 3, scope: !8)
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll b/llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll
index 38723f59b62f7..4cf38a84bd605 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll
@@ -36,6 +36,7 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; CHECK: store i8 %[[X_TAG2]], i8* %[[X_I8_GEP]]
; CHECK: call void @llvm.dbg.value(
; CHECK-SAME: metadata !DIArgList(i32* %[[X_BC]], i32* %[[X_BC]])
+; CHECK-SAME: metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0,
; CHECK: call void @use32(i32* nonnull %[[X_HWASAN]])
; UAR-TAGS: %[[BASE_TAG_COMPL:[^ ]*]] = xor i64 %[[BASE_TAG]], 255
More information about the llvm-commits
mailing list