Re: [PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

["Ulrich Weigand" <uweigand@de.ibm.com>]

Andreas Arnez wrote:
> On Mon, Mar 13 2017, Ulrich Weigand wrote:
> > Thinking about this more, maybe it would be clearer to swap around the
> > two types, and have the semantics of the function be something like:
> >
> > /* Evaluate a location description, starting at DATA and with length
> >    SIZE, to find the current location of variable of TYPE in the
> >    context of FRAME.  If SUBOBJ_TYPE is non-NULL, return instead the
> >    location of the subobject of type SUBOBJ_TYPE at byte offset
> >    SUBOBJ_BYTE_OFFSET within the variable of type TYPE.  */
> 
> OK.  Of course this increases the amount of change a bit.  New patch
> below.

Well, yes ... I still think it is clearer overall in the end.

> +  /* Get type of pointed-to DIE.  */
> +  struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu);
> +
>    /* If pointed-to DIE has a DW_AT_location, evaluate it and return the
>       resulting value.  Otherwise, it may have a DW_AT_const_value instead,
>       or it may've been optimized out.  */
>    if (baton.data != NULL)
> -    return dwarf2_evaluate_loc_desc_full (TYPE_TARGET_TYPE (type), frame,
> -					  baton.data, baton.size, baton.per_cu,
> +    return dwarf2_evaluate_loc_desc_full (orig_type, frame, baton.data,

Can orig_type ever be NULL here?  It probably would not be a good idea
to pass NULL as the type argument.

> -	    if (byte_offset != 0)
> +	    if (subobj_byte_offset != 0)
>  	      error (_("cannot use offset on synthetic pointer to register"));

As an aside: now that we have the full object type, we may actually be able
to lift that restriction (do a value_from_register on the full type, and
then extract the subobject).  Might be interesting to look into as a
follow-on change ...

>  	    size_t n = TYPE_LENGTH (value_type (value));
> +	    size_t len = TYPE_LENGTH (subobj_type);
> +	    size_t max = type != NULL ? TYPE_LENGTH (type) : n;

As mentioned above, I don't think it makes sense to allow a NULL type here.

Otherwise, this looks good to me.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com