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
diff --git a/bindings/go/llvm/DIBuilderBindings.cpp b/bindings/go/llvm/DIBuilderBindings.cpp index ee2e70a..e5734a7 100644 --- a/bindings/go/llvm/DIBuilderBindings.cpp +++ b/bindings/go/llvm/DIBuilderBindings.cpp
@@ -234,10 +234,14 @@ 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<MDLocalVariable>(VarInfo), - unwrap<MDExpression>(Expr), unwrap(Block)); + Instruction *Instr = D->insertDeclare( + unwrap(Storage), unwrap<MDLocalVariable>(VarInfo), + unwrap<MDExpression>(Expr), /* DebugLoc */ nullptr, unwrap(Block)); return wrap(Instr); } @@ -246,9 +250,13 @@ 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<MDLocalVariable>(VarInfo), - unwrap<MDExpression>(Expr), unwrap(Block)); + unwrap<MDExpression>(Expr), /* DebugLoc */ nullptr, unwrap(Block)); return wrap(Instr); }
diff --git a/include/llvm/IR/DIBuilder.h b/include/llvm/IR/DIBuilder.h index ac82947..cadfa31 100644 --- a/include/llvm/IR/DIBuilder.h +++ b/include/llvm/IR/DIBuilder.h
@@ -656,26 +656,32 @@ /// @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 @@ /// @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 1f087888..654dafc 100644 --- a/lib/IR/DIBuilder.cpp +++ b/lib/IR/DIBuilder.cpp
@@ -761,10 +761,19 @@ return MetadataAsValue::get(VMContext, ValueAsMetadata::get(V)); } +static Instruction *withDebugLoc(Instruction *I, const MDLocation *DL) { + I->setDebugLoc(const_cast<MDLocation *>(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 @@ 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 @@ // 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 @@ 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 @@ 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 e8310d7..59dc528 100644 --- a/lib/Transforms/Scalar/SROA.cpp +++ b/lib/Transforms/Scalar/SROA.cpp
@@ -1166,10 +1166,9 @@ } 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 @@ 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 72eada2..693c5ae 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregates.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregates.cpp
@@ -1117,10 +1117,9 @@ } 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 a8f4c21..51a3fc6 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp
@@ -1015,11 +1015,11 @@ if (SExtInst *SExt = dyn_cast<SExtInst>(SI->getOperand(0))) ExtendedArg = dyn_cast<Argument>(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 @@ 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 @@ // 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 @@ // 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 807a2f0..b56d453 100644 --- a/unittests/Transforms/Utils/Cloning.cpp +++ b/unittests/Transforms/Utils/Cloning.cpp
@@ -255,8 +255,10 @@ 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();