From cb334476f1dfdfc662bbfe85576b800367fa0f68 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Wed, 15 Apr 2015 21:18:07 +0000 Subject: [PATCH] DebugInfo: Require a DebugLoc in DIBuilder::insertDeclare() Change `DIBuilder::insertDeclare()` and `insertDbgValueIntrinsic()` to take an `MDLocation*`/`DebugLoc` parameter which it attaches to the created intrinsic. Assert at creation time that the `scope:` field's subprogram matches the variable's. There's a matching `clang` commit to use the API. The context for this is PR22778, which is removing the `inlinedAt:` field from `MDLocalVariable`, instead deferring to the `!dbg` location attached to the debug info intrinsic. The best way to ensure we always have a `!dbg` attachment is to require one at creation time. I'll be adding verifier checks next, but this API change is the best way to shake out frontend bugs. Note: I added an `llvm_unreachable()` in `bindings/go` and passed in `nullptr` for the `DebugLoc`. The `llgo` folks will eventually need to pass a valid `DebugLoc` here. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@235041 91177308-0d34-0410-b5e6-96231b3b80d8 --- bindings/go/llvm/DIBuilderBindings.cpp | 16 ++++++-- include/llvm/IR/DIBuilder.h | 12 +++++- lib/IR/DIBuilder.cpp | 39 +++++++++++++++---- lib/Transforms/Scalar/SROA.cpp | 11 +++--- .../Scalar/ScalarReplAggregates.cpp | 7 ++-- lib/Transforms/Utils/Local.cpp | 21 +++++----- unittests/Transforms/Utils/Cloning.cpp | 6 ++- 7 files changed, 74 insertions(+), 38 deletions(-) diff --git a/bindings/go/llvm/DIBuilderBindings.cpp b/bindings/go/llvm/DIBuilderBindings.cpp index ee2e70a579b..e5734a7c0f3 100644 --- a/bindings/go/llvm/DIBuilderBindings.cpp +++ b/bindings/go/llvm/DIBuilderBindings.cpp @@ -234,10 +234,14 @@ LLVMValueRef LLVMDIBuilderInsertDeclareAtEnd(LLVMDIBuilderRef Dref, LLVMMetadataRef VarInfo, LLVMMetadataRef Expr, LLVMBasicBlockRef Block) { + // Fail immediately here until the llgo folks update their bindings. The + // called function is going to assert out anyway. + llvm_unreachable("DIBuilder API change requires a DebugLoc"); + DIBuilder *D = unwrap(Dref); - Instruction *Instr = - D->insertDeclare(unwrap(Storage), unwrap(VarInfo), - unwrap(Expr), unwrap(Block)); + Instruction *Instr = D->insertDeclare( + unwrap(Storage), unwrap(VarInfo), + unwrap(Expr), /* DebugLoc */ nullptr, unwrap(Block)); return wrap(Instr); } @@ -246,9 +250,13 @@ LLVMValueRef LLVMDIBuilderInsertValueAtEnd(LLVMDIBuilderRef Dref, LLVMMetadataRef VarInfo, LLVMMetadataRef Expr, LLVMBasicBlockRef Block) { + // Fail immediately here until the llgo folks update their bindings. The + // called function is going to assert out anyway. + llvm_unreachable("DIBuilder API change requires a DebugLoc"); + DIBuilder *D = unwrap(Dref); Instruction *Instr = D->insertDbgValueIntrinsic( unwrap(Val), Offset, unwrap(VarInfo), - unwrap(Expr), unwrap(Block)); + unwrap(Expr), /* DebugLoc */ nullptr, unwrap(Block)); return wrap(Instr); } diff --git a/include/llvm/IR/DIBuilder.h b/include/llvm/IR/DIBuilder.h index ac82947741c..cadfa3165c3 100644 --- a/include/llvm/IR/DIBuilder.h +++ b/include/llvm/IR/DIBuilder.h @@ -656,26 +656,32 @@ namespace llvm { /// @param Storage llvm::Value of the variable /// @param VarInfo Variable's debug info descriptor. /// @param Expr A complex location expression. + /// @param DL Debug info location. /// @param InsertAtEnd Location for the new intrinsic. Instruction *insertDeclare(llvm::Value *Storage, DIVariable VarInfo, - DIExpression Expr, BasicBlock *InsertAtEnd); + DIExpression Expr, const MDLocation *DL, + BasicBlock *InsertAtEnd); /// insertDeclare - Insert a new llvm.dbg.declare intrinsic call. /// @param Storage llvm::Value of the variable /// @param VarInfo Variable's debug info descriptor. /// @param Expr A complex location expression. + /// @param DL Debug info location. /// @param InsertBefore Location for the new intrinsic. Instruction *insertDeclare(llvm::Value *Storage, DIVariable VarInfo, - DIExpression Expr, Instruction *InsertBefore); + DIExpression Expr, const MDLocation *DL, + Instruction *InsertBefore); /// insertDbgValueIntrinsic - Insert a new llvm.dbg.value intrinsic call. /// @param Val llvm::Value of the variable /// @param Offset Offset /// @param VarInfo Variable's debug info descriptor. /// @param Expr A complex location expression. + /// @param DL Debug info location. /// @param InsertAtEnd Location for the new intrinsic. Instruction *insertDbgValueIntrinsic(llvm::Value *Val, uint64_t Offset, DIVariable VarInfo, DIExpression Expr, + const MDLocation *DL, BasicBlock *InsertAtEnd); /// insertDbgValueIntrinsic - Insert a new llvm.dbg.value intrinsic call. @@ -683,9 +689,11 @@ namespace llvm { /// @param Offset Offset /// @param VarInfo Variable's debug info descriptor. /// @param Expr A complex location expression. + /// @param DL Debug info location. /// @param InsertBefore Location for the new intrinsic. Instruction *insertDbgValueIntrinsic(llvm::Value *Val, uint64_t Offset, DIVariable VarInfo, DIExpression Expr, + const MDLocation *DL, Instruction *InsertBefore); /// \brief Replace the vtable holder in the given composite type. diff --git a/lib/IR/DIBuilder.cpp b/lib/IR/DIBuilder.cpp index 1f087888028..654dafca1b9 100644 --- a/lib/IR/DIBuilder.cpp +++ b/lib/IR/DIBuilder.cpp @@ -761,10 +761,19 @@ static Value *getDbgIntrinsicValueImpl(LLVMContext &VMContext, Value *V) { return MetadataAsValue::get(VMContext, ValueAsMetadata::get(V)); } +static Instruction *withDebugLoc(Instruction *I, const MDLocation *DL) { + I->setDebugLoc(const_cast(DL)); + return I; +} + Instruction *DIBuilder::insertDeclare(Value *Storage, DIVariable VarInfo, - DIExpression Expr, + DIExpression Expr, const MDLocation *DL, Instruction *InsertBefore) { assert(VarInfo && "empty or invalid DIVariable passed to dbg.declare"); + assert(DL && "Expected debug loc"); + assert(DL->getScope()->getSubprogram() == + VarInfo->getScope()->getSubprogram() && + "Expected matching subprograms"); if (!DeclareFn) DeclareFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_declare); @@ -773,13 +782,17 @@ Instruction *DIBuilder::insertDeclare(Value *Storage, DIVariable VarInfo, Value *Args[] = {getDbgIntrinsicValueImpl(VMContext, Storage), MetadataAsValue::get(VMContext, VarInfo), MetadataAsValue::get(VMContext, Expr)}; - return CallInst::Create(DeclareFn, Args, "", InsertBefore); + return withDebugLoc(CallInst::Create(DeclareFn, Args, "", InsertBefore), DL); } Instruction *DIBuilder::insertDeclare(Value *Storage, DIVariable VarInfo, - DIExpression Expr, + DIExpression Expr, const MDLocation *DL, BasicBlock *InsertAtEnd) { assert(VarInfo && "empty or invalid DIVariable passed to dbg.declare"); + assert(DL && "Expected debug loc"); + assert(DL->getScope()->getSubprogram() == + VarInfo->getScope()->getSubprogram() && + "Expected matching subprograms"); if (!DeclareFn) DeclareFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_declare); @@ -792,17 +805,21 @@ Instruction *DIBuilder::insertDeclare(Value *Storage, DIVariable VarInfo, // If this block already has a terminator then insert this intrinsic // before the terminator. if (TerminatorInst *T = InsertAtEnd->getTerminator()) - return CallInst::Create(DeclareFn, Args, "", T); - else - return CallInst::Create(DeclareFn, Args, "", InsertAtEnd); + return withDebugLoc(CallInst::Create(DeclareFn, Args, "", T), DL); + return withDebugLoc(CallInst::Create(DeclareFn, Args, "", InsertAtEnd), DL); } Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, uint64_t Offset, DIVariable VarInfo, DIExpression Expr, + const MDLocation *DL, Instruction *InsertBefore) { assert(V && "no value passed to dbg.value"); assert(VarInfo && "empty or invalid DIVariable passed to dbg.value"); + assert(DL && "Expected debug loc"); + assert(DL->getScope()->getSubprogram() == + VarInfo->getScope()->getSubprogram() && + "Expected matching subprograms"); if (!ValueFn) ValueFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_value); @@ -812,15 +829,20 @@ Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, uint64_t Offset, ConstantInt::get(Type::getInt64Ty(VMContext), Offset), MetadataAsValue::get(VMContext, VarInfo), MetadataAsValue::get(VMContext, Expr)}; - return CallInst::Create(ValueFn, Args, "", InsertBefore); + return withDebugLoc(CallInst::Create(ValueFn, Args, "", InsertBefore), DL); } Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, uint64_t Offset, DIVariable VarInfo, DIExpression Expr, + const MDLocation *DL, BasicBlock *InsertAtEnd) { assert(V && "no value passed to dbg.value"); assert(VarInfo && "empty or invalid DIVariable passed to dbg.value"); + assert(DL && "Expected debug loc"); + assert(DL->getScope()->getSubprogram() == + VarInfo->getScope()->getSubprogram() && + "Expected matching subprograms"); if (!ValueFn) ValueFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_value); @@ -830,7 +852,8 @@ Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, uint64_t Offset, ConstantInt::get(Type::getInt64Ty(VMContext), Offset), MetadataAsValue::get(VMContext, VarInfo), MetadataAsValue::get(VMContext, Expr)}; - return CallInst::Create(ValueFn, Args, "", InsertAtEnd); + + return withDebugLoc(CallInst::Create(ValueFn, Args, "", InsertAtEnd), DL); } void DIBuilder::replaceVTableHolder(DICompositeType &T, DICompositeType VTableHolder) { diff --git a/lib/Transforms/Scalar/SROA.cpp b/lib/Transforms/Scalar/SROA.cpp index e8310d7f53e..59dc52811c9 100644 --- a/lib/Transforms/Scalar/SROA.cpp +++ b/lib/Transforms/Scalar/SROA.cpp @@ -1166,10 +1166,9 @@ public: } else { continue; } - Instruction *DbgVal = - DIB.insertDbgValueIntrinsic(Arg, 0, DIVariable(DVI->getVariable()), - DIExpression(DVI->getExpression()), Inst); - DbgVal->setDebugLoc(DVI->getDebugLoc()); + DIB.insertDbgValueIntrinsic(Arg, 0, DIVariable(DVI->getVariable()), + DIExpression(DVI->getExpression()), + DVI->getDebugLoc(), Inst); } } }; @@ -4211,8 +4210,8 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) { if (DbgDeclareInst *OldDDI = FindAllocaDbgDeclare(Piece.Alloca)) OldDDI->eraseFromParent(); - auto *NewDDI = DIB.insertDeclare(Piece.Alloca, Var, PieceExpr, &AI); - NewDDI->setDebugLoc(DbgDecl->getDebugLoc()); + DIB.insertDeclare(Piece.Alloca, Var, PieceExpr, DbgDecl->getDebugLoc(), + &AI); } } return Changed; diff --git a/lib/Transforms/Scalar/ScalarReplAggregates.cpp b/lib/Transforms/Scalar/ScalarReplAggregates.cpp index 72eada2d3d9..693c5ae9708 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregates.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregates.cpp @@ -1117,10 +1117,9 @@ public: } else { continue; } - Instruction *DbgVal = DIB->insertDbgValueIntrinsic( - Arg, 0, DIVariable(DVI->getVariable()), - DIExpression(DVI->getExpression()), Inst); - DbgVal->setDebugLoc(DVI->getDebugLoc()); + DIB->insertDbgValueIntrinsic(Arg, 0, DIVariable(DVI->getVariable()), + DIExpression(DVI->getExpression()), + DVI->getDebugLoc(), Inst); } } }; diff --git a/lib/Transforms/Utils/Local.cpp b/lib/Transforms/Utils/Local.cpp index a8f4c211009..51a3fc6a7bf 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp @@ -1015,11 +1015,11 @@ bool llvm::ConvertDebugDeclareToDebugValue(DbgDeclareInst *DDI, if (SExtInst *SExt = dyn_cast(SI->getOperand(0))) ExtendedArg = dyn_cast(SExt->getOperand(0)); if (ExtendedArg) - DbgVal = Builder.insertDbgValueIntrinsic(ExtendedArg, 0, DIVar, DIExpr, SI); + DbgVal = Builder.insertDbgValueIntrinsic(ExtendedArg, 0, DIVar, DIExpr, + DDI->getDebugLoc(), SI); else DbgVal = Builder.insertDbgValueIntrinsic(SI->getOperand(0), 0, DIVar, - DIExpr, SI); - DbgVal->setDebugLoc(DDI->getDebugLoc()); + DIExpr, DDI->getDebugLoc(), SI); return true; } @@ -1035,9 +1035,8 @@ bool llvm::ConvertDebugDeclareToDebugValue(DbgDeclareInst *DDI, if (LdStHasDebugValue(DIVar, LI)) return true; - Instruction *DbgVal = - Builder.insertDbgValueIntrinsic(LI->getOperand(0), 0, DIVar, DIExpr, LI); - DbgVal->setDebugLoc(DDI->getDebugLoc()); + Builder.insertDbgValueIntrinsic(LI->getOperand(0), 0, DIVar, DIExpr, + DDI->getDebugLoc(), LI); return true; } @@ -1079,10 +1078,9 @@ bool llvm::LowerDbgDeclare(Function &F) { // This is a call by-value or some other instruction that // takes a pointer to the variable. Insert a *value* // intrinsic that describes the alloca. - auto DbgVal = DIB.insertDbgValueIntrinsic( - AI, 0, DIVariable(DDI->getVariable()), - DIExpression(DDI->getExpression()), CI); - DbgVal->setDebugLoc(DDI->getDebugLoc()); + DIB.insertDbgValueIntrinsic(AI, 0, DIVariable(DDI->getVariable()), + DIExpression(DDI->getExpression()), + DDI->getDebugLoc(), CI); } DDI->eraseFromParent(); } @@ -1128,8 +1126,7 @@ bool llvm::replaceDbgDeclareForAlloca(AllocaInst *AI, Value *NewAllocaAddress, // Insert llvm.dbg.declare in the same basic block as the original alloca, // and remove old llvm.dbg.declare. BasicBlock *BB = AI->getParent(); - Builder.insertDeclare(NewAllocaAddress, DIVar, DIExpr, BB) - ->setDebugLoc(Loc); + Builder.insertDeclare(NewAllocaAddress, DIVar, DIExpr, Loc, BB); DDI->eraseFromParent(); return true; } diff --git a/unittests/Transforms/Utils/Cloning.cpp b/unittests/Transforms/Utils/Cloning.cpp index 807a2f03152..b56d453997f 100644 --- a/unittests/Transforms/Utils/Cloning.cpp +++ b/unittests/Transforms/Utils/Cloning.cpp @@ -255,8 +255,10 @@ protected: DIExpression E = DBuilder.createExpression(); DIVariable Variable = DBuilder.createLocalVariable( dwarf::DW_TAG_auto_variable, Subprogram, "x", File, 5, IntType, true); - DBuilder.insertDeclare(Alloca, Variable, E, Store); - DBuilder.insertDbgValueIntrinsic(AllocaContent, 0, Variable, E, Terminator); + auto *DL = MDLocation::get(Subprogram->getContext(), 5, 0, Subprogram); + DBuilder.insertDeclare(Alloca, Variable, E, DL, Store); + DBuilder.insertDbgValueIntrinsic(AllocaContent, 0, Variable, E, DL, + Terminator); // Finalize the debug info DBuilder.finalize(); -- 2.34.1