Revise prologue-end setting as per Davids feedback

Author: jmorse

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

@@ -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.
@@ -2305,15 +2306,36 @@ findPrologueEndLoc(const MachineFunction *MF) {
         return *FoundInst;
     }
 
-    // We choose to ignore line-zero locations when setting the prologue as they
-    // can't be stepped on anyway; however in very rare scenarios function calls
-    // can have line zero, and we shouldn't step over those. In these
-    // extraordinary conditions, just bail out and refuse to set a prologue_end.
-    if (CurInst->isCall())
-      if (const DILocation *Loc = CurInst->getDebugLoc().get())
-        if (Loc->getLine() == 0)
+    // 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()) {
+      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);

llvm/test/DebugInfo/X86/no-prologue-end-after-line0-calls.mir

@@ -1,4 +1,4 @@
-# RUN: llc %s -start-after=livedebugvalues -o - -filetype=obj | llvm-dwarfdump - --debug-line | FileCheck %s --implicit-check-not=prologue_end
+# RUN: llc %s -start-after=livedebugvalues -o - -filetype=obj | llvm-dwarfdump - --debug-line | FileCheck %s
 #
 ## Original code, compiled clang -O2 -g -c
 ## 
@@ -23,25 +23,25 @@
 ##   a:   59                      pop    %rcx
 ##   b:   c3                      ret
 ##
-## Where prologue_end is placed on address 8, the clearing of the return
-## register, because it's the first "real" instruction that isn't line zero.
-## This then causes debuggers to skip over the call instruction when entering
-## the function, which is catastrophic.
+## 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 shouldn't put a prologue_end on this function at all. It's too
-## deformed from the original code to truly have a position (with a line number)
-## that is both true, and after frame setup. This gives consumers the
+## 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 ensure that there's something meaningful in the line table
-## involving line 2, the implicit-check-not is making sure there isn't a
-## prologue_end flag on any line entry.
+## 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: standard_opcode_lengths[DW_LNS_set_prologue_end] = 0
-#
-# CHECK:  Address            Line   Column File
-# CHECK:  2      0      0 
-# CHECK:  end_sequence
+# 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
+
 --- |
   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"