Re: [PATCH 01/23] dwarf: add dwarf3 DW_OP_push_object_address opcode

[Joel Brobecker <brobecker at adacore dot com>]

Hello Keven,

On Wed, Jun 04, 2014 at 07:54:04AM +0200, Keven Boell wrote:
> The opcode pushes the address of the object being evaluated. The semantic is
> equivalent to the implicit push of the base address of a data member location.
> 
> 2014-05-28  Sanimir Agovic  <sanimir.agovic@intel.com>
>             Keven Boell  <keven.boell@intel.com>
> 
> 	* dwarf2expr.c (execute_stack_op) <DW_OP_push_object_address>: New case.
> 	* dwarf2expr.h (struct dwarf_expr_context_funcs)
> 	<DW_OP_push_object_address>: New function pointer get_object_addr.
> 	* dwarf2loc.c (struct dwarf_expr_baton): Add obj_address.
> 	(dwarf_expr_get_obj_addr): New function.
> 	(struct dwarf_expr_context_funcs): Add
> 	dwarf_expr_get_obj_addr to dwarf_expr_ctx_funcs.
> 	(dwarf2_evaluate_loc_desc_full): Initialize baton.obj_address.
> 	(dwarf2_locexpr_baton_eval): Set baton.obj_address to addr.
> 	(needs_get_obj_addr): New function.
> 	(struct dwarf_expr_context_funcs): Add needs_get_obj_addr to
> 	needs_frame_ctx_funcs.

Some comments and questions below...

My main question is relative to the new "addr" field. In many cases,
inferior data is not referenced by address, but via  "struct value"
objects. And such objects are not necessarily addressable (Eg: they
live inside a register, or are broken down into pieces, etc).
I am wondering how things are going to be working in this case,
and in particular, for a code that have a struct value object,
and tries to resolve its type into a static type, how does it
provide an address, knowning that it does not always exist? Perhaps
we can make the assumption for such complex objects that when
a DW_OP_push_object_address is used, the object is always assumed
to be addressable? Whatever the answer is, I think it's worth
documentation clearly what the expectations are.  Also, it's interesting
to note that several callers currently pass 0 as the (unused) address,
but will eventually need to be update to pass the correct info.

> +/* DW_OP_push_object_address has a frame already passed thru.  */
                                                           ^^^^
Sorry to nit-pick on this one, but I'd rather we spelled words
correctly.

>  CORE_ADDR dwarf2_read_addr_index (struct dwarf2_per_cu_data *per_cu,
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index d58193e..2a0cfe4 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -1646,7 +1646,7 @@ is_dynamic_type (struct type *type)
>  }
>  
>  static struct type *
> -resolve_dynamic_range (struct type *dyn_range_type)
> +resolve_dynamic_range (struct type *dyn_range_type, CORE_ADDR addr)

I just noticed that this function was missing a short description
(my bad), so I added it. Would you mind amending this patch to
update the description and add the meaning of ADDR?

-- 
Joel