-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[DebugInfo][DWARF] Allow memory locations in DW_AT_call_target expressions #171183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…sions Fixes llvm#70949. Prior to PR llvm#151378 memory locations were incorrect; that patch prevented the emission of the incorrect locations. This patch fixes the underlying issue.
|
@llvm/pr-subscribers-debuginfo Author: Orlando Cazalet-Hyams (OCHyams) ChangesFixes #70949. Prior to PR #151378 memory locations were incorrect; that patch prevented the emission of the incorrect locations. This patch fixes the underlying issue. Patch to start using DW_AT_call_target_clobbered to follow once this is merged. Full diff: https://github.com/llvm/llvm-project/pull/171183.diff 5 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 751d3735d3b2b..a8d3e84cf43c0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -1319,15 +1319,22 @@ DwarfCompileUnit::getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const {
DIE &DwarfCompileUnit::constructCallSiteEntryDIE(
DIE &ScopeDIE, const DISubprogram *CalleeSP, const Function *CalleeF,
bool IsTail, const MCSymbol *PCAddr, const MCSymbol *CallAddr,
- unsigned CallReg, DIType *AllocSiteTy) {
+ MachineLocation CallTarget, int64_t Offset, DIType *AllocSiteTy) {
// Insert a call site entry DIE within ScopeDIE.
DIE &CallSiteDIE = createAndAddDIE(getDwarf5OrGNUTag(dwarf::DW_TAG_call_site),
ScopeDIE, nullptr);
- if (CallReg) {
- // Indirect call.
- addAddress(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_target),
- MachineLocation(CallReg));
+ // A valid register in CallTarget indicates an indirect call.
+ if (CallTarget.getReg()) {
+ // CallTarget is the location of the address of an indirect call. The
+ // location may be indirect, modified by Offset.
+ if (CallTarget.isIndirect())
+ addMemoryLocation(CallSiteDIE,
+ getDwarf5OrGNUAttr(dwarf::DW_AT_call_target),
+ CallTarget, Offset);
+ else
+ addAddress(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_target),
+ CallTarget);
} else if (CalleeSP) {
DIE *CalleeDIE = getOrCreateSubprogramDIE(CalleeSP, CalleeF);
assert(CalleeDIE && "Could not create DIE for call site entry origin");
@@ -1640,15 +1647,15 @@ void DwarfCompileUnit::addVariableAddress(const DbgVariable &DV, DIE &Die,
addAddress(Die, dwarf::DW_AT_location, Location);
}
-/// Add an address attribute to a die based on the location provided.
-void DwarfCompileUnit::addAddress(DIE &Die, dwarf::Attribute Attribute,
- const MachineLocation &Location) {
+void DwarfCompileUnit::addLocationWithExpr(DIE &Die, dwarf::Attribute Attribute,
+ const MachineLocation &Location,
+ ArrayRef<uint64_t> Expr) {
DIELoc *Loc = new (DIEValueAllocator) DIELoc;
DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
if (Location.isIndirect())
DwarfExpr.setMemoryLocationKind();
- DIExpressionCursor Cursor({});
+ DIExpressionCursor Cursor(Expr);
const TargetRegisterInfo &TRI = *Asm->MF->getSubtarget().getRegisterInfo();
if (!DwarfExpr.addMachineRegExpression(TRI, Cursor, Location.getReg()))
return;
@@ -1662,6 +1669,21 @@ void DwarfCompileUnit::addAddress(DIE &Die, dwarf::Attribute Attribute,
*DwarfExpr.TagOffset);
}
+/// Add an address attribute to a die based on the location provided.
+void DwarfCompileUnit::addAddress(DIE &Die, dwarf::Attribute Attribute,
+ const MachineLocation &Location) {
+ addLocationWithExpr(Die, Attribute, Location, {});
+}
+
+void DwarfCompileUnit::addMemoryLocation(DIE &Die, dwarf::Attribute Attribute,
+ const MachineLocation &Location,
+ int64_t Offset) {
+ assert(Location.isIndirect() && "Memory loc should be indirect");
+ SmallVector<uint64_t, 3> Ops;
+ DIExpression::appendOffset(Ops, Offset);
+ addLocationWithExpr(Die, Attribute, Location, Ops);
+}
+
/// Start with the address based on the location provided, and generate the
/// DWARF information necessary to find the actual variable given the extra
/// address information encoded in the DbgVariable, starting from the starting
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index a3bbc8364599d..a90fec0da0837 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -160,6 +160,12 @@ class DwarfCompileUnit final : public DwarfUnit {
DIE &createAbstractSubprogramDIE(const DISubprogram *SP, DIE *ContextDIE,
DwarfCompileUnit *ContextCU);
+ /// Add a location exprloc to \p DIE with attribute \p Attribute at
+ /// for \p Location modified by raw DIExpression \p Expr.
+ void addLocationWithExpr(DIE &Die, dwarf::Attribute Attribute,
+ const MachineLocation &Location,
+ ArrayRef<uint64_t> Expr);
+
public:
DwarfCompileUnit(unsigned UID, const DICompileUnit *Node, AsmPrinter *A,
DwarfDebug *DW, DwarfFile *DWU,
@@ -309,12 +315,14 @@ class DwarfCompileUnit final : public DwarfUnit {
/// \p IsTail specifies whether the call is a tail call.
/// \p PCAddr points to the PC value after the call instruction.
/// \p CallAddr points to the PC value at the call instruction (or is null).
- /// \p CallReg is a register location for an indirect call. For direct calls
- /// the \p CallReg is set to 0.
+ /// \p CallTarget a location holding the target address for an indirect call.
+ /// For direct calls \p CallTarget register is set to 0.
+ /// \p Offset from \p CallTarget register value if the location is indirect.
DIE &constructCallSiteEntryDIE(DIE &ScopeDIE, const DISubprogram *CalleeSP,
const Function *CalleeF, bool IsTail,
const MCSymbol *PCAddr,
- const MCSymbol *CallAddr, unsigned CallReg,
+ const MCSymbol *CallAddr,
+ MachineLocation CallTarget, int64_t Offset,
DIType *AllocSiteTy);
/// Construct call site parameter DIEs for the \p CallSiteDIE. The \p Params
/// were collected by the \ref collectCallSiteParameters.
@@ -385,6 +393,10 @@ class DwarfCompileUnit final : public DwarfUnit {
void addAddress(DIE &Die, dwarf::Attribute Attribute,
const MachineLocation &Location);
+ /// Add a memory location exprloc to \p DIE with attribute \p Attribute
+ /// at \p Location + \p Offset.
+ void addMemoryLocation(DIE &Die, dwarf::Attribute Attribute,
+ const MachineLocation &Location, int64_t Offset);
/// Start with the address based on the location provided, and generate the
/// DWARF information necessary to find the actual variable (navigating the
/// extra location information encoded in the type) based on the starting
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 567acf75d1b8d..a86fe3009e98d 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -942,25 +942,28 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
DIType *AllocSiteTy = dyn_cast_or_null<DIType>(MI.getHeapAllocMarker());
// If this is a direct call, find the callee's subprogram.
- // In the case of an indirect call find the register that holds
- // the callee.
+ // In the case of an indirect call find the register or memory location
+ // that holds the callee address.
const MachineOperand &CalleeOp = TII->getCalleeOperand(MI);
bool PhysRegCalleeOperand =
CalleeOp.isReg() && CalleeOp.getReg().isPhysical();
- // Hack: WebAssembly CALL instructions have MCInstrDesc that does not
- // describe the call target operand.
- if (CalleeOp.getOperandNo() < MI.getDesc().operands().size()) {
- const MCOperandInfo &MCOI =
- MI.getDesc().operands()[CalleeOp.getOperandNo()];
- PhysRegCalleeOperand =
- PhysRegCalleeOperand && MCOI.OperandType == MCOI::OPERAND_REGISTER;
- }
-
- unsigned CallReg = 0;
+ MachineLocation CallTarget{0};
+ int64_t Offset = 0;
const DISubprogram *CalleeSP = nullptr;
const Function *CalleeDecl = nullptr;
if (PhysRegCalleeOperand) {
- CallReg = CalleeOp.getReg(); // might be zero
+ bool Scalable = false;
+ const MachineOperand *BaseOp = nullptr;
+ const TargetRegisterInfo &TRI =
+ *Asm->MF->getSubtarget().getRegisterInfo();
+ if (TII->getMemOperandWithOffset(MI, BaseOp, Offset, Scalable, &TRI)) {
+ if (BaseOp && BaseOp->isReg() && !Scalable)
+ CallTarget = MachineLocation(BaseOp->getReg(), /*Indirect*/ true);
+ }
+
+ if (!CallTarget.isIndirect())
+ CallTarget = MachineLocation(CalleeOp.getReg()); // Might be zero.
+
} else if (CalleeOp.isGlobal()) {
CalleeDecl = dyn_cast<Function>(CalleeOp.getGlobal());
if (CalleeDecl)
@@ -969,7 +972,8 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
// Omit DIE if we can't tell where the call goes *and* we don't want to
// add metadata to it.
- if (CalleeSP == nullptr && CallReg == 0 && AllocSiteTy == nullptr)
+ if (CalleeSP == nullptr && CallTarget.getReg() == 0 &&
+ AllocSiteTy == nullptr)
continue;
// TODO: Omit call site entries for runtime calls (objc_msgSend, etc).
@@ -997,16 +1001,18 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
assert((IsTail || PCAddr) && "Non-tail call without return PC");
- LLVM_DEBUG(dbgs() << "CallSiteEntry: " << MF.getName() << " -> "
- << (CalleeDecl ? CalleeDecl->getName()
- : StringRef(MF.getSubtarget()
- .getRegisterInfo()
- ->getName(CallReg)))
- << (IsTail ? " [IsTail]" : "") << "\n");
-
- DIE &CallSiteDIE =
- CU.constructCallSiteEntryDIE(ScopeDIE, CalleeSP, CalleeDecl, IsTail,
- PCAddr, CallAddr, CallReg, AllocSiteTy);
+ LLVM_DEBUG(
+ dbgs() << "CallSiteEntry: " << MF.getName() << " -> "
+ << (CalleeDecl
+ ? CalleeDecl->getName()
+ : StringRef(
+ MF.getSubtarget().getRegisterInfo()->getName(
+ CallTarget.getReg())))
+ << (IsTail ? " [IsTail]" : "") << "\n");
+
+ DIE &CallSiteDIE = CU.constructCallSiteEntryDIE(
+ ScopeDIE, CalleeSP, CalleeDecl, IsTail, PCAddr, CallAddr, CallTarget,
+ Offset, AllocSiteTy);
// Optionally emit call-site-param debug info.
if (emitDebugEntryValues()) {
diff --git a/llvm/test/DebugInfo/X86/dwarf-call-target-mem-loc.mir b/llvm/test/DebugInfo/X86/dwarf-call-target-mem-loc.mir
new file mode 100644
index 0000000000000..ef8a080cebaae
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/dwarf-call-target-mem-loc.mir
@@ -0,0 +1,91 @@
+# RUN: llc %s --start-after=livedebugvalues -o - --filetype=obj | llvm-dwarfdump - | FileCheck %s
+
+## Check the memory location of the target address for the indirect call
+## (virtual in this case) is described by a DW_AT_call_target expression.
+
+# CHECK: DW_TAG_call_site
+# CHECK-NEXT: DW_AT_call_target (DW_OP_breg0 RAX+8)
+
+## Generated from this C++ with llc -stop-after=livedebugvalues -simplify-mir:
+## struct Base {
+## virtual int zz() { return x; }
+## [[clang::noinline]]
+## virtual int v() { return zz(); }
+## int x;
+## };
+## struct Child: public Base {
+## [[clang::noinline]]
+## virtual int v() { return x * 2; }
+## int x;
+## };
+##
+## [[clang::noinline]]
+## [[clang::disable_tail_calls]]
+## int foo(Base* b) {
+## return b->v();
+## }
+
+--- |
+ target triple = "x86_64-unknown-linux-gnu"
+
+ define dso_local noundef i32 @_Z3fooP4Base(ptr noundef %b) local_unnamed_addr !dbg !5 {
+ entry:
+ %vtable = load ptr, ptr %b, align 8, !dbg !12
+ %vfn = getelementptr inbounds nuw i8, ptr %vtable, i64 8, !dbg !12
+ %0 = load ptr, ptr %vfn, align 8, !dbg !12
+ %call = call noundef i32 %0(ptr noundef nonnull align 8 dereferenceable(12) %b), !dbg !12
+ ret i32 %call, !dbg !12
+ }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!2, !3}
+ !llvm.ident = !{!4}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 22.0.0git", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+ !1 = !DIFile(filename: "test.cpp", directory: "/")
+ !2 = !{i32 7, !"Dwarf Version", i32 5}
+ !3 = !{i32 2, !"Debug Info Version", i32 3}
+ !4 = !{!"clang version 22.0.0git"}
+ !5 = distinct !DISubprogram(name: "foo", linkageName: "_Z3fooP4Base", scope: !1, file: !1, line: 15, type: !6, scopeLine: 15, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11, keyInstructions: true)
+ !6 = !DISubroutineType(types: !7)
+ !7 = !{!8, !9}
+ !8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+ !9 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !10, size: 64)
+ !10 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Base", file: !1, line: 1, size: 128, flags: DIFlagFwdDecl | DIFlagNonTrivial, identifier: "_ZTS4Base")
+ !11 = !{}
+ !12 = !DILocation(line: 16, scope: !5)
+...
+---
+name: _Z3fooP4Base
+alignment: 16
+tracksRegLiveness: true
+noPhis: true
+isSSA: false
+noVRegs: true
+hasFakeUses: false
+debugInstrRef: true
+tracksDebugUserValues: true
+liveins:
+ - { reg: '$rdi' }
+frameInfo:
+ stackSize: 8
+ offsetAdjustment: -8
+ maxAlignment: 1
+ adjustsStack: true
+ hasCalls: true
+ maxCallFrameSize: 0
+ isCalleeSavedInfoValid: true
+machineFunctionInfo:
+ amxProgModel: None
+body: |
+ bb.0.entry:
+ liveins: $rdi
+
+ frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp
+ frame-setup CFI_INSTRUCTION def_cfa_offset 16
+ renamable $rax = MOV64rm renamable $rdi, 1, $noreg, 0, $noreg, debug-location !12 :: (load (s64) from %ir.b)
+ CALL64m killed renamable $rax, 1, $noreg, 8, $noreg, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp, implicit-def $eax, debug-location !12 :: (load (s64) from %ir.vfn)
+ $rcx = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !12
+ frame-destroy CFI_INSTRUCTION def_cfa_offset 8, debug-location !12
+ RET64 $eax, debug-location !12
+...
diff --git a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs-indirect.ll b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs-indirect.ll
index 6c81e2e72d93c..f64b78f5820e4 100644
--- a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs-indirect.ll
+++ b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs-indirect.ll
@@ -8,7 +8,7 @@
; RUN: llvm-dwarfdump -statistics %t.o | FileCheck %s -check-prefix=STATS
; VERIFY: No errors.
-; STATS: "#call site DIEs": 1,
+; STATS: "#call site DIEs": 2,
; OBJ: DW_TAG_subprogram
; OBJ: DW_AT_name ("call_reg")
@@ -18,7 +18,7 @@ entry:
#dbg_value(ptr %f, !17, !DIExpression(), !18)
; OBJ: DW_TAG_call_site
-; OBJ: DW_AT_call_target
+; OBJ: DW_AT_call_target (DW_OP_reg[[#]] {{.*}})
; OBJ: DW_AT_call_return_pc
call void (...) %f() #1, !dbg !19
ret void, !dbg !20
@@ -31,6 +31,10 @@ define dso_local void @call_mem(ptr noundef readonly captures(none) %f) local_un
entry:
#dbg_value(ptr %f, !26, !DIExpression(), !27)
%0 = load ptr, ptr %f, align 8, !dbg !28, !tbaa !29
+
+; OBJ: DW_TAG_call_site
+; OBJ: DW_AT_call_target (DW_OP_breg[[#]] {{.*}})
+; OBJ: DW_AT_call_return_pc
call void (...) %0() #1, !dbg !28
ret void, !dbg !33
}
|
dzhidzhoev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good from my perspective. Might be good to have someone more experienced give a look as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this logic covered somewhere else? I’m not quite familiar with WASM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good question! I did some digging -
I believe the comment refers to the need for the if statement to avoid performing the MI.getDesc().operands()[CalleeOp.getOperandNo()] lookup. Without my patch and without that if statement we get a crash in /home/och/dev/llvm-project/llvm/test/DebugInfo/WebAssembly/call-site.ll and /home/och/dev/llvm-project/llvm/test/DebugInfo/WebAssembly/stackified-debug.ll.
call-site.ll checks direct calls work as expected and that indirect calls simple don't cause a crash/assertion and says in a comment that DW_AT_call_target isn't supported for indirect calls for WebAssembly.
Those tests passes with my patch, and it looks like PhysRegCalleeOperand is false even for indirect calls for WebAssembly, meaning we continue to not-support WebAssembly indirect calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for explanation!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add description for this function also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 copied the comment from the header to here too
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/39/builds/9098 Here is the relevant piece of the build log for the reference |
Looks like a flaky bot or test (other commits have caused the same issue on and off) |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/135/builds/2828 Here is the relevant piece of the build log for the reference |
Fixes #70949. Prior to PR #151378 memory locations were incorrect; that patch prevented the emission of the incorrect locations. This patch fixes the underlying issue.
Patch to start using DW_AT_call_target_clobbered to follow once this is merged.