Re: [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames

[Luis Machado <luis.machado@arm.com>]

Simon,

On 11/9/23 19:04, Simon Marchi wrote:
> On 11/8/23 14:34, Simon Marchi wrote:
>> Ah, damn, probably because I switched to byte_vector,  which doesn't do
>> the zero-initialization we want to do.  Here's a new patch (that applies
>> on the series directly) that doesn't use byte_vector.
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 1815d78dec4..200e740e013 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -3300,7 +3300,7 @@ aarch64_pseudo_write_1 (gdbarch *gdbarch, frame_info_ptr next_frame,
>>                         int regnum_offset,
>>                         gdb::array_view<const gdb_byte> buf)
>>  {
>> -  unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
>> +  unsigned raw_regnum = AARCH64_V0_REGNUM + regnum_offset;
>>    gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
>>
>>    /* Enough space for a full vector register.
>> @@ -3309,11 +3309,11 @@ aarch64_pseudo_write_1 (gdbarch *gdbarch, frame_info_ptr next_frame,
>>       various 'scalar' pseudo registers to behavior like architectural
>>       writes, register width bytes are written the remainder are set to
>>       zero.  */
>> -  constexpr int raw_reg_size = 16;
>> +  int raw_reg_size = register_size (gdbarch, raw_regnum);
>>    gdb_byte raw_buf[raw_reg_size] {};
>> -  gdb::array_view<gdb_byte> raw_view (raw_buf);
>> +  gdb::array_view<gdb_byte> raw_view (raw_buf, raw_reg_size);
>>    copy (buf, raw_view.slice (0, buf.size ()));
>> -  put_frame_register (next_frame, v_regnum, raw_view);
>> +  put_frame_register (next_frame, raw_regnum, raw_view);
>>  }
>>
>>  /* Given REGNUM, a SME pseudo-register number, store the bytes from DATA to the
>>
>> Simon
> 
> I managed to run a Debian AArch64 image in qemu, with SVE support, so I
> was able to reproduce the failures you mentioned.  In the end, here's a
> version of aarch64_pseudo_write_1 that works for me (written as to
> minimize the number of unnecessary changes, since that seems to
> introduce unexpected bugs...).
> 
> static void
> aarch64_pseudo_write_1 (gdbarch *gdbarch, frame_info_ptr next_frame,
> 			int regnum_offset,
> 			gdb::array_view<const gdb_byte> buf)
> {
>   unsigned raw_regnum = AARCH64_V0_REGNUM + regnum_offset;
> 
>   /* Enough space for a full vector register.  */
>   int raw_reg_size = register_size (gdbarch, raw_regnum);
>   gdb_byte raw_buf[raw_reg_size];
>   gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
> 
>   /* Ensure the register buffer is zero, we want gdb writes of the
>      various 'scalar' pseudo registers to behavior like architectural
>      writes, register width bytes are written the remainder are set to
>      zero.  */
>   memset (raw_buf, 0, register_size (gdbarch, AARCH64_V0_REGNUM));
> 
>   gdb::array_view<gdb_byte> raw_view (raw_buf, raw_reg_size);
>   copy (buf, raw_view.slice (0, buf.size ()));
>   put_frame_register (next_frame, raw_regnum, raw_view);
> }
> 
> Simon

Sorry for the late reply. I was out a couple days last week.

The above seems to make gdb.arch/aarch64-fp.exp happy, but gdb.arch/aarch64-pseudo-unwind.exp is still slightly unhappy:

FAIL: gdb.arch/aarch64-pseudo-unwind.exp: caller, after change: p/x $v8.q.u
FAIL: gdb.arch/aarch64-pseudo-unwind.exp: caller, after change: p/x $s8.u
FAIL: gdb.arch/aarch64-pseudo-unwind.exp: continue to breakpoint: continue to break_here_c
FAIL: gdb.arch/aarch64-pseudo-unwind.exp: p/x value

This was tested on hardware (AWS's Graviton 3). Let me play with it a bit to understand what's up. I also see a
SIGSEGV in the test that shouldn't be there. I'm guessing some sort of raw register corruption, as it leads to
pc == 0x0.