[llvm] [DebugInfo] Don't set prologue_end behind line-zero call insts (PR #156850)

[Jeremy Morse via llvm-commits]

jmorse wrote:

(Still working through my email backlog sorry),

> The issue with putting prologue_end before such a call is that the frame might not be fully set up, so our variable locations might not be valid yet? Or some other issue?

Mostly that the frame usually won't be fully set up. In the added test case we can choose between:
 * Placing prologue_end on the call instruction itself, or the xor, which are both unhelpful as they're line-zero locations,
 * Placing prologue_end on the push instruction, which gets the "function scope" line number.

Where the push instruction is actually an OK choice in this specific case. However, I think for a majority of functions the last frame-setup instruction will be `sub $0x10, %rsp` or similar, and if we put prologue_end on that instruction or earlier then stack-variable-locations would be invalid.

> (& if our stack-frame based variable locations aren't valid yet - having no prologue_end might be as-bad-or-worse, right? Since you'll break before the very first instruction - so they'll definitely not be valid then?)

Indeed there are only poor choices in this situation -- I figure not setting prologue_end signals to the consumer that there's no good position at all (although I don't think this is a documented convention), which gives the consumer an opportunity to make decisions about it. For the sce debugger, with no prologue_end I believe we break on the first instruction in the function, and don't show any variable locations until stepping begins (85% confidence).

> & might be wroth knowing/discussing a bit a specific example of this problem to see what the tradeoffs are? Do you have a particular optimization/sequence of optimizations that produces an especially bad case?

Happily it's an extremely rare scenario -- some QA on a mid-to-large-sized C++ codebase found this problem in a single function out of thousands, and it was broadly what's in the test case code:

     void ext();
     int main(int argc, char **argv) {
        if (argc == 1)
          ext();
        else
          ext();
         return 0;
      }

Where the branch-folder pass, running extremely late (post-regalloc), identifies the code sequence on either side of the branch as being identical and hoists the calls, merging the source location into line zero. The original codebase had a much complicated tail. At entry it produced this assembly:

      0080: 55                            push    rbp
      0081: 48 89 e5                      mov     rbp, rsp
      0084: 53                            push    rbx
      0085: 50                            push    rax
      0086: 48 89 fb                      mov     rbx, rdi      ; line zero
      0089: e8 42 37 00 00                call    [something]   ; line zero

Where with todays clang/llvm we place prologue_end after the call instruction, meaning a breakpoint on the function enters after the call completes. Placing it on `mov rbx, rdi` would be putting it on line zero. Putting it on `push rax` would mean any stack locations would be slightly off.

I suppose we can determine whether there are any stack variables at risk by examining the `MachineFunction`, and we could emit a mostly-correct prologue_end if there aren't variables at risk, but there would still be functions with stack variables to consider.

Most of my motivation for fixing this (even though it's rare) is that, IMHO, stopping at the wrong point in the program is massively worse that displaying some uninitialized variables. Developers can usually spot the latter, but will be dismayed if they can't break into the right place.

https://github.com/llvm/llvm-project/pull/156850