From 6cce004c4ff45df3d864a88c36582fca96cd74d3 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Fri, 22 Nov 2024 11:58:34 +0000 Subject: [PATCH 1/2] [DebugInfo] Handle trailing empty blocks when seeking prologue_end spot The optimiser will produce empty blocks that are unconditionally executed according to the CFG -- while it may not be meaningful code, and won't get a prologue_end position, we need to not crash on this input. --- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 57 +++++++++++-------- .../MIR/X86/dbg-prologue-backup-loc2.mir | 49 ++++++++++++++++ 2 files changed, 83 insertions(+), 23 deletions(-) diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp index affc72b5950fd..09adf79dac478 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -2192,14 +2192,42 @@ findPrologueEndLoc(const MachineFunction *MF) { // better off synthesising an early prologue_end. auto CurBlock = MF->begin(); auto CurInst = CurBlock->begin(); - while (true) { + + // Find the initial instruction, we're guaranteed one by the caller. + while (CurBlock->empty()) + CurInst = (++CurBlock)->begin(); + + // Helper function for stepping through the initial sequence of + // unconditionally executed instructions. The optimiser will also chuck in + // sequences of empty blocks alas. + auto getNextInst = [&CurBlock, &CurInst, MF]() -> bool { + // We've reached the end of the block. Did we just look at a terminator? + if (CurInst->isTerminator()) { + // Some kind of "real" control flow is occurring. At the very least + // we would have to start exploring the CFG, a good signal that the + // prologue is over. + return false; + } + + // If we've already fallen through into a loop, don't fall through + // further, use a backup-location. + if (CurBlock->pred_size() > 1) + return false; + + // Fall-through from entry to the next block. This is common at -O0 when + // there's no initialisation in the function. Bail if we're also at the + // end of the function, or the remaining blocks have no instructions. // Skip empty blocks, in rare cases the entry can be empty. - if (CurInst == CurBlock->end()) { + do { ++CurBlock; - CurInst = CurBlock->begin(); - continue; - } + if (CurBlock == MF->end()) + return false; + } while (CurBlock->empty()); + CurInst = CurBlock->begin(); + return true; + }; + while (true) { // Check whether this non-meta instruction a good position for prologue_end. if (!CurInst->isMetaInstruction()) { auto FoundInst = ExamineInst(*CurInst); @@ -2216,25 +2244,8 @@ findPrologueEndLoc(const MachineFunction *MF) { continue; } - // We've reached the end of the block. Did we just look at a terminator? - if (CurInst->isTerminator()) { - // Some kind of "real" control flow is occurring. At the very least - // we would have to start exploring the CFG, a good signal that the - // prologue is over. - break; - } - - // If we've already fallen through into a loop, don't fall through - // further, use a backup-location. - if (CurBlock->pred_size() > 1) - break; - - // Fall-through from entry to the next block. This is common at -O0 when - // there's no initialisation in the function. Bail if we're also at the - // end of the function. - if (++CurBlock == MF->end()) + if (!getNextInst()) break; - CurInst = CurBlock->begin(); } // We couldn't find any source-location, suggesting all meaningful information diff --git a/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir b/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir index c27655ac80131..29cdbc0853365 100644 --- a/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir +++ b/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir @@ -31,6 +31,13 @@ # CHECK-NEXT: .LBB0_1: # CHECK-LABEL: addl %esi, %edx +## Second function in this file: test that we don't crash when having trailing +## empty blocks and no location for a prologue. Test that a .loc is produced, +## with an implicit-not check for there being no prologue_end. +# +# CHECK-LABEL: f: +# CHECK: .loc 0 1234 0 + --- | ; ModuleID = 'out2.ll' @@ -66,6 +73,17 @@ ret i32 0, !dbg !17 } + define void @f() !dbg !18 { + entry: + %0 = call ptr @llvm.returnaddress(i32 0) + br label %do.body + + do.body: + unreachable + } + + declare ptr @llvm.returnaddress(i32 immarg) + !llvm.dbg.cu = !{!2} !llvm.module.flags = !{!6, !7} !llvm.ident = !{!8} @@ -88,6 +106,7 @@ !15 = distinct !DILexicalBlock(scope: !9, file: !3, line: 4, column: 15) !16 = !DILocation(line: 6, column: 9, scope: !15) !17 = !DILocation(line: 8, column: 3, scope: !9) + !18 = distinct !DISubprogram(name: "f", scope: !3, file: !3, line: 37, type: !10, scopeLine: 1234, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !2) ... --- @@ -132,3 +151,33 @@ body: | RET64 $eax, debug-location !17 ... +--- +name: f +alignment: 16 +tracksRegLiveness: true +noPhis: true +isSSA: false +noVRegs: true +hasFakeUses: false +tracksDebugUserValues: true +frameInfo: + stackSize: 8 + offsetAdjustment: -8 + maxAlignment: 1 + maxCallFrameSize: 0 + isCalleeSavedInfoValid: true +fixedStack: + - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16 } +machineFunctionInfo: + amxProgModel: None +body: | + bb.0.entry: + frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp + frame-setup CFI_INSTRUCTION def_cfa_offset 16 + frame-setup CFI_INSTRUCTION offset $rbp, -16 + $rbp = frame-setup MOV64rr $rsp + frame-setup CFI_INSTRUCTION def_cfa_register $rbp + + bb.1.do.body: + +... From d1899c0598ed349fffac3bdcfc5dbfa59733cd1d Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Mon, 25 Nov 2024 18:56:26 +0000 Subject: [PATCH 2/2] Review feedback --- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp index 09adf79dac478..e1291e2a14a66 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -2193,13 +2193,14 @@ findPrologueEndLoc(const MachineFunction *MF) { auto CurBlock = MF->begin(); auto CurInst = CurBlock->begin(); - // Find the initial instruction, we're guaranteed one by the caller. + // Find the initial instruction, we're guaranteed one by the caller, but not + // which block it's in. while (CurBlock->empty()) CurInst = (++CurBlock)->begin(); + assert(CurInst != CurBlock->end()); // Helper function for stepping through the initial sequence of - // unconditionally executed instructions. The optimiser will also chuck in - // sequences of empty blocks alas. + // unconditionally executed instructions. auto getNextInst = [&CurBlock, &CurInst, MF]() -> bool { // We've reached the end of the block. Did we just look at a terminator? if (CurInst->isTerminator()) { @@ -2217,7 +2218,9 @@ findPrologueEndLoc(const MachineFunction *MF) { // Fall-through from entry to the next block. This is common at -O0 when // there's no initialisation in the function. Bail if we're also at the // end of the function, or the remaining blocks have no instructions. - // Skip empty blocks, in rare cases the entry can be empty. + // Skip empty blocks, in rare cases the entry can be empty, and + // other optimisations may add empty blocks that the control flow falls + // through. do { ++CurBlock; if (CurBlock == MF->end())