[RFC PATCH v4 02/15] GDB: Use gdb::array_view for buffers used in register reading and unwinding

[Thiago Jung Bauermann]

Hello Maciej,

"Maciej W. Rozycki" <macro@orcam.me.uk> writes:

> On Fri, 10 Jan 2025, Thiago Jung Bauermann wrote:
>
>> >> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
>> >> index c00efbd02ad0..24970c7637ad 100644
>> >> --- a/gdb/mips-tdep.c
>> >> +++ b/gdb/mips-tdep.c
>> >> @@ -951,14 +951,17 @@ mips_register_to_value (const frame_info_ptr &frame, int
>> >> regnum,
>> >>
>> >>    if (mips_convert_register_float_case_p (gdbarch, regnum, type))
>> >>      {
>> >> -      get_frame_register (frame, regnum + 0, to + 4);
>> >> -      get_frame_register (frame, regnum + 1, to + 0);
>> >> +      gdb::array_view<gdb_byte> first_half = gdb::make_array_view (to, 4);
>> >> +      gdb::array_view<gdb_byte> second_half = gdb::make_array_view (to + 4, 4);
>> >>
>> >> -      if (!get_frame_register_bytes (next_frame, regnum + 0, 0, { to + 4, 4 },
>> >> +      get_frame_register (frame, regnum + 0, second_half);
>> >> +      get_frame_register (frame, regnum + 1, first_half);
>> >> +
>> >> +      if (!get_frame_register_bytes (next_frame, regnum + 0, 0, second_half,
>> >>  				     optimizedp, unavailablep))
>> >>  	return 0;
>> >>
>> >> -      if (!get_frame_register_bytes (next_frame, regnum + 1, 0, { to + 0, 4 },
>> >> +      if (!get_frame_register_bytes (next_frame, regnum + 1, 0, first_half,
>> >>  				     optimizedp, unavailablep))
>> >>  	return 0;
>> >>        *optimizedp = *unavailablep = 0;
>> >> @@ -6257,8 +6260,10 @@ mips_read_fp_register_single (const frame_info_ptr &frame, int
>> >> regno,
>> >>    struct gdbarch *gdbarch = get_frame_arch (frame);
>> >>    int raw_size = register_size (gdbarch, regno);
>> >>    gdb_byte *raw_buffer = (gdb_byte *) alloca (raw_size);
>> >> +  gdb::array_view<gdb_byte> raw_view = gdb::make_array_view (raw_buffer,
>> >> +							     raw_size);
>> >
>> > I noticed one thing after sending the previous email on this patch.
>> > For simpler code, can we use here
>> >
>> >   gdb::byte_vector raw_buffer (raw_size);
>> >
>> > and eliminate raw_view?
>>
>> That's a good idea. I took this approach in order to make a minimally
>> invasive patch, given that I can't test on a MIPS system.
>
>  What needs testing?  Perhaps I can push it through (although I find MIPS
> testing unreliable in my lab recently: a lot of intermittent failures and
> I don't yet know if it's a target regression or an issue specific to
> hardware, which I had to replace with a different one not so long ago due
> to a catastrophic failure).

Thank you! I posted a v2 of this patch with more changes in mips-tdep.c:

https://inbox.sourceware.org/gdb-patches/20250110164430.3376697-3-thiago.bauermann@linaro.org/

I actually committed it already: commit 7fcdec025c05 ("GDB: Use
gdb::array_view for buffers used in register reading and unwinding") so
if you think it's warranted, it could be worth checking whether MIPS
testsuite results for current GDB trunk aren't worse than before the
commit. The changes in mips-tdep.c are only about floating point
registers, so theoretically they should affect only tests related to
float and double.

--
Thiago