Assertion failure: "conflicting locations for variable" #64149
Comments
|
@llvm/issue-subscribers-debuginfo |
|
The problem here is the two dbg.declares sort-of overlapping (one describes the location of a field in the source variable, the other describes the location of an entire variable). I don't believe we can have a well defined answer to what that means: variables can't have two locations for any part of them(DWARF can express it, but I don't believe LLVM is designed to handle that). Adding a fragment to the second dbg.declare that overlaps with the first causes a different assertion to fire for similar reasons. IMO the solution is:
|
|
@cuviper Do you happen to still have the original, unreduced bitcode for this? |
|
I do -- I just added it to bugzilla. |
|
The IR that rustc produces reduces to something like this: https://gist.github.com/nikic/efc3134a0aee3cb9f71601e356c5065b This doesn't trigger an assert by itself, but does after However that input still has two dbg.declares on different allocas with the same DILocalVariable, so I guess that's still an invalid input, just not in a way that crashes llc. |
|
Here's a simple example where rustc assigns the same DILocalVariable to multiple allocas: https://rust.godbolt.org/z/M4cPKhWaE The IR contains something like this: This looks pretty nonsensical to me. |
|
On the producer side, this is now filed at rust-lang/rust#115113 But on the LLVM side, it would still be nice to catch this in the IR verifier. |
|
@jmorse Could you please clarify what the precise requirements here are? Just having multiple dbg.declares for the same DILocalVariable seems like something that should be fine when inlining is involved, as you can legitimately have multiple copies of the same variable. That behavior exists in clang as well (https://clang.godbolt.org/z/1v1orvxzW with assignment tracking disabled for more direct comparison). So I'm not entirely clear at which point "multiple overlapping dbg.declares" becomes invalid. Is it just the case where different fragments are involved? (The debuginfo generated by rustc is wrong in any case, I'm just trying to understand what the verifier check would actually do.) |
|
fyi @jmorse is away at the moment.
In that case the DILocation attached to the debugging intrinsic has an InlinedAt field which is another DILocation that describes the original call site. After inlining that debugging intrinsic a second time its InlinedAt DILocation will have an InlinedAt field too for that inlined call site. Unlike most DILocations, InlinedAt DILocations are created as distinct (rather than uniqued) metadata. We generally identify a variable in LLVM as a tuple of the (DILocalVariable, FragmentInfo from the DIExpression, InlinedAt from the DILocation). |
Thanks! This is the bit I was missing. It looks like it's not possible to verify that there are no repeated DebugVariables, because these may be introduced by transforms that perform code cloning (e.g. LoopRotate, LoopUnswitch, etc). This is discussed in #32504. So I guess the only thing we can verify is that all fragments for a given (DILocalVariable, InlinedAt) are non-overlapping (excluding exact overlaps). |
|
Testing this patch: https://gist.github.com/nikic/d8c328c4fe13df2d71ac468923b6cec9 Results in a few test failures: Annoyingly, this doesn't fail verification on the IR originally produced by rustc, because it doesn't specify fragments, so it looks like the variables are exactly the same. Possibly the way to catch that would be to check whether the size implied by the DILocalVariable is consistent with the size of the alloca. |
|
I've put up https://reviews.llvm.org/D158743 with a sanity check that the fragment can fit into the alloca, but still needs some test fixes. This doesn't catch the case that actually asserts, but does catch the invalid debuginfo produced by SROA in Rust. |
…size Add a check that the DILocalVariable fragment size in dbg.declare does not exceed the size of the alloca. This would have caught the invalid debuginfo regenerated by rustc in #64149. Differential Revision: https://reviews.llvm.org/D158743
…ragment size Reapply after fixing a clang bug this exposed in D158972 and adjusting a number of tests that failed for 32-bit targets. ----- Add a check that the DILocalVariable fragment size in dbg.declare does not exceed the size of the alloca. This would have caught the invalid debuginfo regenerated by rustc in #64149. Differential Revision: https://reviews.llvm.org/D158743
…ragment size Reapply after fixing a clang bug this exposed in D158972 and adjusting a number of tests that failed for 32-bit targets. ----- Add a check that the DILocalVariable fragment size in dbg.declare does not exceed the size of the alloca. This would have caught the invalid debuginfo regenerated by rustc in llvm#64149. Differential Revision: https://reviews.llvm.org/D158743
…ragment size Reapply now that generation of incorrect debuginfo for FnDef in rustc has been fixed. ----- Add a check that the DILocalVariable fragment size in dbg.declare does not exceed the size of the alloca. This would have caught the invalid debuginfo regenerated by rustc in #64149. Differential Revision: https://reviews.llvm.org/D158743
|
Has this been fixed in main? |
From https://bugzilla.redhat.com/show_bug.cgi?id=2226564:
Results in:
The text was updated successfully, but these errors were encountered: