[DebugInfo@O2] Incorrect debug info for escaped variables after LowerDbgDeclare runs #48063
Comments
|
I've written a few of these cases as dexter tests in D94761. unused-merged-value.c in that patch demonstrates how we are able to run into this issue for non-escaped variables too. |
|
Oops, I said:
$ cat test.c $ clang -O2 -g test.c -o test $ lldb test
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. |
|
@llvm/issue-subscribers-debuginfo Author: Orlando Cazalet-Hyams (OCHyams)
| | |
| --- | --- |
| Bugzilla Link | [48719](https://llvm.org/bz48719) |
| Version | trunk |
| OS | All |
| Blocks | llvm/llvm-project#38116 |
| CC | @gregbedwell,@jmorse,@jdm |
Extended DescriptionTL;DRllvm'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. Exampleclang built at 1ecae1e (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: $ cat -n test.c 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 walkthroughIn 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. *** IR Dump After Combine redundant instructions *** SimplifyCFG folds these blocks together, converting the PHI to a select, and leaves us with the incorrect debug info shown earlier: SummaryThis 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. |
|
@OCHyams Can we close this now that assignment-tracking exists? LTO-mode notwithstanding |
|
Hmm yes and no. The more fundamental issue has been fixed but it looks like issue that SimplifyCFG introduces still exists https://godbolt.org/z/3xP5qjx73 I think the dbg.assign after |
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 1ecae1e (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.
The text was updated successfully, but these errors were encountered: