[PATCH] D106557: [DebugInfo] Fix crash when updating DBG_VALUE users of an SSA value modified by tail duplication

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer created this revision.
StephenTozer added reviewers: aprantl, djtodoro, dblaikie, respindola, probinson.
StephenTozer added a project: debug-info.
Herald added subscribers: pengfei, ormris, hiraditya.
StephenTozer requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Fixes bug 50441: https://bugs.llvm.org/show_bug.cgi?id=50441

Currently, during the "Update SSA" step of tail duplication, any `DBG_VALUE*` users of any updated SSA values will be deleted. Currently this is done by iterating the use list of the vreg being updated and erasing instructions as they are seen. This worked previously but may fail if a `DBG_VALUE_LIST` instruction uses the vreg multiple times, because after erasing the instruction for the first use we will encounter the second use and attempt to erase the same instruction again, causing an error. This is fixed simply by collecting the instructions to be removed into a set, and erasing after completing iteration.

As an open question to anyone vaguely familiar with this code, is there a reason that the debug values are being deleted instead of set as undef in this block? The patch that added this behaviour, e0304d1df, seems to have done so intentionally (the commit says "Avoid creating debug uses of undef, as they become a kill."), but this looks like it will produce incorrect debug information to me - if a debug value is deleted, it may incorrectly lengthen the range of the previous debug value for that variable. Furthermore, it should be a trivial matter to update the debug values - a direct update for the previously used SSA value is available, and I don't know if there's a good reason for not doing so.

I will likely open another review making this change, but it doesn't need to be tied to this fix, and it would also be useful if anyone with more domain knowledge has a good reason for the current behaviour.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106557

Files:
  llvm/lib/CodeGen/TailDuplicator.cpp
  llvm/test/CodeGen/X86/tail-dup-debugvalue.mir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D106557.360815.patch
Type: text/x-patch
Size: 7553 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210722/2df8f00c/attachment.bin>