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

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

Andreas Arnez wrote:

> Recently I fixed a bug that caused a DW_OP_implicit_pointer with non-zero
> offset into a DW_OP_implicit_value to be handled incorrectly on big-endian
> targets.  GDB ignored the offset and copied the wrong bytes:
> 
>   https://sourceware.org/ml/gdb-patches/2017-01/msg00251.html
> 
> But there is still a similar issue when a DW_OP_implicit_pointer points
> into a DW_OP_stack_value instead; and again, the offset is ignored.  There
> is an important difference, though: While implicit values are treated like
> blocks of data and anchored at the lowest-addressed byte, stack values
> traditionally contain integer numbers and are anchored at the *least
> significant* byte.  Also, stack values do not come in varying sizes, but
> are cut down appropriately when used.  Thus, on big-endian targets the
> scenario looks like this (higher addresses shown right):
> 
>   |<- - - - - Stack value - - - - - - ->|
>                   |                     |
>                   |<- original object ->|
>                   |
>                   | offset ->|####|
> 			      ^^^^
>                               de-referenced
> 			      implicit pointer
> 
> (Note how the original object's size influences the position of the
> de-referenced implicit pointer within the stack value.  This is not the
> case for little-endian targets, where the original object starts at offset
> zero within the stack value.)
> 
> This patch implements the logic indicated in the above diagram and adds an
> appropriate test case.  A new function dwarf2_fetch_die_type_sect_off is
> added; it is used for retrieving the original object's type, so its size
> can be determined.  That type is passed to dwarf2_evaluate_loc_desc_full
> via a new parameter.

This makes sense to me.



> @@ -50,7 +50,8 @@ static struct value *dwarf2_evaluate_loc_desc_full (struct type *type,
>  						    const gdb_byte *data,
>  						    size_t size,
>  						    struct dwarf2_per_cu_data *per_cu,
> -						    LONGEST byte_offset);
> +						    LONGEST byte_offset,
> +						    struct type *orig_type);

Please update the comment to indicate the precise meaning of the ORIG_TYPE
parameter, and when it has to be specified and when it may be omitted.

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.  */


Otherwise the patch looks good to me.

Bye,
Ulrich

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