[PATCH] D75270: [DebugInfo] Do not emit entry values for composite locations

Wed Mar 4 05:06:37 PST 2020

dstenb added a comment.

In D75270#1897102 <https://reviews.llvm.org/D75270#1897102>, @vsk wrote:

> Fixing the assertion failure in this way sgtm. Re:

Sorry, coming back to this now after the similar discussion in D75326 <https://reviews.llvm.org/D75326>.

I just want to clarify that this patch does not fix any assertion failures. The entry value operation is emitted without crashing; it's just that I think that it's invalid DWARF. The assertion I mentioned in the description was for the `DW_AT_location` at the call site parameter entry (but see my correction about that at the end of this comment).

>   DW_OP_entry_value(DW_OP_regx sub_reg0), DW_OP_stack_value, DW_OP_piece 8, DW_OP_entry_value(DW_OP_regx sub_reg1), DW_OP_stack_value, DW_OP_piece 8, ...
> 
> 
> It sounds like you expect debuggers to have some difficulty handling this expression. How come? I didn't understand this part.

I don't know how much about how the call site and entry value matching works and is implemented in the debuggers, but I thought there could be some difficulties to understand that the each sub-register's entry value at the callee's location:

  DW_OP_entry_value(DW_OP_regx sub_reg0), DW_OP_stack_value, DW_OP_piece 8, DW_OP_entry_value(DW_OP_regx sub_reg1), DW_OP_stack_value, DW_OP_piece 8, ...

should be matched to the corresponding piece of the `DW_AT_location` for the call site's single parameter entry:

  DW_OP_regx sub_reg0, DW_OP_piece 8, DW_OP_regx sub_reg1, DW_OP_stack_value, DW_OP_piece 8, ...

Then, if we e.g. have a `DW_OP_constu` as the `DW_AT_call_value` for that call site parameter entry, I assume that the debugger has to split up that value into different parts according to the `DW_AT_location` composition, and push those to the DWARF stacks for the entry values in the different pieces?

Again, as I don't know much about the debuggers' implementations, maybe nothing of that is an issue really?

> My lack of familiarity with DWARF is showing now, but would `DW_OP_entry_value(DW_OP_regx sub_reg0, DW_OP_piece 8, DW_OP_regx sub_reg1, DW_OP_piece 8)` be valid (sorry if I've left out any necessary OP_stack_values)?

That is what we currently emit (with a stack value after the DW_OP_entry_value). I think that's invalid DWARF since the standard specifies that the `DW_OP_entry_value` operation's block is //"containing a DWARF expression or a register location description"//, so I don't think it's valid to emit a composite location description like that one.

And sorry, I noticed something incorrect in my patch description:

In D75270 <https://reviews.llvm.org/D75270>, @dstenb wrote:

> Currently when trying to emit a call site entry for a parameter composed
>  of multiple DWARF registers a (DwarfRegs.size() == 1) assert is
>  triggered in addMachineRegExpression(). Until the call site
>  representation is figured out, and until there is use for these entry
>  values in practice, this commit simply stops the invalid DWARF from
>  being emitted.

The call site entry's `DW_AT_location` is emitted fine for composite locations, so what I'm describing there does not happen for those cases. However, if you were to describe the call site value using a preserved register, which is composed of multiple DWARF register pieces, you'll currently hit that assertion (code here: [0]) since `isParameterValue()` is true.

[0] https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp#L279

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75270