Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2218,6 +2218,7 @@ findPrologueEndLoc(const MachineFunction *MF) {
const auto &TII = *MF->getSubtarget().getInstrInfo();
const MachineInstr *NonTrivialInst = nullptr;
const Function &F = MF->getFunction();
DISubprogram *SP = const_cast<DISubprogram*>(F.getSubprogram());

// Some instructions may be inserted into prologue after this function. Must
// keep prologue for these cases.
Expand Down Expand Up @@ -2305,6 +2306,36 @@ findPrologueEndLoc(const MachineFunction *MF) {
return *FoundInst;
}

// In very rare scenarios function calls can have line zero, and we
// shouldn't step over such a call while trying to reach prologue_end. In
// these extraordinary conditions, force an earlier setup instruction to
// have the scope line and put prologue_end there. This will be suboptimal,
// and might still be in setup code, but is less catastrophic than missing
// a call.
if (CurInst->isCall()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this covers tail calls too (which is good, just noting my thoughts for the review -- no action here)

if (const DILocation *Loc = CurInst->getDebugLoc().get();
Loc && Loc->getLine() == 0) {
// Go back one instruction.
auto RIt = std::next(CurInst->getIterator().getReverse());
// In the radically unlikely event that there's no prior instruction,
// meaning the first instruction in the function is a call, don't set a
// prologue_end at all.
if (RIt == CurInst->getParent()->rend())
return std::make_pair(nullptr, true);

// The prior instruction was either line-zero or unset, or a setup
// instruction, or otherwise uninteresting. Force it to have the
// scope line.
unsigned ScopeLine = SP->getScopeLine();
DILocation *ScopeLineDILoc =
DILocation::get(SP->getContext(), ScopeLine, 0, SP);
const_cast<MachineInstr*>(&*RIt)->setDebugLoc(ScopeLineDILoc);

// Consider this position to be where prologue_end is placed.
return std::make_pair(&*RIt, false);
}
}

// Try to continue searching, but use a backup-location if substantive
// computation is happening.
auto NextInst = std::next(CurInst);
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/DebugInfo/MIR/X86/debug-loc-0.mir
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# CHECK: Ltmp0:
# CHECK: .loc 1 0 0
# CHECK-NOT: .loc 1 0 0
# CHECK: .loc 1 37 1 prologue_end
# CHECK: .loc 1 37 1

--- |
; ModuleID = '<stdin>'
Expand Down
127 changes: 127 additions & 0 deletions llvm/test/DebugInfo/X86/no-prologue-end-after-line0-calls.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
# RUN: llc %s -start-after=livedebugvalues -o - -filetype=obj | llvm-dwarfdump - --debug-line | FileCheck %s
#
## Original code, compiled clang -O2 -g -c
##
## void ext();
## int main(int argc, char **argv) {
## if (argc == 1)
## ext();
## else
## ext();
## return 0;
## }
##
## In the code sequence above, the call to ext is given line zero during
## optimisation, because the code is duplicated down all function paths thus
## gets merged. We get something like this as the output:
##
## 0: 50 push %rax
## 1: 31 c0 xor %eax,%eax
## 3: e8 00 00 00 00 call 8 <main+0x8>
## 4: R_X86_64_PLT32 ext-0x4
## 8: 31 c0 xor %eax,%eax
## a: 59 pop %rcx
## b: c3 ret
##
## And we could choose to set prologue_end on address 8, the clearing of the
## return register, because it's the first "real" instruction that isn't line
## zero. But this then causes debuggers to skip over the call instruction when
## entering the function, which is catastrophic.
##
## Instead: we force the xor at address 1 to have a source location (the
## function scope line number), and put a prologue_end there. While it's a
## setup instruction, it's better to have a prologue_end that's still slightly
## in the prologue than to step over the call. This gives consumers the
## opportunity to recognise "this is a crazy function" and act accordingly.
##
## Check lines: the first entry is the start-of-function scope line, the second
## entry is the prologue_end on the xor, while the third is the zero-line-number
## call instruction.
#
# CHECK: 2 0 0 0 0 0 is_stmt
# CHECK-NEXT: 2 0 0 0 0 0 is_stmt prologue_end
# CHECK-NEXT: 0 0 0 0 0 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it could possibly be worth objdumping and cross referencing the addresses? I have done that in other similar tests. I worry that this could become rotten-green. If that fear seems unfounded to you, feel free to ignore this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've switched to examining the textual output, which has the upside of not printing hex, but still having the meaning of the .loc lines and their flags in the context of the instructions.


--- |
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

; Function Attrs: nounwind uwtable
define dso_local noundef i32 @main(i32 noundef %argc, ptr noundef readnone captures(none) %argv) local_unnamed_addr !dbg !10 {
entry:
#dbg_value(i32 %argc, !19, !DIExpression(), !21)
#dbg_value(ptr %argv, !20, !DIExpression(), !21)
tail call void (...) @ext(), !dbg !22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the variable location metadata (in the IR, metadata blob, or MIR)?

ret i32 0, !dbg !24
}

declare !dbg !25 void @ext(...) local_unnamed_addr

!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
!llvm.ident = !{!9}

!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
!1 = !DIFile(filename: "/tmp/test.c", directory: "")
!2 = !{i32 7, !"Dwarf Version", i32 5}
!3 = !{i32 2, !"Debug Info Version", i32 3}
!4 = !{i32 1, !"wchar_size", i32 4}
!5 = !{i32 8, !"PIC Level", i32 2}
!6 = !{i32 7, !"PIE Level", i32 2}
!7 = !{i32 7, !"uwtable", i32 2}
!8 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
!9 = !{!"clang"}
!10 = distinct !DISubprogram(name: "main", scope: !11, file: !11, line: 2, type: !12, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !18, keyInstructions: true)
!11 = !DIFile(filename: "/tmp/test.c", directory: "")
!12 = !DISubroutineType(types: !13)
!13 = !{!14, !14, !15}
!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !16, size: 64)
!16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !17, size: 64)
!17 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
!18 = !{!19, !20}
!19 = !DILocalVariable(name: "argc", arg: 1, scope: !10, file: !11, line: 2, type: !14)
!20 = !DILocalVariable(name: "argv", arg: 2, scope: !10, file: !11, line: 2, type: !15)
!21 = !DILocation(line: 0, scope: !10)
!22 = !DILocation(line: 0, scope: !23)
!23 = distinct !DILexicalBlock(scope: !10, file: !11, line: 3, column: 7)
!24 = !DILocation(line: 7, column: 4, scope: !10, atomGroup: 2, atomRank: 1)
!25 = !DISubprogram(name: "ext", scope: !11, file: !11, line: 1, type: !26, spFlags: DISPFlagOptimized)
!26 = !DISubroutineType(types: !27)
!27 = !{null}
...
---
name: main
alignment: 16
tracksRegLiveness: true
noPhis: true
isSSA: false
noVRegs: true
hasFakeUses: false
debugInstrRef: true
tracksDebugUserValues: true
frameInfo:
stackSize: 8
offsetAdjustment: -8
maxAlignment: 1
adjustsStack: true
hasCalls: true
maxCallFrameSize: 0
isCalleeSavedInfoValid: true
machineFunctionInfo:
amxProgModel: None
body: |
bb.0.entry:
DBG_VALUE $edi, $noreg, !19, !DIExpression(), debug-location !21
DBG_VALUE $rsi, $noreg, !20, !DIExpression(), debug-location !21
frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp
frame-setup CFI_INSTRUCTION def_cfa_offset 16
dead $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $al, debug-location !22
CALL64pcrel32 target-flags(x86-plt) @ext, csr_64, implicit $rsp, implicit $ssp, implicit killed $al, implicit-def $rsp, implicit-def $ssp, debug-location !22
DBG_VALUE $rsi, $noreg, !20, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location !21
DBG_VALUE $edi, $noreg, !19, !DIExpression(DW_OP_LLVM_entry_value, 1), debug-location !21
$eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, debug-location !24
$rcx = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !24
frame-destroy CFI_INSTRUCTION def_cfa_offset 8, debug-location !24
RET64 $eax, debug-location !24
...
Loading