Re: [PATCH 05/24] gdb: change regcache interface to use array_view

[Simon Marchi <simon.marchi@polymtl.ca>]

On 11/13/23 09:00, Andrew Burgess wrote:
>>> diff --git a/gdbsupport/common-regcache.h b/gdbsupport/common-regcache.h
>>> index 6d98ca8c92ed..75d209f7cbc0 100644
>>> --- a/gdbsupport/common-regcache.h
>>> +++ b/gdbsupport/common-regcache.h
>>> @@ -78,11 +78,44 @@ struct reg_buffer_common
>>>       buffer.  */
>>>    virtual register_status get_register_status (int regnum) const = 0;
>>>  
>>> -  /* Supply register REGNUM, whose contents are stored in BUF, to REGCACHE.  */
>>> -  virtual void raw_supply (int regnum, const void *buf) = 0;
>>> +  /* Supply register REGNUM, whose contents are stored in SRC, to this register
>>> +     buffer.  */
>>> +  virtual void raw_supply (int regnum, gdb::array_view<const gdb_byte> src,
>>> +			   float) = 0;
>>
>> The addition of the 'float' argument is pretty unexpected here.  It's
>> not mentioned in the commit message or the function comment, nor is it
>> actually used in any of the function implementations as far as I can
>> tell.  I'm assuming this is resolving some C++ overload problem.
>>
>> Sorry if I've missed something ... I've only looked at the diff so far.
>> I thought I'd reach out before I start playing with the code trying to
>> figure out what's going on.
> 
> As an initial experiment, I tried removing the float parameter after
> applying every patch up to and including this one, and GDB still rebuilt
> fine.
> 
> Then I applied every patch in the series, and removed the float
> parameter, and again GDB built fine.
> 
> Maybe I'm missing something though...
> 
> Thanks,
> Andrew

Haha, good catch, I added them during development as a temporary thing,
to make sure I inspected all call sites, but of course I totally forgot
to remove them.  I added them to differentiate the calls to the raw
pointer and array_view overloads of the functions, I don't recall my
logic exactly.  I remember I was worried about cases like these:

  gdb_byte buf[SOME_ARCH_MAX_REG_SIZE];
  cooked_read (regnum, buf);

We want this to pick up the raw pointer overload, because the size of
register regnum is possibly smaller than SOME_ARCH_MAX_REG_SIZE.  When I
remove the dummy float parameters, clangd tells me that these
cooked_read calls still pick up the raw pointer overload, so it seems
ok.  But I'll see in testing.

For now I did the changes locally, I wull send a v2 once I
fix the regressions identified by Luis.

Thanks,

Simon