[DebugInfo][RemoveDIs] Support DPValues in HWAsan #78731
Merged
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
This patch extends HWASAN to support maintenence of debug-info that isn't stored as intrinsics, but is instead in a DPValue object. This is straight-forwards: we collect any such objects in StackInfoBuilder, and apply the same operations to them as we would to dbg.value and similar intrinsics. I've also replaced some calls to getNextNode with debug-info skipping next calls, and use iterators for instruction insertion rather than instruction pointers. This avoids any difference in output between intrinsic / non-intrinsic debug-info, but also means that any debug-info comes before code inserted by HWAsan, rather than afterwards (see the test modifications). Seeing how the code inserted by HWAsan is always (AFAIUI) given the source-location of the instruction being instrumented, I don't believe this will have any effect on which lines variable assignments become visible on; it may extend the number of instructions covered by the assignments though.
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-debuginfo Author: Jeremy Morse (jmorse) ChangesThis patch extends HWASAN to support maintenance of debug-info that isn't stored as intrinsics, but is instead in a DPValue object. This is straight-forwards: we collect any such objects in StackInfoBuilder, and apply the same operations to them as we would to dbg.value and similar intrinsics. I've also replaced some calls to getNextNode with debug-info skipping next calls, and use iterators for instruction insertion rather than instruction pointers. This avoids any difference in output between intrinsic / non-intrinsic debug-info, but also means that any debug-info comes before code inserted by HWAsan, rather than afterwards. See the test modifications, where the variable assignment (presented as a dbg.value) jumps up over all the code inserted by HWAsan. Seeing how the code inserted by HWAsan is always (AFAIUI) given the source-location of the instruction being instrumented, I don't believe this will have any effect on which lines variable assignments become visible on; it may extend the number of instructions covered by the assignments though. Full diff: https://github.com/llvm/llvm-project/pull/78731.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h b/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
index 990cf60a09c270..eb00e6c4e856df 100644
--- a/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
+++ b/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
@@ -52,6 +52,8 @@ struct AllocaInfo {
SmallVector<IntrinsicInst *, 2> LifetimeStart;
SmallVector<IntrinsicInst *, 2> LifetimeEnd;
SmallVector<DbgVariableIntrinsic *, 2> DbgVariableIntrinsics;
+ // Non-intrinsic records of variable locations.
+ SmallVector<DPValue *, 2> DbgVariableRecords;
};
struct StackInfo {
diff --git a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
index b5b15022cda4fa..73bee2f5bfb1c6 100644
--- a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -578,6 +578,8 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
// Fixup debug intrinsics to point to the new alloca.
for (auto *DVI : Info.DbgVariableIntrinsics)
DVI->replaceVariableLocationOp(OldAI, Info.AI);
+ for (auto *DPV : Info.DbgVariableRecords)
+ DPV->replaceVariableLocationOp(OldAI, Info.AI);
}
// If we have instrumented at least one alloca, all unrecognized lifetime
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index efb621cde90656..db8c6a39eeb0b8 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1357,7 +1357,7 @@ Value *HWAddressSanitizer::readRegister(IRBuilder<> &IRB, StringRef Name) {
bool HWAddressSanitizer::instrumentLandingPads(
SmallVectorImpl<Instruction *> &LandingPadVec) {
for (auto *LP : LandingPadVec) {
- IRBuilder<> IRB(LP->getNextNode());
+ IRBuilder<> IRB(LP->getNextNonDebugInstruction());
IRB.CreateCall(
HwasanHandleVfork,
{readRegister(IRB, (TargetTriple.getArch() == Triple::x86_64) ? "rsp"
@@ -1387,7 +1387,7 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
auto N = I++;
auto *AI = KV.first;
memtag::AllocaInfo &Info = KV.second;
- IRBuilder<> IRB(AI->getNextNode());
+ IRBuilder<> IRB(AI->getNextNonDebugInstruction());
// Replace uses of the alloca with tagged address.
Value *Tag = getAllocaTag(IRB, StackTag, N);
@@ -1425,17 +1425,24 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
return User != AILong && User != AICast && !isLifetimeIntrinsic(User);
});
- for (auto *DDI : Info.DbgVariableIntrinsics) {
+ // Helper utility for adding DW_OP_LLVM_tag_offset to debug-info records,
+ // abstracted over whether they're intrinsic-stored or DPValue stored.
+ auto AnnotateDbgRecord = [&](auto *DPtr) {
// Prepend "tag_offset, N" to the dwarf expression.
// Tag offset logically applies to the alloca pointer, and it makes sense
// to put it at the beginning of the expression.
SmallVector<uint64_t, 8> NewOps = {dwarf::DW_OP_LLVM_tag_offset,
retagMask(N)};
- for (size_t LocNo = 0; LocNo < DDI->getNumVariableLocationOps(); ++LocNo)
- if (DDI->getVariableLocationOp(LocNo) == AI)
- DDI->setExpression(DIExpression::appendOpsToArg(DDI->getExpression(),
- NewOps, LocNo));
- }
+ for (size_t LocNo = 0; LocNo < DPtr->getNumVariableLocationOps(); ++LocNo)
+ if (DPtr->getVariableLocationOp(LocNo) == AI)
+ DPtr->setExpression(DIExpression::appendOpsToArg(DPtr->getExpression(),
+ NewOps, LocNo));
+ };
+
+ for (auto *DDI : Info.DbgVariableIntrinsics)
+ AnnotateDbgRecord(DDI);
+ for (auto *DPV : Info.DbgVariableRecords)
+ AnnotateDbgRecord(DPV);
auto TagEnd = [&](Instruction *Node) {
IRB.SetInsertPoint(Node);
@@ -1532,8 +1539,8 @@ void HWAddressSanitizer::sanitizeFunction(Function &F,
assert(!ShadowBase);
- Instruction *InsertPt = &*F.getEntryBlock().begin();
- IRBuilder<> EntryIRB(InsertPt);
+ BasicBlock::iterator InsertPt = F.getEntryBlock().begin();
+ IRBuilder<> EntryIRB(&F.getEntryBlock(), InsertPt);
emitPrologue(EntryIRB,
/*WithFrameRecord*/ ClRecordStackHistory != none &&
Mapping.WithFrameRecord &&
@@ -1552,12 +1559,12 @@ void HWAddressSanitizer::sanitizeFunction(Function &F,
// entry block back into the entry block so that they aren't treated as
// dynamic allocas.
if (EntryIRB.GetInsertBlock() != &F.getEntryBlock()) {
- InsertPt = &*F.getEntryBlock().begin();
+ InsertPt = F.getEntryBlock().begin();
for (Instruction &I :
llvm::make_early_inc_range(*EntryIRB.GetInsertBlock())) {
if (auto *AI = dyn_cast<AllocaInst>(&I))
if (isa<ConstantInt>(AI->getArraySize()))
- I.moveBefore(InsertPt);
+ I.moveBefore(F.getEntryBlock(), InsertPt);
}
}
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index f94047633022ca..648a5276a3d60a 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -149,6 +149,21 @@ void StackInfoBuilder::visit(Instruction &Inst) {
}
}
}
+
+ // Check for non-intrinsic debug-info records.
+ for (auto &DPV : Inst.getDbgValueRange()) {
+ for (Value *V : DPV.location_ops()) {
+ if (auto *AI = dyn_cast_or_null<AllocaInst>(V)) {
+ if (!isInterestingAlloca(*AI))
+ continue;
+ AllocaInfo &AInfo = Info.AllocasToInstrument[AI];
+ auto &DPVVec = AInfo.DbgVariableRecords;
+ if (DPVVec.empty() || DPVVec.back() != &DPV)
+ DPVVec.push_back(&DPV);
+ }
+ }
+ }
+
Instruction *ExitUntag = getUntagLocationIfFunctionExit(Inst);
if (ExitUntag)
Info.RetVec.push_back(ExitUntag);
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/RISCV/alloca.ll b/llvm/test/Instrumentation/HWAddressSanitizer/RISCV/alloca.ll
index de0bd84e2c579f..7c899095ffdef6 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/RISCV/alloca.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/RISCV/alloca.ll
@@ -42,6 +42,7 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; DYNAMIC-SHADOW-NEXT: [[HWASAN_STACK_BASE_TAG:%.*]] = xor i64 [[TMP1]], [[TMP2]]
; DYNAMIC-SHADOW-NEXT: [[HWASAN_UAR_TAG:%.*]] = lshr i64 [[TMP1]], 56
; DYNAMIC-SHADOW-NEXT: [[X:%.*]] = alloca { i32, [12 x i8] }, align 16
+; DYNAMIC-SHADOW-NEXT: call void @llvm.dbg.value(metadata !DIArgList(ptr [[X]], ptr [[X]]), metadata [[META11:![0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0, DW_OP_plus, DW_OP_deref)), !dbg [[DBG13:![0-9]+]]
; DYNAMIC-SHADOW-NEXT: [[TMP3:%.*]] = xor i64 [[HWASAN_STACK_BASE_TAG]], 0, !dbg [[DBG10:![0-9]+]]
; DYNAMIC-SHADOW-NEXT: [[TMP4:%.*]] = ptrtoint ptr [[X]] to i64, !dbg [[DBG10]]
; DYNAMIC-SHADOW-NEXT: [[TMP5:%.*]] = and i64 [[TMP4]], 72057594037927935, !dbg [[DBG10]]
@@ -57,7 +58,6 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; DYNAMIC-SHADOW-NEXT: store i8 4, ptr [[TMP13]], align 1, !dbg [[DBG10]]
; DYNAMIC-SHADOW-NEXT: [[TMP14:%.*]] = getelementptr i8, ptr [[X]], i32 15, !dbg [[DBG10]]
; DYNAMIC-SHADOW-NEXT: store i8 [[TMP8]], ptr [[TMP14]], align 1, !dbg [[DBG10]]
-; DYNAMIC-SHADOW-NEXT: call void @llvm.dbg.value(metadata !DIArgList(ptr [[X]], ptr [[X]]), metadata [[META11:![0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0, DW_OP_plus, DW_OP_deref)), !dbg [[DBG13:![0-9]+]]
; DYNAMIC-SHADOW-NEXT: call void @use32(ptr nonnull [[X_HWASAN]]), !dbg [[DBG10]]
; DYNAMIC-SHADOW-NEXT: [[TMP15:%.*]] = trunc i64 [[HWASAN_UAR_TAG]] to i8, !dbg [[DBG14:![0-9]+]]
; DYNAMIC-SHADOW-NEXT: [[TMP16:%.*]] = ptrtoint ptr [[X]] to i64, !dbg [[DBG14]]
@@ -77,6 +77,7 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; ZERO-BASED-SHADOW-NEXT: [[HWASAN_STACK_BASE_TAG:%.*]] = xor i64 [[TMP1]], [[TMP2]]
; ZERO-BASED-SHADOW-NEXT: [[HWASAN_UAR_TAG:%.*]] = lshr i64 [[TMP1]], 56
; ZERO-BASED-SHADOW-NEXT: [[X:%.*]] = alloca { i32, [12 x i8] }, align 16
+; ZERO-BASED-SHADOW-NEXT: call void @llvm.dbg.value(metadata !DIArgList(ptr [[X]], ptr [[X]]), metadata [[META11:![0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0, DW_OP_plus, DW_OP_deref)), !dbg [[DBG13:![0-9]+]]
; ZERO-BASED-SHADOW-NEXT: [[TMP3:%.*]] = xor i64 [[HWASAN_STACK_BASE_TAG]], 0, !dbg [[DBG10:![0-9]+]]
; ZERO-BASED-SHADOW-NEXT: [[TMP4:%.*]] = ptrtoint ptr [[X]] to i64, !dbg [[DBG10]]
; ZERO-BASED-SHADOW-NEXT: [[TMP5:%.*]] = and i64 [[TMP4]], 72057594037927935, !dbg [[DBG10]]
@@ -92,7 +93,6 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; ZERO-BASED-SHADOW-NEXT: store i8 4, ptr [[TMP13]], align 1, !dbg [[DBG10]]
; ZERO-BASED-SHADOW-NEXT: [[TMP14:%.*]] = getelementptr i8, ptr [[X]], i32 15, !dbg [[DBG10]]
; ZERO-BASED-SHADOW-NEXT: store i8 [[TMP8]], ptr [[TMP14]], align 1, !dbg [[DBG10]]
-; ZERO-BASED-SHADOW-NEXT: call void @llvm.dbg.value(metadata !DIArgList(ptr [[X]], ptr [[X]]), metadata [[META11:![0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0, DW_OP_plus, DW_OP_deref)), !dbg [[DBG13:![0-9]+]]
; ZERO-BASED-SHADOW-NEXT: call void @use32(ptr nonnull [[X_HWASAN]]), !dbg [[DBG10]]
; ZERO-BASED-SHADOW-NEXT: [[TMP15:%.*]] = trunc i64 [[HWASAN_UAR_TAG]] to i8, !dbg [[DBG14:![0-9]+]]
; ZERO-BASED-SHADOW-NEXT: [[TMP16:%.*]] = ptrtoint ptr [[X]] to i64, !dbg [[DBG14]]
@@ -153,10 +153,10 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
; DYNAMIC-SHADOW: [[DBG7]] = distinct !DISubprogram(name: "test_alloca", linkageName: "_Z11test_allocav", scope: !2, file: !2, line: 4, type: !8, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !3)
; DYNAMIC-SHADOW: [[META8:![0-9]+]] = !DISubroutineType(types: !9)
; DYNAMIC-SHADOW: [[META9:![0-9]+]] = !{null}
-; DYNAMIC-SHADOW: [[DBG10]] = !DILocation(line: 7, column: 5, scope: !7)
-; DYNAMIC-SHADOW: [[META11]] = !DILocalVariable(name: "x", scope: !7, file: !2, line: 5, type: !12)
-; DYNAMIC-SHADOW: [[META12:![0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+; DYNAMIC-SHADOW: [[META11]] = !DILocalVariable(name: "x", scope: !7, file: !2, line: 5, type: [[META12:![0-9]+]])
+; DYNAMIC-SHADOW: [[META12]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
; DYNAMIC-SHADOW: [[DBG13]] = !DILocation(line: 0, scope: !7)
+; DYNAMIC-SHADOW: [[DBG10]] = !DILocation(line: 7, column: 5, scope: !7)
; DYNAMIC-SHADOW: [[DBG14]] = !DILocation(line: 8, column: 1, scope: !7)
;.
; ZERO-BASED-SHADOW: [[META0:![0-9]+]] = !{ptr @hwasan.note}
@@ -169,9 +169,9 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
; ZERO-BASED-SHADOW: [[DBG7]] = distinct !DISubprogram(name: "test_alloca", linkageName: "_Z11test_allocav", scope: !2, file: !2, line: 4, type: !8, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !3)
; ZERO-BASED-SHADOW: [[META8:![0-9]+]] = !DISubroutineType(types: !9)
; ZERO-BASED-SHADOW: [[META9:![0-9]+]] = !{null}
-; ZERO-BASED-SHADOW: [[DBG10]] = !DILocation(line: 7, column: 5, scope: !7)
-; ZERO-BASED-SHADOW: [[META11]] = !DILocalVariable(name: "x", scope: !7, file: !2, line: 5, type: !12)
-; ZERO-BASED-SHADOW: [[META12:![0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+; ZERO-BASED-SHADOW: [[META11]] = !DILocalVariable(name: "x", scope: !7, file: !2, line: 5, type: [[META12:![0-9]+]])
+; ZERO-BASED-SHADOW: [[META12]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
; ZERO-BASED-SHADOW: [[DBG13]] = !DILocation(line: 0, scope: !7)
+; ZERO-BASED-SHADOW: [[DBG10]] = !DILocation(line: 7, column: 5, scope: !7)
; ZERO-BASED-SHADOW: [[DBG14]] = !DILocation(line: 8, column: 1, scope: !7)
;.
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll b/llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll
index 7707c90506e3e6..8db14222652f51 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll
@@ -5,6 +5,9 @@
; RUN: opt < %s -passes=hwasan -hwasan-with-ifunc=1 -S | FileCheck %s --check-prefixes=DYNAMIC-SHADOW
; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=ZERO-BASED-SHADOW
+; RUN: opt < %s -passes=hwasan -hwasan-with-ifunc=1 -S --try-experimental-debuginfo-iterators | FileCheck %s --check-prefixes=DYNAMIC-SHADOW
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset=0 -S --try-experimental-debuginfo-iterators | FileCheck %s --check-prefixes=ZERO-BASED-SHADOW
+
target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
target triple = "aarch64--linux-android10000"
@@ -40,6 +43,7 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; DYNAMIC-SHADOW-NEXT: [[HWASAN_STACK_BASE_TAG:%.*]] = xor i64 [[TMP1]], [[TMP2]]
; DYNAMIC-SHADOW-NEXT: [[HWASAN_UAR_TAG:%.*]] = lshr i64 [[TMP1]], 56
; DYNAMIC-SHADOW-NEXT: [[X:%.*]] = alloca { i32, [12 x i8] }, align 16
+; DYNAMIC-SHADOW-NEXT: call void @llvm.dbg.value(metadata !DIArgList(ptr [[X]], ptr [[X]]), metadata [[META11:![0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0, DW_OP_plus, DW_OP_deref)), !dbg [[DBG13:![0-9]+]]
; DYNAMIC-SHADOW-NEXT: [[TMP3:%.*]] = xor i64 [[HWASAN_STACK_BASE_TAG]], 0, !dbg [[DBG10:![0-9]+]]
; DYNAMIC-SHADOW-NEXT: [[TMP4:%.*]] = ptrtoint ptr [[X]] to i64, !dbg [[DBG10]]
; DYNAMIC-SHADOW-NEXT: [[TMP5:%.*]] = and i64 [[TMP4]], 72057594037927935, !dbg [[DBG10]]
@@ -55,7 +59,6 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; DYNAMIC-SHADOW-NEXT: store i8 4, ptr [[TMP13]], align 1, !dbg [[DBG10]]
; DYNAMIC-SHADOW-NEXT: [[TMP14:%.*]] = getelementptr i8, ptr [[X]], i32 15, !dbg [[DBG10]]
; DYNAMIC-SHADOW-NEXT: store i8 [[TMP8]], ptr [[TMP14]], align 1, !dbg [[DBG10]]
-; DYNAMIC-SHADOW-NEXT: call void @llvm.dbg.value(metadata !DIArgList(ptr [[X]], ptr [[X]]), metadata [[META11:![0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0, DW_OP_plus, DW_OP_deref)), !dbg [[DBG13:![0-9]+]]
; DYNAMIC-SHADOW-NEXT: call void @use32(ptr nonnull [[X_HWASAN]]), !dbg [[DBG10]]
; DYNAMIC-SHADOW-NEXT: [[TMP15:%.*]] = trunc i64 [[HWASAN_UAR_TAG]] to i8, !dbg [[DBG14:![0-9]+]]
; DYNAMIC-SHADOW-NEXT: [[TMP16:%.*]] = ptrtoint ptr [[X]] to i64, !dbg [[DBG14]]
@@ -75,6 +78,7 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; ZERO-BASED-SHADOW-NEXT: [[HWASAN_STACK_BASE_TAG:%.*]] = xor i64 [[TMP1]], [[TMP2]]
; ZERO-BASED-SHADOW-NEXT: [[HWASAN_UAR_TAG:%.*]] = lshr i64 [[TMP1]], 56
; ZERO-BASED-SHADOW-NEXT: [[X:%.*]] = alloca { i32, [12 x i8] }, align 16
+; ZERO-BASED-SHADOW-NEXT: call void @llvm.dbg.value(metadata !DIArgList(ptr [[X]], ptr [[X]]), metadata [[META11:![0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0, DW_OP_plus, DW_OP_deref)), !dbg [[DBG13:![0-9]+]]
; ZERO-BASED-SHADOW-NEXT: [[TMP3:%.*]] = xor i64 [[HWASAN_STACK_BASE_TAG]], 0, !dbg [[DBG10:![0-9]+]]
; ZERO-BASED-SHADOW-NEXT: [[TMP4:%.*]] = ptrtoint ptr [[X]] to i64, !dbg [[DBG10]]
; ZERO-BASED-SHADOW-NEXT: [[TMP5:%.*]] = and i64 [[TMP4]], 72057594037927935, !dbg [[DBG10]]
@@ -90,7 +94,6 @@ define void @test_alloca() sanitize_hwaddress !dbg !15 {
; ZERO-BASED-SHADOW-NEXT: store i8 4, ptr [[TMP13]], align 1, !dbg [[DBG10]]
; ZERO-BASED-SHADOW-NEXT: [[TMP14:%.*]] = getelementptr i8, ptr [[X]], i32 15, !dbg [[DBG10]]
; ZERO-BASED-SHADOW-NEXT: store i8 [[TMP8]], ptr [[TMP14]], align 1, !dbg [[DBG10]]
-; ZERO-BASED-SHADOW-NEXT: call void @llvm.dbg.value(metadata !DIArgList(ptr [[X]], ptr [[X]]), metadata [[META11:![0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_tag_offset, 0, DW_OP_LLVM_arg, 1, DW_OP_LLVM_tag_offset, 0, DW_OP_plus, DW_OP_deref)), !dbg [[DBG13:![0-9]+]]
; ZERO-BASED-SHADOW-NEXT: call void @use32(ptr nonnull [[X_HWASAN]]), !dbg [[DBG10]]
; ZERO-BASED-SHADOW-NEXT: [[TMP15:%.*]] = trunc i64 [[HWASAN_UAR_TAG]] to i8, !dbg [[DBG14:![0-9]+]]
; ZERO-BASED-SHADOW-NEXT: [[TMP16:%.*]] = ptrtoint ptr [[X]] to i64, !dbg [[DBG14]]
@@ -151,10 +154,10 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
; DYNAMIC-SHADOW: [[DBG7]] = distinct !DISubprogram(name: "test_alloca", linkageName: "_Z11test_allocav", scope: !2, file: !2, line: 4, type: !8, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !3)
; DYNAMIC-SHADOW: [[META8:![0-9]+]] = !DISubroutineType(types: !9)
; DYNAMIC-SHADOW: [[META9:![0-9]+]] = !{null}
-; DYNAMIC-SHADOW: [[DBG10]] = !DILocation(line: 7, column: 5, scope: !7)
-; DYNAMIC-SHADOW: [[META11]] = !DILocalVariable(name: "x", scope: !7, file: !2, line: 5, type: !12)
-; DYNAMIC-SHADOW: [[META12:![0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+; DYNAMIC-SHADOW: [[META11]] = !DILocalVariable(name: "x", scope: !7, file: !2, line: 5, type: ![[META12:[0-9]+]])
+; DYNAMIC-SHADOW: [[META12]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
; DYNAMIC-SHADOW: [[DBG13]] = !DILocation(line: 0, scope: !7)
+; DYNAMIC-SHADOW: [[DBG10]] = !DILocation(line: 7, column: 5, scope: !7)
; DYNAMIC-SHADOW: [[DBG14]] = !DILocation(line: 8, column: 1, scope: !7)
;.
; ZERO-BASED-SHADOW: [[META0:![0-9]+]] = !{ptr @hwasan.note}
@@ -167,9 +170,9 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
; ZERO-BASED-SHADOW: [[DBG7]] = distinct !DISubprogram(name: "test_alloca", linkageName: "_Z11test_allocav", scope: !2, file: !2, line: 4, type: !8, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !3)
; ZERO-BASED-SHADOW: [[META8:![0-9]+]] = !DISubroutineType(types: !9)
; ZERO-BASED-SHADOW: [[META9:![0-9]+]] = !{null}
-; ZERO-BASED-SHADOW: [[DBG10]] = !DILocation(line: 7, column: 5, scope: !7)
-; ZERO-BASED-SHADOW: [[META11]] = !DILocalVariable(name: "x", scope: !7, file: !2, line: 5, type: !12)
-; ZERO-BASED-SHADOW: [[META12:![0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+; ZERO-BASED-SHADOW: [[META11]] = !DILocalVariable(name: "x", scope: !7, file: !2, line: 5, type: ![[META12:[0-9]+]])
+; ZERO-BASED-SHADOW: [[META12]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
; ZERO-BASED-SHADOW: [[DBG13]] = !DILocation(line: 0, scope: !7)
+; ZERO-BASED-SHADOW: [[DBG10]] = !DILocation(line: 7, column: 5, scope: !7)
; ZERO-BASED-SHADOW: [[DBG14]] = !DILocation(line: 8, column: 1, scope: !7)
;.
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
fmayer
reviewed
Jan 19, 2024
|
Just to explain the addition of And that becomes (without the patch): Right now, if someone operating a debugger requests a breakpoint on line 2 then they'll very likely end up stopped on the hwasan instrumentation instructions, and won't be able to see any variable assignments that were supposed to happen before line 2, which creates a small risk that they'll get confused. Using Now when someone requests a breakpoint on line 2, they'll still stop on the hwasan instrumentation instructions, but all the variable assignments that happen before line 2 will be visible. It sort of makes the addition of the instrumentation more transparent to the developer. Normally this sort of change deserves it's own patch -- however it's inherently connected with the debuginfo iterators change, as what we're doing there effectively makes |
fmayer
approved these changes
Jan 23, 2024
Please fix the formatting |
|
How will this interact with #78606 |
|
Thanks for the reviews,
SGTM, landing with formatting fixes, |
fmayer
reviewed
Feb 2, 2024
| @@ -578,6 +578,8 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) { | |||
| // Fixup debug intrinsics to point to the new alloca. | |||
| for (auto *DVI : Info.DbgVariableIntrinsics) | |||
| DVI->replaceVariableLocationOp(OldAI, Info.AI); | |||
| for (auto *DPV : Info.DbgVariableRecords) | |||
There was a problem hiding this comment.
This seems untested. I can comment this out and
build/bin/llvm-lit -v llvm/test/CodeGen/AArch64/
still passes.
There was a problem hiding this comment.
It's not tested under the default configuration and so the lack-of-failure is expected. To ensure the code isn't rotting, we've got this [0] buildbot set up to exercise the "new debug-info" configuration, which requires passing LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS=On to cmake. With that enabled, the test RUN lines where --try-experimental-debuginfo-iterators has been added will operate under "new" debug-info mode, and you Should (TM) see a test failure.
We're inches away from turning this on-by-default and it being tested everywhere, after which there won't be this obscure testing arrangement happening any more.
There was a problem hiding this comment.
Oh thanks! I'll keep that in mind. I wanted to change this code a bit so wanted to make sure I'm actually running the tests, now I know how.
There was a problem hiding this comment.
I re-rean this with -DLLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS=On in the CMake but it still passes if I comment out these lines.
jmorse
added a commit
that referenced
this pull request
Feb 6, 2024
In github PR #78731 it looks like I added test coverage for RemoveDIs to either the wrong test, or not enough. Adding --try-experimental-debuginfo-iterators to this particular test is enough to restore some coverage it seems.
|
/me squints -- either we added the coverage to the wrong file, or something else weird has gone wrong. Either way, in 5ce2f73 I've added another |
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.
This patch extends HWASAN to support maintenance of debug-info that isn't stored as intrinsics, but is instead in a DPValue object. This is straight-forwards: we collect any such objects in StackInfoBuilder, and apply the same operations to them as we would to dbg.value and similar intrinsics.
I've also replaced some calls to getNextNode with debug-info skipping next calls, and use iterators for instruction insertion rather than instruction pointers. This avoids any difference in output between intrinsic / non-intrinsic debug-info, but also means that any debug-info comes before code inserted by HWAsan, rather than afterwards. See the test modifications, where the variable assignment (presented as a dbg.value) jumps up over all the code inserted by HWAsan. Seeing how the code inserted by HWAsan is always (AFAIUI) given the source-location of the instruction being instrumented, I don't believe this will have any effect on which lines variable assignments become visible on; it may extend the number of instructions covered by the assignments though.