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

[Keven Boell <keven dot boell at linux dot intel dot com>]

Hi Joel,

The address fields were introduced as DW_OP_push_object_address
needs an object address to be pushed and this address needs to be stored
somewhere. However, I will extend the log message why this address fields
were introduced.

I've addressed all your other comments and will send out 
a version 2 of the patch series soon, because some rebasing 
had to be done anyway.

Keven


> 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?
>