[3/5] Fix "finish" for vector types on ARM

  On a big-endian ARM system, "finish" printed the wrong value when
finishing from a function that returned a vector type.  Similarly,
calls to a function also resulted in the wrong value being passed.  I
think both the read- and write-functions here should ignore the
endian-ness.

I tested this using the AdaCore internal test suite; the test case
that caught this is identical to gdb.base/gnu_vector.exp.
---
 gdb/arm-tdep.c | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)
  

On 10/20/23 20:15, Tom Tromey wrote:
> On a big-endian ARM system, "finish" printed the wrong value when
> finishing from a function that returned a vector type.  Similarly,
> calls to a function also resulted in the wrong value being passed.  I
> think both the read- and write-functions here should ignore the
> endian-ness.
> 
> I tested this using the AdaCore internal test suite; the test case
> that caught this is identical to gdb.base/gnu_vector.exp.
> ---
>  gdb/arm-tdep.c | 26 ++++++--------------------
>  1 file changed, 6 insertions(+), 20 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index ab0df0f16a8..493e5b84758 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -9777,29 +9777,22 @@ arm_neon_quad_read (struct gdbarch *gdbarch, readable_regcache *regcache,
>  {
>    char name_buf[4];
>    gdb_byte reg_buf[8];
> -  int offset, double_regnum;
> +  int double_regnum;
>    enum register_status status;
>  
>    xsnprintf (name_buf, sizeof (name_buf), "d%d", regnum << 1);
>    double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
>  					       strlen (name_buf));
>  
> -  /* d0 is always the least significant half of q0.  */
> -  if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
> -    offset = 8;
> -  else
> -    offset = 0;
> -
>    status = regcache->raw_read (double_regnum, reg_buf);
>    if (status != REG_VALID)
>      return status;
> -  memcpy (buf + offset, reg_buf, 8);
> +  memcpy (buf, reg_buf, 8);
>  
> -  offset = 8 - offset;
>    status = regcache->raw_read (double_regnum + 1, reg_buf);
>    if (status != REG_VALID)
>      return status;
> -  memcpy (buf + offset, reg_buf, 8);
> +  memcpy (buf + 8, reg_buf, 8);
>  
>    return REG_VALID;
>  }
> @@ -9874,21 +9867,14 @@ arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache,
>  		     int regnum, const gdb_byte *buf)
>  {
>    char name_buf[4];
> -  int offset, double_regnum;
> +  int double_regnum;
>  
>    xsnprintf (name_buf, sizeof (name_buf), "d%d", regnum << 1);
>    double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
>  					       strlen (name_buf));
>  
> -  /* d0 is always the least significant half of q0.  */
> -  if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
> -    offset = 8;
> -  else
> -    offset = 0;
> -
> -  regcache->raw_write (double_regnum, buf + offset);
> -  offset = 8 - offset;
> -  regcache->raw_write (double_regnum + 1, buf + offset);
> +  regcache->raw_write (double_regnum, buf);
> +  regcache->raw_write (double_regnum + 1, buf + 8);
>  }
>  
>  /* Store the contents of BUF to the MVE pseudo register REGNUM.  */
> 

Unfortunately I'm not sure passing vector types around is exercised by the testsuite, in
particular for ARM.

Do you have an example test where this is failing? I'd like to make sure things still
work for non-BE ARM.
  

>>>>> "Luis" == Luis Machado <luis.machado@arm.com> writes:

>> I tested this using the AdaCore internal test suite; the test case
>> that caught this is identical to gdb.base/gnu_vector.exp.

Luis> Unfortunately I'm not sure passing vector types around is exercised by the testsuite, in
Luis> particular for ARM.

Luis> Do you have an example test where this is failing? I'd like to make sure things still
Luis> work for non-BE ARM.

The test code itself in the AdaCore test suite is pretty much identical
to gdb's gnu_vector.exp.  I don't know if that can run on ARM or not --
but if not, it should be fixed, because it runs fine internally.

Could you try that?

FWIW, AdaCore is also doing little-endian ARM testing.

Tom
  

On 10/27/23 18:42, Tom Tromey wrote:
>>>>>> "Luis" == Luis Machado <luis.machado@arm.com> writes:
> 
>>> I tested this using the AdaCore internal test suite; the test case
>>> that caught this is identical to gdb.base/gnu_vector.exp.
> 
> Luis> Unfortunately I'm not sure passing vector types around is exercised by the testsuite, in
> Luis> particular for ARM.
> 
> Luis> Do you have an example test where this is failing? I'd like to make sure things still
> Luis> work for non-BE ARM.
> 
> The test code itself in the AdaCore test suite is pretty much identical
> to gdb's gnu_vector.exp.  I don't know if that can run on ARM or not --
> but if not, it should be fixed, because it runs fine internally.
> 
> Could you try that?
> 

I don't see any change to the results of that test with or without your patch. This is likely
broken code for BE only, which is not well-exercised.

Do you have some stats on how well the BE testsuite runs for 32-bit ARM?

> FWIW, AdaCore is also doing little-endian ARM testing.
> 
> Tom

Approved-By: Luis Machado <luis.machado@arm.com>
  

Luis> Do you have some stats on how well the BE testsuite runs for 32-bit ARM?

No, I've never run gdb's own test suite on any sort of ARM -- only the
AdaCore internal test suite.  This test suite is more or less a subset
of gdb's test suite, though I think there are probably some Ada tests
that aren't upstream.

With this patch series, BE ARM passes pretty well -- I think as well as
LE ARM.

Tom
  

On 10/30/23 13:44, Tom Tromey wrote:
> Luis> Do you have some stats on how well the BE testsuite runs for 32-bit ARM?
> 
> No, I've never run gdb's own test suite on any sort of ARM -- only the
> AdaCore internal test suite.  This test suite is more or less a subset
> of gdb's test suite, though I think there are probably some Ada tests
> that aren't upstream.
> 
> With this patch series, BE ARM passes pretty well -- I think as well as
> LE ARM.

Great. That's good to know. Thanks for the info.

> 
> Tom