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

[Leonard Chan via Phabricator via llvm-commits]

leonardchan marked an inline comment as done.
leonardchan added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp:758
+    MIB.addImm(0U);
+  else
+    MIB.addReg(0U, RegState::Debug);
----------------
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
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73526