[DebugInfo] LowerDbgDeclare: Add derefs when handling CallInst users
LowerDbgDeclare inserts a dbg.value before each use of an address
described by a dbg.declare. When inserting a dbg.value before a CallInst
use, however, it fails to append DW_OP_deref to the DIExpression.
The DW_OP_deref is needed to reflect the fact that a dbg.value describes
a source variable directly (as opposed to a dbg.declare, which relies on
pointer indirection).
This patch adds in the DW_OP_deref where needed. This results in the
correct values being shown during a debug session for a program compiled
with ASan and optimizations (see https://reviews.llvm.org/D49520). Note
that ConvertDebugDeclareToDebugValue is already correct -- no changes
there were needed.
One complication is that SelectionDAG is unable to distinguish between
direct and indirect frame-index (FRAMEIX) SDDbgValues. This patch also
fixes this long-standing issue in order to not regress integration tests
relying on the incorrect assumption that all frame-index SDDbgValues are
indirect. This is a necessary fix: the newly-added DW_OP_derefs cannot
be lowered properly otherwise. Basically the fix prevents a direct
SDDbgValue with DIExpression(DW_OP_deref) from being dereferenced twice
by a debugger. There were a handful of tests relying on this incorrect
"FRAMEIX => indirect" assumption which actually had incorrect
DW_AT_locations: these are all fixed up in this patch.
Testing:
- check-llvm, and an end-to-end test using lldb to debug an optimized
program.
- Existing unit tests for DIExpression::appendToStack fully cover the
new DIExpression::append utility.
- check-debuginfo (the debug info integration tests)
Differential Revision: https://reviews.llvm.org/D49454
llvm-svn: 338069
diff --git a/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp b/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
index 65ee381..d6171f3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
@@ -697,11 +697,15 @@
if (SD->getKind() == SDDbgValue::FRAMEIX) {
// Stack address; this needs to be lowered in target-dependent fashion.
// EmitTargetCodeForFrameDebugValue is responsible for allocation.
- return BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE))
- .addFrameIndex(SD->getFrameIx())
- .addImm(0)
- .addMetadata(Var)
- .addMetadata(Expr);
+ auto FrameMI = BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE))
+ .addFrameIndex(SD->getFrameIx());
+ if (SD->isIndirect())
+ // Push [fi + 0] onto the DIExpression stack.
+ FrameMI.addImm(0);
+ else
+ // Push fi onto the DIExpression stack.
+ FrameMI.addReg(0);
+ return FrameMI.addMetadata(Var).addMetadata(Expr);
}
// Otherwise, we're going to create an instruction here.
const MCInstrDesc &II = TII->get(TargetOpcode::DBG_VALUE);
diff --git a/llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h b/llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h
index 703eaa4..7e6b574 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h
+++ b/llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h
@@ -71,20 +71,18 @@
u.Const = C;
}
- /// Constructor for frame indices.
- SDDbgValue(DIVariable *Var, DIExpression *Expr, unsigned FI, DebugLoc dl,
- unsigned O)
- : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(false) {
- kind = FRAMEIX;
- u.FrameIx = FI;
- }
-
- /// Constructor for virtual registers.
- SDDbgValue(DIVariable *Var, DIExpression *Expr, unsigned VReg, bool indir,
- DebugLoc dl, unsigned O)
- : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(indir) {
- kind = VREG;
- u.VReg = VReg;
+ /// Constructor for virtual registers and frame indices.
+ SDDbgValue(DIVariable *Var, DIExpression *Expr, unsigned VRegOrFrameIdx,
+ bool IsIndirect, DebugLoc DL, unsigned Order,
+ enum DbgValueKind Kind)
+ : Var(Var), Expr(Expr), DL(DL), Order(Order), IsIndirect(IsIndirect) {
+ assert((Kind == VREG || Kind == FRAMEIX) &&
+ "Invalid SDDbgValue constructor");
+ kind = Kind;
+ if (kind == VREG)
+ u.VReg = VRegOrFrameIdx;
+ else
+ u.FrameIx = VRegOrFrameIdx;
}
/// Returns the kind.
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 9870f21..1f2c50a 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -7391,11 +7391,13 @@
/// FrameIndex
SDDbgValue *SelectionDAG::getFrameIndexDbgValue(DIVariable *Var,
DIExpression *Expr, unsigned FI,
+ bool IsIndirect,
const DebugLoc &DL,
unsigned O) {
assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
"Expected inlined-at fields to agree");
- return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, FI, DL, O);
+ return new (DbgInfo->getAlloc())
+ SDDbgValue(Var, Expr, FI, IsIndirect, DL, O, SDDbgValue::FRAMEIX);
}
/// VReg
@@ -7405,8 +7407,8 @@
const DebugLoc &DL, unsigned O) {
assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
"Expected inlined-at fields to agree");
- return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, VReg, IsIndirect, DL,
- O);
+ return new (DbgInfo->getAlloc())
+ SDDbgValue(Var, Expr, VReg, IsIndirect, DL, O, SDDbgValue::VREG);
}
void SelectionDAG::transferDbgValues(SDValue From, SDValue To,
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 12ce222..737bb1f 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -4973,13 +4973,20 @@
unsigned DbgSDNodeOrder) {
if (auto *FISDN = dyn_cast<FrameIndexSDNode>(N.getNode())) {
// Construct a FrameIndexDbgValue for FrameIndexSDNodes so we can describe
- // stack slot locations as such instead of as indirectly addressed
- // locations.
- return DAG.getFrameIndexDbgValue(Variable, Expr, FISDN->getIndex(), dl,
- DbgSDNodeOrder);
+ // stack slot locations.
+ //
+ // Consider "int x = 0; int *px = &x;". There are two kinds of interesting
+ // debug values here after optimization:
+ //
+ // dbg.value(i32* %px, !"int *px", !DIExpression()), and
+ // dbg.value(i32* %px, !"int x", !DIExpression(DW_OP_deref))
+ //
+ // Both describe the direct values of their associated variables.
+ return DAG.getFrameIndexDbgValue(Variable, Expr, FISDN->getIndex(),
+ /*IsIndirect*/ false, dl, DbgSDNodeOrder);
}
- return DAG.getDbgValue(Variable, Expr, N.getNode(), N.getResNo(), false, dl,
- DbgSDNodeOrder);
+ return DAG.getDbgValue(Variable, Expr, N.getNode(), N.getResNo(),
+ /*IsIndirect*/ false, dl, DbgSDNodeOrder);
}
// VisualStudio defines setjmp as _setjmp
@@ -5191,9 +5198,9 @@
// the MachineFunction variable table.
if (FI != std::numeric_limits<int>::max()) {
if (Intrinsic == Intrinsic::dbg_addr) {
- SDDbgValue *SDV = DAG.getFrameIndexDbgValue(Variable, Expression,
- FI, dl, SDNodeOrder);
- DAG.AddDbgValue(SDV, getRoot().getNode(), isParameter);
+ SDDbgValue *SDV = DAG.getFrameIndexDbgValue(
+ Variable, Expression, FI, /*IsIndirect*/ true, dl, SDNodeOrder);
+ DAG.AddDbgValue(SDV, getRoot().getNode(), isParameter);
}
return nullptr;
}
@@ -5210,8 +5217,9 @@
auto FINode = dyn_cast<FrameIndexSDNode>(N.getNode());
if (isParameter && FINode) {
// Byval parameter. We have a frame index at this point.
- SDV = DAG.getFrameIndexDbgValue(Variable, Expression,
- FINode->getIndex(), dl, SDNodeOrder);
+ SDV =
+ DAG.getFrameIndexDbgValue(Variable, Expression, FINode->getIndex(),
+ /*IsIndirect*/ true, dl, SDNodeOrder);
} else if (isa<Argument>(Address)) {
// Address is an argument, so try to emit its dbg value using
// virtual register info from the FuncInfo.ValueMap.
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index db28d9a..910e8c2 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -841,9 +841,37 @@
return DIExpression::get(Expr->getContext(), Ops);
}
+DIExpression *DIExpression::append(const DIExpression *Expr,
+ ArrayRef<uint64_t> Ops) {
+ assert(Expr && !Ops.empty() && "Can't append ops to this expression");
+
+ // Copy Expr's current op list.
+ SmallVector<uint64_t, 16> NewOps;
+ for (auto Op : Expr->expr_ops()) {
+ // Append new opcodes before DW_OP_{stack_value, LLVM_fragment}.
+ if (Op.getOp() == dwarf::DW_OP_stack_value ||
+ Op.getOp() == dwarf::DW_OP_LLVM_fragment) {
+ NewOps.append(Ops.begin(), Ops.end());
+
+ // Ensure that the new opcodes are only appended once.
+ Ops = None;
+ }
+ Op.appendToVector(NewOps);
+ }
+
+ NewOps.append(Ops.begin(), Ops.end());
+ return DIExpression::get(Expr->getContext(), NewOps);
+}
+
DIExpression *DIExpression::appendToStack(const DIExpression *Expr,
ArrayRef<uint64_t> Ops) {
assert(Expr && !Ops.empty() && "Can't append ops to this expression");
+ assert(none_of(Ops,
+ [](uint64_t Op) {
+ return Op == dwarf::DW_OP_stack_value ||
+ Op == dwarf::DW_OP_LLVM_fragment;
+ }) &&
+ "Can't append this op");
// Append a DW_OP_deref after Expr's current op list if it's non-empty and
// has no DW_OP_stack_value.
@@ -851,30 +879,21 @@
// Match .* DW_OP_stack_value (DW_OP_LLVM_fragment A B)?.
Optional<FragmentInfo> FI = Expr->getFragmentInfo();
unsigned DropUntilStackValue = FI.hasValue() ? 3 : 0;
- bool NeedsDeref =
- (Expr->getNumElements() > DropUntilStackValue) &&
- (Expr->getElements().drop_back(DropUntilStackValue).back() !=
- dwarf::DW_OP_stack_value);
+ ArrayRef<uint64_t> ExprOpsBeforeFragment =
+ Expr->getElements().drop_back(DropUntilStackValue);
+ bool NeedsDeref = (Expr->getNumElements() > DropUntilStackValue) &&
+ (ExprOpsBeforeFragment.back() != dwarf::DW_OP_stack_value);
+ bool NeedsStackValue = NeedsDeref || ExprOpsBeforeFragment.empty();
- // Copy Expr's current op list, add a DW_OP_deref if needed, and ensure that
- // a DW_OP_stack_value is present.
+ // Append a DW_OP_deref after Expr's current op list if needed, then append
+ // the new ops, and finally ensure that a single DW_OP_stack_value is present.
SmallVector<uint64_t, 16> NewOps;
- for (auto Op : Expr->expr_ops()) {
- if (Op.getOp() == dwarf::DW_OP_stack_value ||
- Op.getOp() == dwarf::DW_OP_LLVM_fragment)
- break;
- Op.appendToVector(NewOps);
- }
if (NeedsDeref)
NewOps.push_back(dwarf::DW_OP_deref);
NewOps.append(Ops.begin(), Ops.end());
- NewOps.push_back(dwarf::DW_OP_stack_value);
-
- // If Expr is a fragment, make the new expression a fragment as well.
- if (FI)
- NewOps.append(
- {dwarf::DW_OP_LLVM_fragment, FI->OffsetInBits, FI->SizeInBits});
- return DIExpression::get(Expr->getContext(), NewOps);
+ if (NeedsStackValue)
+ NewOps.push_back(dwarf::DW_OP_stack_value);
+ return DIExpression::append(Expr, NewOps);
}
Optional<DIExpression *> DIExpression::createFragmentExpression(
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index a0e79b6..ae3cb07 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1419,12 +1419,13 @@
} else if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
ConvertDebugDeclareToDebugValue(DDI, LI, DIB);
} else if (CallInst *CI = dyn_cast<CallInst>(U)) {
- // 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.
- DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(),
- DDI->getExpression(), DDI->getDebugLoc(),
- CI);
+ // This is a call by-value or some other instruction that takes a
+ // pointer to the variable. Insert a *value* intrinsic that describes
+ // the variable by dereferencing the alloca.
+ auto *DerefExpr =
+ DIExpression::append(DDI->getExpression(), dwarf::DW_OP_deref);
+ DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr,
+ DDI->getDebugLoc(), CI);
}
}
DDI->eraseFromParent();