Re: [PATCH 7/11] Add BFIN_MAX_REGISTER_SIZE

[Alan Hayward <Alan.Hayward@arm.com>]

> On 5 Apr 2017, at 16:03, Pedro Alves <palves@redhat.com> wrote:
> 
> On 04/04/2017 11:14 AM, Alan Hayward wrote:
>> Max size set to 32bits, which I determined using regformats/reg-bfin.dat
> 
> Makes me wonder sth (and in general, not for this patch in particular):
> 
> Is it possible that any of these code paths that hardcode an arch specific
> max register size end up seeing a larger register size because the reported
> xml target description includes such a larger register?
> 
> E.g., say arch A normally only has 32-bit registers, for as much GDB knows.
> And then some stub for some variant of A includes a register
> in the description like:
> 
>  <reg name="foo" bitsize="64" type="uint64"/>
> 
> It kinds of sounds like the max register size is capped by what target
> descriptions can describe for that architecture, not exactly by the size
> of the registers that GDB considers "core" registers.  That may
> already have been taken into account and it may well be that the paths
> that use the FOO_MAX_REGISTER_SIZE macros only ever work with registers
> that GDB does know about (haven't checked carefully), rendering the concern
> moot, but I wanted to put the thought out there anyway.
> 
> Thanks,
> Pedro Alves
> 

There is some existing code in regcache.c that checks that no register in the
target descriptor is greater than MAX_REGISTER_SIZE.
Obviously, this code will vanish when MAX_REGISTER_SIZE disappears.

If people think that this is an important check to have, then maybe there needs
to be an additional patch set. For each target with a FOO_MAX_REGISTER_SIZE,
in the init code for that target add:
  gdb_assert (FOO_MAX_REGISTER_SIZE >= max_register_size (gdbarch));
?

(To keep this simple I’d do it after adding these patches which add
 FOO_MAX_REGISTER_SIZE).


Alan.