Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (4/4)

[Alan Hayward <Alan dot Hayward at arm dot com>]

> On 8 Jun 2017, at 21:26, Maciej W. Rozycki <macro@imgtec.com> wrote:
> 
> On Wed, 7 Jun 2017, Alan Hayward wrote:
> 
>> I don't have a MIPS machine to test on.
> 
> I could schedule a test run over the coming weekend, however your change 
> applies to EABI support, which I believe is long dead (as in: nobody uses 
> it).  Consequently I don't have a way to test with an EABI target either.

Thanks for the offer, but sounds like the only real option then is to just
ensure the change is safe enough to not break.

> 
>> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
>> index 82f91ba2cd950c5f48f8f408f645ea49e952ef29..52d2ca134f8d14f54c6f4e450c84597c4d6a0e0e 100644
>> --- a/gdb/mips-tdep.c
>> +++ b/gdb/mips-tdep.c
>> @@ -4528,7 +4528,7 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>>   for (argnum = 0; argnum < nargs; argnum++)
>>     {
>>       const gdb_byte *val;
>> -      gdb_byte valbuf[MAX_REGISTER_SIZE];
>> +      gdb_byte valbuf[8];
>>       struct value *arg = args[argnum];
>>       struct type *arg_type = check_typedef (value_type (arg));
>>       int len = TYPE_LENGTH (arg_type);
> 
> If you need to get rid of MAX_REGISTER_SIZE here (what was reason 
> again?), then why not simply use `regsize' instead as the array size?
> 
> AFAICS `valbuf' is only written once, with the size of data requested 
> specified exactly as `regsize'.  The only explanation I have been able to 
> come up with as to why MAX_REGISTER_SIZE has been chosen for `valbuf' 
> allocation is it was made before we could assume variable-length automatic 
> array support.

The reason for getting rid of MAX_REGISTER_SIZE is that Aarch64 SVE will
require increasing the size massively, with potential for future increases.
Hence the large number of patches that are gradually removing MAX_REGISTER_SIZE.

I’ve avoided using variable-length arrays because it has the potential to
break the stack.
However, here we know that we aren’t going to get a value >8, so maybe in
this case a VLA would be ok?

Anyone else have an opinion here?


Alan.