Re: [RFA/Ada] Crash when trying to set value of packed array element

[Jan Kratochvil <jan.kratochvil@redhat.com>]

Hi Joel,

On Thu, 01 Mar 2012 00:03:09 +0100, Joel Brobecker wrote:
> Still, this was not sufficient, because it caused a regression when
> trying to perform an aggregate assignment of a packed array of packed
> record.  The key element here is the nesting of packed entities.
> Looking at the way ada_value_primitive_packed_val creates the value
> of each sub-component, one can see that the value's offset is set
> to the offset compared to the start of the parent. This was meant to
> match what value_primitive_field does as well.
> 
> So, with our array of records, if the record offset was 2, and if
> the field we're interested in that record is at offset 1, the record
> value's offset would be set to 2, and the field value's offset would
> be set to 1. But the address for both values would be left to the
> array's address. This is where things start breaking down, because
> the value_address function for our field value would return the
> address of the array + 1, instead of + 3.
> 
> This is what causes the final issue, here, because ada-lang.c's
> value_assign_to_component needs to compute the offset of the
> subcomponent compared to the top-level aggregate's start address
> (the array in our case).

It depends on how is defined functionality of ada_value_primitive_packed_val.
Whether its parameter OFFSET is relative to the start of the array or whether
it is relative to the element.  I believe you want the latter and your patch
also implements the latter, otherwise I cannot imagine how callers would
handle it correctly.  In the latter case the parameter name OFFSET is
confusing and it should be renamed as it is _added_ to value_offset (OBJ).

In general I do not think we can ever fix it all on top of the current
infrastructure.  These OFFSETs, EMBEDDED_OFFSETs etc. should be dropped, there
should be support for discontiguos set of memory snapshot associated with
struct value (so that it supports for example even virtual method tables or
slices of multidimensional Fortran arrays).  I should find time for
archer-jankratochvil-vla.


> @@ -2310,18 +2309,22 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
>  
>    if (obj != NULL)
>      {
> -      CORE_ADDR new_addr;
> +      long new_offset = offset;

Nitpick - LONGEST (Siddhesh Poyarekar is working on a more thorough fix).


>  }
>  
> +void
> +set_value_parent (struct value *value, struct value *parent)

Missing function comment.


Thanks,
Jan