[PATCH] D73526: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

The problem is that ever since we got https://reviews.llvm.org/D68945 LiveDebugVariables no longer expects that the DBG_VALUE is indirect (having an immeadiate as second operand) before DBG_VALUE is removed (prior to regalloc). So that patch changed the playground a little bit, as LiveDebugVariables no longer adds DW_OP_deref when adding back the DBG_VALUE instructions after regalloc.

That is why SelectionDAG etc was changed to add DW_OP_deref explicitly. And I suspect that it incorrectly used append instead of prepend in some places when doing that change.

A little bit interesting that D68945 <https://reviews.llvm.org/D68945> says "I've stripped out the "IsIndirect" field throughout LiveDebugVariables as its use case is gone, and added an assertion that LiveDebugVariables doesn't see any incoming DBG_VALUEs with IsIndirect set." and after first have landed this patch (https://reviews.llvm.org/rGfff6a1b0f1fe57b46379001db75952d2a06eab1f) it looks like it was reverted, and then it was landed again as https://reviews.llvm.org/rG2d3174c4df6b5f4131346828d0a31675d80d6e2b), but since the revision wasn't re-opened (?) there was no "update to reflect the commited change". However, that commit was changing the condition in the assert which was supposed to protect this from happening. Bah... yuck... that was evil.

So maybe we should revert this again then. Because afaict this isn't playing by the rules stipulated by D68945 <https://reviews.llvm.org/D68945>. Simply changing the assert condition was definitely not enough to add back all the logic that was removed from LiveDebugVariables in D68945 <https://reviews.llvm.org/D68945>.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73526/new/

https://reviews.llvm.org/D73526