maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[Local] Do not move around dbg.declares during replaceDbgDeclare 

replaceDbgDeclare is used to update the descriptions of stack variables
when they are moved (e.g. by ASan or SafeStack). A side effect of
replaceDbgDeclare is that it moves dbg.declares around in the
instruction stream (typically by hoisting them into the entry block).
This behavior was introduced in llvm/r227544 to fix an assertion failure
(llvm.org/PR22386), but no longer appears to be necessary.

Hoisting a dbg.declare generally does not create problems. Usually,
dbg.declare either describes an argument or an alloca in the entry
block, and backends have special handling to emit locations for these.
In optimized builds, LowerDbgDeclare places dbg.values in the right
spots regardless of where the dbg.declare is. And no one uses
replaceDbgDeclare to handle things like VLAs.

However, there doesn't seem to be a positive case for moving
dbg.declares around anymore, and this reordering can get in the way of
understanding other bugs. I propose getting rid of it.

Testing: stage2 RelWithDebInfo sanitized build, check-llvm

rdar://59397340

Differential Revision: https://reviews.llvm.org/D74517
This commit is contained in:
Vedant Kumar 2020-02-10 15:37:56 -08:00
parent 19b62b79db
commit 8e77b33b3c
11 changed files with 35 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
llvm/include/llvm/Transforms/Utils/Local.h
View File

@ -332,20 +332,9 @@ void findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgInsts, Value *V);
/// additional DW_OP_deref is prepended to the expression. If Offset
/// is non-zero, a constant displacement is added to the expression
/// (between the optional Deref operations). Offset can be negative.
bool replaceDbgDeclare(Value *Address, Value *NewAddress,
Instruction *InsertBefore, DIBuilder &Builder,
bool replaceDbgDeclare(Value *Address, Value *NewAddress, DIBuilder &Builder,
uint8_t DIExprFlags, int Offset);
/// Replaces llvm.dbg.declare instruction when the alloca it describes
/// is replaced with a new value. If Deref is true, an additional
/// DW_OP_deref is prepended to the expression. If Offset is non-zero,
/// a constant displacement is added to the expression (between the
/// optional Deref operations). Offset can be negative. The new
/// llvm.dbg.declare is inserted immediately after AI.
bool replaceDbgDeclareForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
DIBuilder &Builder, uint8_t DIExprFlags,
int Offset);
/// Replaces multiple llvm.dbg.value instructions when the alloca it describes
/// is replaced with a new value. If Offset is non-zero, a constant displacement
/// is added to the expression (after the mandatory Deref). Offset can be

9
llvm/lib/CodeGen/SafeStack.cpp
View File

@ -576,8 +576,8 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
Arg->getName() + ".unsafe-byval");
// Replace alloc with the new location.
replaceDbgDeclare(Arg, BasePointer, BasePointer->getNextNode(), DIB,
DIExpression::ApplyOffset, -Offset);
replaceDbgDeclare(Arg, BasePointer, DIB, DIExpression::ApplyOffset,
-Offset);
Arg->replaceAllUsesWith(NewArg);
IRB.SetInsertPoint(cast<Instruction>(NewArg)->getNextNode());
IRB.CreateMemCpy(Off, Align, Arg, Arg->getParamAlign(), Size);
@ -588,8 +588,7 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
IRB.SetInsertPoint(AI);
unsigned Offset = SSL.getObjectOffset(AI);
replaceDbgDeclareForAlloca(AI, BasePointer, DIB, DIExpression::ApplyOffset,
-Offset);
replaceDbgDeclare(AI, BasePointer, DIB, DIExpression::ApplyOffset, -Offset);
replaceDbgValueForAlloca(AI, BasePointer, DIB, -Offset);
// Replace uses of the alloca with the new location.
@ -676,7 +675,7 @@ void SafeStack::moveDynamicAllocasToUnsafeStack(
if (AI->hasName() && isa<Instruction>(NewAI))
NewAI->takeName(AI);
replaceDbgDeclareForAlloca(AI, NewAI, DIB, DIExpression::ApplyOffset, 0);
replaceDbgDeclare(AI, NewAI, DIB, DIExpression::ApplyOffset, 0);
AI->replaceAllUsesWith(NewAI);
AI->eraseFromParent();
}

4
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
View File

@ -3132,8 +3132,8 @@ void FunctionStackPoisoner::processStaticAllocas() {
// Replace Alloca instructions with base+offset.
for (const auto &Desc : SVD) {
AllocaInst *AI = Desc.AI;
replaceDbgDeclareForAlloca(AI, LocalStackBaseAllocaPtr, DIB, DIExprFlags,
Desc.Offset);
replaceDbgDeclare(AI, LocalStackBaseAllocaPtr, DIB, DIExprFlags,
Desc.Offset);
Value *NewAllocaPtr = IRB.CreateIntToPtr(
IRB.CreateAdd(LocalStackBase, ConstantInt::get(IntptrTy, Desc.Offset)),
AI->getType());

4
llvm/lib/Transforms/Utils/InlineFunction.cpp
View File

@ -1848,10 +1848,6 @@ llvm::InlineResult llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
Caller->getEntryBlock().getInstList().splice(
InsertPoint, FirstNewBlock->getInstList(), AI->getIterator(), I);
}
// Move any dbg.declares describing the allocas into the entry basic block.
DIBuilder DIB(*Caller->getParent());
for (auto &AI : IFI.StaticAllocas)
replaceDbgDeclareForAlloca(AI, AI, DIB, DIExpression::ApplyOffset, 0);
}
SmallVector<Value*,4> VarArgsToForward;

17
llvm/lib/Transforms/Utils/Local.cpp
View File

@ -1568,8 +1568,8 @@ void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
}
bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
Instruction *InsertBefore, DIBuilder &Builder,
uint8_t DIExprFlags, int Offset) {
DIBuilder &Builder, uint8_t DIExprFlags,
int Offset) {
auto DbgAddrs = FindDbgAddrUses(Address);
for (DbgVariableIntrinsic *DII : DbgAddrs) {
DebugLoc Loc = DII->getDebugLoc();
@ -1577,23 +1577,14 @@ bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
auto *DIExpr = DII->getExpression();
assert(DIVar && "Missing variable");
DIExpr = DIExpression::prepend(DIExpr, DIExprFlags, Offset);
// Insert llvm.dbg.declare immediately before InsertBefore, and remove old
// Insert llvm.dbg.declare immediately before DII, and remove old
// llvm.dbg.declare.
Builder.insertDeclare(NewAddress, DIVar, DIExpr, Loc, InsertBefore);
if (DII == InsertBefore)
InsertBefore = InsertBefore->getNextNode();
Builder.insertDeclare(NewAddress, DIVar, DIExpr, Loc, DII);
DII->eraseFromParent();
}
return !DbgAddrs.empty();
}
bool llvm::replaceDbgDeclareForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
DIBuilder &Builder, uint8_t DIExprFlags,
int Offset) {
return replaceDbgDeclare(AI, NewAllocaAddress, AI->getNextNode(), Builder,
DIExprFlags, Offset);
}
static void replaceOneDbgValueForAlloca(DbgValueInst *DVI, Value *NewAddress,
DIBuilder &Builder, int Offset) {
DebugLoc Loc = DVI->getDebugLoc();

6
llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
View File

@ -21,15 +21,11 @@ entry:
}
; CHECK: define i32 @_Z3zzzi
; CHECK: entry:
; Verify that llvm.dbg.declare calls are in the entry basic block.
; CHECK-NEXT: [[MyAlloca:%.*]] = alloca i8, i64 64
; CHECK-NOT: %entry
; CHECK: [[MyAlloca:%.*]] = alloca i8, i64 64
; Note: these dbg.declares used to contain `ptrtoint` operands. The instruction
; selector would then decline to put the variable in the MachineFunction side
; table. Check that the dbg.declares have `alloca` operands.
; CHECK: call void @llvm.dbg.declare(metadata i8* [[MyAlloca]], metadata ![[ARG_ID:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 32))
; CHECK-NOT: %entry
; CHECK: call void @llvm.dbg.declare(metadata i8* [[MyAlloca]], metadata ![[VAR_ID:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 48))
declare void @llvm.dbg.declare(metadata, metadata, metadata) nounwind readnone

6
llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
View File

@ -19,12 +19,16 @@ entry:
; CHECK: %[[ALLOCA:.*]] = ptrtoint i8* %MyAlloca to i64
; CHECK: %[[PHI:.*]] = phi i64 {{.*}} %[[ALLOCA]],
; CHECK: store i64 %[[PHI]], i64* %asan_local_stack_base
; CHECK: call void @llvm.dbg.declare(metadata i64* %asan_local_stack_base, metadata !12, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 32)), !dbg !13
; CHECK: call void @llvm.dbg.declare(metadata i64* %asan_local_stack_base, metadata [[VAR_I:![0-9]+]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 32)), !dbg [[LOC_I:![0-9]+]]
%0 = load i32, i32* %i.addr, align 4, !dbg !14
%add = add nsw i32 %0, 2, !dbg !15
ret i32 %add, !dbg !16
}
; CHECK: [[SCOPE:![0-9]+]] = distinct !DISubprogram(name: "foo"
; CHECK: [[VAR_I]] = !DILocalVariable(name: "i", arg: 1, scope: [[SCOPE]]
; CHECK: [[LOC_I]] = !DILocation(line: 1, column: 13, scope: [[SCOPE]]
; Function Attrs: nounwind readnone speculatable
declare void @llvm.dbg.declare(metadata, metadata, metadata) #1

16
llvm/test/Transforms/Inline/alloca-dbgdeclare-merge.ll
View File

@ -1,6 +1,5 @@
; Test that alloca merging in the inliner places dbg.declare calls immediately
; after the merged alloca. Not at the end of the entry BB, and definitely not
; before the alloca.
; Test that alloca merging in the inliner places dbg.declare calls after the
; merged alloca, but does not otherwise reorder them.
;
; clang -g -S -emit-llvm -Xclang -disable-llvm-optzns
;
@ -20,13 +19,20 @@
;}
;
; RUN: opt -always-inline -S < %s | FileCheck %s
; FIXME: Why does the dbg.declare for "aaa" occur later in @h than the
; dbg.declare for "bbb"? I'd expect the opposite, given @f is inlined earlier.
;
; CHECK: define void @h()
; CHECK-NEXT: entry:
; CHECK-NEXT: %[[AI:.*]] = alloca [100 x i8]
; CHECK-NEXT: call void @llvm.dbg.declare(metadata [100 x i8]* %[[AI]],
; CHECK-NEXT: call void @llvm.dbg.declare(metadata [100 x i8]* %[[AI]],
; CHECK-NEXT: call void @llvm.dbg.declare(metadata [100 x i8]* %[[AI]], metadata [[BBB:![0-9]+]]
; CHECK-NEXT: bitcast
; CHECK-NEXT: llvm.lifetime.start
; CHECK-NEXT: call void @llvm.dbg.declare(metadata [100 x i8]* %[[AI]], metadata [[AAA:![0-9]+]]
; CHECK: [[AAA]] = !DILocalVariable(name: "aaa"
; CHECK: [[BBB]] = !DILocalVariable(name: "bbb"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

4
llvm/test/Transforms/Inline/alloca-dbgdeclare.ll
View File

@ -42,6 +42,10 @@ entry:
; CHECK: define void @_Z3fn5v()
; CHECK-NEXT: entry:
; CHECK-NEXT: %agg.tmp.sroa.3.i = alloca [20 x i8], align 4
; CHECK-NEXT: br label %while.body
; CHECK: while.body:
; CHECK-NEXT: bitcast
; CHECK-NEXT: llvm.lifetime.start
; CHECK-NEXT: call void @llvm.dbg.declare(metadata [20 x i8]* %agg.tmp.sroa.3.i,
%agg.tmp.sroa.3 = alloca [20 x i8], align 4
tail call void @llvm.dbg.declare(metadata [20 x i8]* %agg.tmp.sroa.3, metadata !25, metadata !30), !dbg !31

3
llvm/test/Transforms/Inline/inline_dbg_declare.ll
View File

@ -42,7 +42,8 @@ declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
define void @bar(float* %dst) #0 !dbg !9 {
entry:
; CHECK: [[x_addr_i:%[a-zA-Z0-9.]+]] = alloca float, align 4
; CHECK: [[x_addr_i:%.+]] = alloca float, align 4
; CHECK: store float {{.*}}, float* [[x_addr_i]]
; CHECK-NEXT: void @llvm.dbg.declare(metadata float* [[x_addr_i]], metadata [[m23:![0-9]+]], metadata !DIExpression()), !dbg [[m24:![0-9]+]]
%dst.addr = alloca float*, align 4

2
llvm/unittests/Transforms/Utils/LocalTest.cpp
View File

@ -154,7 +154,7 @@ TEST(Local, ReplaceDbgDeclare) {
ASSERT_TRUE(DII);
Value *NewBase = Constant::getNullValue(Type::getInt32PtrTy(C));
DIBuilder DIB(*M);
replaceDbgDeclare(AI, NewBase, DII, DIB, DIExpression::ApplyOffset, 0);
replaceDbgDeclare(AI, NewBase, DIB, DIExpression::ApplyOffset, 0);
// There should be exactly two dbg.declares.
int Declares = 0;