[Debuginfo][TailCallElim] Misleading stepping caused by new BranchInst in NewEntry

[Apochens]

The source code:

int func(int a) {
    if (a > 10) {
        return 5 * func(a - 1);
    }
    if (a > 1) {
        return a * func(a - 1);
    } 
    return 1;
}

int main() {
    func(1);
}

The compilation commands:

$ clang -S -emit-llvm -Xclang -disable-O0-optnone main.c -o main.ll -g
$ opt -S -passes=mem2reg,tailcallelim main.ll -o opt.ll
$ clang opt.ll

The misleading stepping:

$ debug a.out 
(lldb) target create "a.out"
Current executable set to '/llvm-test/TailCallElim/a.out' (x86_64).
(lldb) b func
Breakpoint 1: where = a.out`func + 8 at main.c:3:20, address = 0x0000000000001138
(lldb) r
Process 2859495 launched: '/llvm-test/TailCallElim/a.out' (x86_64)
Process 2859495 stopped
* thread #1, name = 'a.out', stop reason = breakpoint 1.1
    frame #0: 0x0000555555555138 a.out`func(a=<unavailable>) at main.c:3:20
   1    int func(int a) {
   2        if (a > 10) {
-> 3            return 5 * func(a - 1);
   4        }
   5        if (a > 1) {
   6            return a * func(a - 1);
   7        } 
(lldb) s
Process 2859495 stopped
* thread #1, name = 'a.out', stop reason = step in
    frame #0: 0x0000555555555154 a.out`func(a=1) at main.c:2:11
   1    int func(int a) {
-> 2        if (a > 10) {
   3            return 5 * func(a - 1);
   4        }
   5        if (a > 1) {
   6            return a * func(a - 1);
   7        } 
(lldb) v
(int) a = 1

The debugger gets into then branch when the condition is not satisfied. This is caused by the newly created BranchInst BI in NewEntry. BI is inserted in the entry of the function (before the first if-statement), however, it preserves the debug location of the tail recursion call CI (the call func(a-1)), which could be located in a conditional branch.

The debug location of BI should be dropped.

[llvmbot]

@llvm/issue-subscribers-debuginfo

Author: Shan Huang (Apochens)

The source code: ```C int func(int a) { if (a > 10) { return 5 * func(a - 1); } if (a > 1) { return a * func(a - 1); } return 1; }

int main() {
func(1);
}

The compilation commands:
```bash
$ clang -S -emit-llvm -Xclang -disable-O0-optnone main.c -o main.ll -g
$ opt -S -passes=mem2reg,tailcallelim main.ll -o opt.ll
$ clang opt.ll

The misleading stepping:

$ debug a.out 
(lldb) target create "a.out"
Current executable set to '/llvm-test/TailCallElim/a.out' (x86_64).
(lldb) b func
Breakpoint 1: where = a.out`func + 8 at main.c:3:20, address = 0x0000000000001138
(lldb) r
Process 2859495 launched: '/llvm-test/TailCallElim/a.out' (x86_64)
Process 2859495 stopped
* thread #<!-- -->1, name = 'a.out', stop reason = breakpoint 1.1
    frame #<!-- -->0: 0x0000555555555138 a.out`func(a=&lt;unavailable&gt;) at main.c:3:20
   1    int func(int a) {
   2        if (a &gt; 10) {
-&gt; 3            return 5 * func(a - 1);
   4        }
   5        if (a &gt; 1) {
   6            return a * func(a - 1);
   7        } 
(lldb) s
Process 2859495 stopped
* thread #<!-- -->1, name = 'a.out', stop reason = step in
    frame #<!-- -->0: 0x0000555555555154 a.out`func(a=1) at main.c:2:11
   1    int func(int a) {
-&gt; 2        if (a &gt; 10) {
   3            return 5 * func(a - 1);
   4        }
   5        if (a &gt; 1) {
   6            return a * func(a - 1);
   7        } 
(lldb) v
(int) a = 1

The debugger gets into then branch when the condition is not satisfied. This is caused by the newly created BranchInst BI in NewEntry. BI is inserted in the entry of the function (before the first if-statement), however, it preserves the debug location of the tail recursion call CI (the call func(a-1)), which could be located in a conditional branch.

The debug location of BI should be dropped.

[dwblaikie]

Maybe rather than dropping, we need to merge the location of the two calls? Not entirely clear - but merging (even when, ultcimately, it results in dropping the location in most cases) does provide us some benefits (if the code is stamped out by a macro - both locations/paths would have the same location anyway, so we can preserve it in that case - and, if in the future we figure out some ways to represent ambiguous locations, we'll be able to update the merging logic once and get a lot of benefits from all the places calling into it - whereas dropped locations will have to be addressed manually)

[Apochens]

@dwblaikie Merging is not suitable for this case. Just as the following graph, the tail call is in a conditional branch, and it will be replaced by a br directly jumping to old entry instead of the new one. So, the new BI can only be executed when this function is called by other functions (not itself). Consequently, BI has no relation to tail calls and we should not propagate the debug locations of tail calls to it.

new entry    <-   BI
   |
old entry
   |     \
   |     tail call    <- CI replaced by a unconditional br to old entry 
   |     /
the  exit