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

[Pedro Alves <palves@redhat.com>]

On 06/09/2017 12:48 PM, Maciej W. Rozycki wrote:
> On Fri, 9 Jun 2017, Pedro Alves wrote:
> 
>>> 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?
>>
>> VLAs are not standard C++, unfortunately.  Do we know whether all compilers
>> we care about support them?  It doesn't seem worth it to me to rely
>> on compiler extensions when we know we're always going to see a size <=8.
> 
>  Hmm, `alloca' then?  It used to be used here actually, up till commit 
> d9d9c31f3130 ("MAX_REGISTER_RAW_SIZE -> MAX_REGISTER_SIZE"), 
> <https://sourceware.org/ml/gdb-patches/2003-05/msg00127.html>.

IMO, alloca calls are a red flag, which force you to reason about
whether you're dangerously calling it in a loop such as in
this case, because alloca memory is unwound on function exit, not
scope exit.    This is both a concern when an alloca call is
introduced, and later when code is moved around/refactored.  If we know
the hard upper bound, and it's just 8 bytes, I don't see the point
of making it variable.  Alignment and padding for following
variables plus the magic needed to handle variable length frames end up
negating the saving anyway.  Also, the function already hardcodes "8"
in several places.  IMO, here it'd be better to keep it simple
and use a static upper bound.

Thanks,
Pedro Alves