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

Luis Machado
2023-11-13T13:12:30Z

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.