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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp:758
+    MIB.addImm(0U);
+  else
+    MIB.addReg(0U, RegState::Debug);
----------------
leonardchan wrote:
> aprantl wrote:
> > This is surprising to me. Can you walk me through this as a sanity check to make sure this isn't papering over some other issue?
> > 
> > Both the before and after state should be equivalent:
> > Before your patch we emit a direct register location + DW_OP_deref afterwards we emit an indirect register location.
> > 
> > What am I missing?
> I'll admit I'm not an expert in this area of LLVM, but my understanding is that this would affect the order of the expression stack.
> 
> Before my patch, if this value was an indirection and the expression was `DW_OP_constu, 8, DW_OP_minus`, then we would append `DW_OP_deref` to the end of the stack and we would not actually deref the value immediately.
> 
> With this patch (which is essentially a revert of https://reviews.llvm.org/D68945#change-VaZel0Y6QQdl), the indirection happens first, then the expressions on the stack.
> 
> This would essentially turn something like:
> 
> ```
> DW_OP_breg6 RBP-40, DW_OP_constu 0x20, DW_OP_minus, DW_OP_deref
> ```
> 
> into
> 
> ```
> DW_OP_breg6 RBP-40, DW_OP_deref, DW_OP_constu 0x20, DW_OP_minus
> ```
I see. So this should have been DIExpression::prepend instead of append? Could we implement it that way instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73526