Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value

[Tom Tromey <tom@tromey.com>]

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

Kevin> This patch adds support for DW_OP_GNU_variable_value to GDB.
Kevin> Jakub Jelinek provides a fairly expansive discussion of this DWARF
Kevin> expression opcode in his GCC patch...

Thank you for doing this.  A few minor comments below.

Kevin> I also have test case (later in this series) which uses the DWARF
Kevin> assembler and, therefore, does not rely on having a compiler with this
Kevin> support.

Very nice.
    
Kevin> +  /* Execute "variable value" operation on DIED at SECT_OFF.  */

I think that should probably read "... on the DIE at ...".

Kevin> +struct value *
Kevin> +sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off,
Kevin> +		     struct dwarf2_per_cu_data *per_cu)

A new function should have an introductory comment explaining the
arguments, return value, and semantics.

Kevin> @@ -3655,6 +3696,9 @@ dwarf2_compile_expr_to_ax (struct agent_expr *expr, struct axs_value *loc,
Kevin>  	case DW_OP_call_ref:
Kevin>  	  unimplemented (op);
 
Kevin> +	case DW_OP_GNU_variable_value:
Kevin> +	  unimplemented (op);

Is it possible to implement this?  Just curious, it doesn't seem super
important to me.  In fact, is it ever even useful in an agent
expression?

I think the patch should also update disassemble_dwarf_expression.
It's an oddity that there are two DWARF disassembler in the tree, but
not your problem :)

Tom