[PATCH] D67492: [DebugInfo] Add a DW_OP_LLVM_entry_value operation

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:693
     // Create an entry value expression where the expression following
-    // the 'DW_OP_entry_value' will be the size of 1 (a register operation).
-    DIExpression *EntryExpr = DIExpression::get(MF->getFunction().getContext(),
-                                                {dwarf::DW_OP_entry_value, 1});
+    // the 'DW_OP_LLVM_entry_value' will be the size of 1 (a register
+    // operation).
----------------
vsk wrote:
> -> "will contain 1 operation (e.g. a register op, or a constant literal)" ?
Currently, we do not support expressions e.g. dw_op_entry_value $constant.


================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:896
       // of following expression should be 1, because we support only
       // entry values of a simple register location.
       return I->get() == expr_op_begin()->get() && I->getArg(0) == 1 &&
----------------
dstenb wrote:
> dstenb wrote:
> > djtodoro wrote:
> > > djtodoro wrote:
> > > > aprantl wrote:
> > > > > djtodoro wrote:
> > > > > > aprantl wrote:
> > > > > > > dstenb wrote:
> > > > > > > > aprantl wrote:
> > > > > > > > > Can someone remind me what the reason for this limitation was again? Do we still need it after this patch?
> > > > > > > > I think that the main (or perhaps only?) reason for this limitation is that we, at emission, currently don't have a good way to calculate the size of the block that the entry value covers, and emit that size as a ULEB128 operand before the block. This patch will not change that.
> > > > > > > I see. Could you please add this to the comment here, so it's clear why the restriction exists and how to lift it in the future?
> > > > > > >I think that the main (or perhaps only?) reason for this limitation is that we, at emission, currently don't have a good way to calculate the size of the block that the entry value covers, and emit that size as a ULEB128 operand before the block.
> > > > > > 
> > > > > > We also wanted to ensure that LLVM currently only generates entry values of size 1.
> > > > > Is there another reason for this other than that it happened to be hard to implement in LLVM at the time? Perhaps on the consumer side?
> > > > Actually no. We found the simple register location as an entry value is very basic case (and easy to implement as the initial patch). We also knew that supporting the other types of entry values will take some time and we just left it as a future work.
> > > > The consumer  (only GDB at the time) supports parsing various types of entry values. 
> > > In addition to this, I work on extending the debug entry values to support complex expressions (more precisely, expressions other than simple register locations).
> > > The consumer (only GDB at the time) supports parsing various types of entry values.
> > 
> > It seems like GDB only supports a few operations inside a `DW_OP_entry_value`. I have tried some examples with more advanced expressions, but then got hit with the following error from its expression evaluator:
> > 
> > ```
> > DWARF-2 expression error: DW_OP_entry_value is supported only for single DW_OP_reg* or for DW_OP_breg*(0)+DW_OP_deref*
> > ```
> > 
> > That error message seems present in GDB 8.3 at least. I might overlook something here though.
> > In addition to this, I work on extending the debug entry values to support complex expressions (more precisely, expressions other than simple register locations).
> 
> Oh, okay! Sorry, I have been working on adding support for `DW_OP_regx` entry values, by extending `DwarfExpression` so that it can count the size of the entry value block, and emit that size operand before the block itself. I guess that overlaps with the complex expressions a bit?
> 
> I uploaded a WIP patch for that: D67674.
> 
> Do you think that would be usable for your work?
>It seems like GDB only supports a few operations inside a DW_OP_entry_value. I have tried some examples with more advanced expressions, but then got hit with the following error from its expression evaluator:

>DWARF-2 expression error: DW_OP_entry_value is supported only for single DW_OP_reg* or for DW_OP_breg*(0)+DW_OP_deref*

>That error message seems present in GDB 8.3 at least. I might overlook something here though.
You are right. Even GDB does not support every type of the debug entry value.

>Oh, okay! Sorry, I have been working on adding support for DW_OP_regx entry values, by extending DwarfExpression so that it can count the size of the entry value block, and emit that size operand before the block itself. I guess that overlaps with the complex expressions a bit?
>
>I uploaded a WIP patch for that: D67674.
>
>Do you think that would be usable for your work?

No problem. :) Please continue with this work, I will focus on reconstructing debug values of modified parameters (and local variables) that can be expressed in terms of the debug entry values (e.g. dw_op_entry_value dw_op_reg; dw_op_plus const).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67492