OCHyams
commented
3 years ago Oops, I said:
If you run this example through a debugger you will see "first=5" on line 6 when you'd expect "first=20". but the example isn't linkable and doesn't include a call to the troublesome function. Here is the executable reproducer which gives the necessary context:
$ cat test.c attribute((optnone)) void esc(int* p) {} attribute((optnone)) void fluff() {} attribute((noinline)) int fun(int first, int second) { if (first) first = second; fluff(); esc(&first); return 0; } int main() { return fun(5, 20); }
$ clang -O2 -g test.c -o test
$ lldb test (lldb) target create "test" Current executable set to 'test' (x86_64). (lldb) b fun Breakpoint 1: where = test`fun + 1 at test.c:4:7, address = 0x00000000004004a1 (lldb) run Process 13431 launched: 'test' (x86_64) Process 13431 stopped
- thread #1, name = 'test', stop reason = breakpoint 1.1 frame #0: 0x00000000004004a1 test`fun(first=5, second=20) at test.c:4:7 1 attribute((optnone)) void esc(int* p) {} 2 attribute((optnone)) void fluff() {} 3 attribute((noinline)) int fun(int first, int second) { -> 4 if (first) 5 first = second; 6 fluff(); 7 esc(&first); (lldb) s Process 13431 stopped
- thread #1, name = 'test', stop reason = step in frame #0: 0x00000000004004aa test`fun(first=5, second=20) at test.c:6:3 3 attribute((noinline)) int fun(int first, int second) { 4 if (first) 5 first = second; -> 6 fluff(); 7 esc(&first); 8 return 0; 9 }
Notice that we see first=5 on line 6, which is wrong. We should see first=20 because the condition (first != 0) guarding the assignment (first = second) was true.
llvmbot
jmorse
Extended Description
TL;DR
llvm's debug info model is ineffective for variables which cannot be promoted in the first round of SROA/mem2reg and we can see incorrect debug info being generated as a result.
Example
clang built at 1ecae1e62ad0 (10th Jan 2021).
We can get incorrect debug info for variables which are promoted (or partially promoted) after InstCombine runs LowerDbgDeclare. I think this is a general problem with the debug info model which manifests in more than one place (see summary at end), but here is a specific example first:
Compile it with the following command and look at the IR:
Notice that the second dbg.value for "first" (#12) unexpectedly comes after the call to "fluff()". If you run this example through a debugger you will see "first=5" on line 6 when you'd expect "first=20". The dbg.value should instead be positioned immediately after the select instruction.
Example walkthrough
In the example "first" is escaped so it is not promoted by the early SROA/mem2reg pass. InstCombine will partially promote it by merging the stores (the initial store and the store on the if-then branch) into their common successor by inserting a PHI and store. Later SimplifyCFG folds all of the blocks together, turning the PHI into a select. InstCombine doesn't insert a dbg.value after the PHI and SimplifyCFG doesn't insert one when converting the PHI to a select, so the merged value is untracked in debug info.
We can see what happened by looking at the -print-after-all output. SROA/mem2reg runs early and is not able to promote "first" because it is escaped. So "first"'s alloca and dbg.declare survive:
Later, InstCombine runs and before doing any optimisations it calls LowerDbgDeclare to convert the dbg.declare into a set of dbg.value intrinsics:
InstCombine merges the two stores to %first.addr ("first"'s alloca address) into the common successor "if.end" as a PHI and store. Notice how it does not insert a dbg.value to describe the merged value %storemerge.
SimplifyCFG folds these blocks together, converting the PHI to a select, and leaves us with the incorrect debug info shown earlier:
Summary
This example shows that InstCombine and SimplifyCFG compose to produce incorrect debug info for escaped variables. However, the reason that I think this is a general problem rather than a specific bug, and why I am struggling to hold any one pass accountable, is because of the following inconsistencies:
If "first" in this example can be fully promoted by mem2reg (https://godbolt.org/z/Es89z7) then there is no issue because mem2reg will insert a dbg.value after the PHI which eventually becomes the select.
However, mem2reg can also cause the same bug because it only inserts dbg.values when promoting a variable if and only if it has a dbg.declare. It's possible for LowerDbgDeclare to remove the dbg.declare before mem2reg runs. For example, when a variable is escaped but the escaping function is later inlined. You can see this happening here https://godbolt.org/z/KrdjGs.
Lastly, we don't see this issue in any of these cases if there isn't any block folding because LiveDebugValues will merge live-out variable locations from preds (https://godbolt.org/z/sjcnbv).
As far as I can tell there are no rules or examples demonstrating how debug info should be updated when promoting or partially promoting variables after LowerDbgDeclare has removed the dbg.declare.