[PATCH] D83495: [DebugInfo] Add DWARF emission for DBG_VALUE_LIST

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added inline comments.


================
Comment at: llvm/test/DebugInfo/X86/dbg_value_list_emission.mir:51-55
+    ; (1) Check a single reg arg works.
+    DBG_VALUE_LIST !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_stack_value), $eax, debug-location !15
+    ; CHECK:      DW_TAG_variable
+    ;  CHECK-NEXT:   (DW_OP_breg0 RAX+0, DW_OP_stack_value)
+    ;  CHECK-NEXT:   DW_AT_name ("locala")
----------------
scott.linder wrote:
> Why does this behave differently than (what I understand to be) the equivalent `DBG_VALUE` ?
> 
> ```
> DBG_VALUE $eax, $noreg, !12, !DIExpression(DW_OP_stack_value), debug-location !15
> ; becomes: DW_AT_location (DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_stack_value)
> ```
> 
> It seems like the `DW_OP_and` is there to select a subregister (I assume EAX), but oddly it comes after the value of the register is already read (i.e. after the DW_OP_breg). I'm lost on what the intended behavior is, and why it differs between `DBG_VALUE` and `DBG_VALUE_LIST`.
> 
> There is also the existing confusion around the "isIndirect" flag in `DBG_VALUE` which makes these two equivalent (and both seemingly wrong):
> 
> ```
> DBG_VALUE $eax, $noreg, !25, !DIExpression(DW_OP_stack_value), debug-location !15
> ; becomes: DW_AT_location    (DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_stack_value)
>   
> DBG_VALUE $eax, 0, !26, !DIExpression(DW_OP_stack_value), debug-location !15
> ; becomes: DW_AT_location    (DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_stack_value)
> ```
> 
> which makes it harder still to compare.
> 
> 
> Would it be more straightforward to always be explicit about indirection in the new form? Why does `DW_OP_stack_value` imply a `DW_OP_deref` at all? I.e. why do we not get:
> 
> ```
> DBG_VALUE_LIST !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_stack_value), $eax, debug-location !15
>     ; CHECK:      DW_TAG_variable
>     ;  CHECK-NEXT:   (DW_OP_reg RAX, DW_OP_stack_value)
>     ;  CHECK-NEXT:   DW_AT_name ("locala")
> ```
> 
> which in this case I imagine would just be an error. I would expect the correct expression to generate the `DW_OP_breg` would be something like:
> 
> ```
> DBG_VALUE_LIST !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_deref, DW_OP_stack_value), $eax, debug-location !15
>     ; CHECK:      DW_TAG_variable
>     ;  CHECK-NEXT:   (DW_OP_breg0 RAX+0, DW_OP_stack_value)
>     ;  CHECK-NEXT:   DW_AT_name ("locala")
> ```
> 
> If we don't do this, we seem to retain some of the same ambiguity that makes the old "isIndirect" field so confusing.
To the first point: I'm looking into it now; I noticed the `DW_OP_and` before, but I'm not sure where it's coming from myself yet - the `DBG_VALUE_LIST` handling should be following essentially the same code path as `DBG_VALUE`, so this is a bug one way or another.

To the second point, I think your examples are slightly incorrect:

The `isIndirect` flag in `DBG_VALUE` is confusing and inconsistent, what it actually does is dependent on the DIExpression and not well explained. The `DBG_VALUE_LIST` implementation has no such inconsistencies however (I hope). The problem with your examples is that I think you're using `DW_OP_reg` to mean a register's literal value, and `DW_OP_breg` to mean the address pointed to by a register. This isn't quite correct, although they do act like that for most variable locations.

The actual meanings are slightly more complicated; the short answer is that `DW_OP_reg` is a Register location description: it refers to the register itself, not to the value of that register. `DW_OP_breg` on the other hand //does// refer to the literal value of a register; it's generally used with an offset as part of a Memory location expression, but if combined with `DW_OP_stack_value` then it gives the value in the register as the variable's value (albeit as an Implicit location rather than a Register location).

So with all of that said, the meaning of `(DW_OP_breg0 RAX+0, DW_OP_stack_value)` is that the variable's value can be found in `$rax`, but the variable should be read-only, which matches the meaning of the `DBG_VALUE_LIST`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83495