This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (4/4)
On Fri, 9 Jun 2017, Pedro Alves wrote:
> > 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.
Contrariwise `mips_read_fp_register_single' already uses `alloca' for a
similar purpose. Good point about the loop though; moving the declaration
and allocation outside the loop will easily solve the problem however.
Hardcoding things, and especially with literals, causes a maintenance
pain when things change. Bad code elsewhere is not an excuse; besides,
the other places where `8' is hardcoded are not (buffer) limits, but just
handle specific register sizes, which are not going to change, and which
are a completely different matter. So if you find `alloca' unacceptable,
then the limit has to be a macro, and an assertion check is also due (as
already proposed), preferably using `sizeof', so that a future change does
not break it by accident.
NB the MSA ASE expands FPRs to 16 bytes and we'll soon have to accomodate
that; patches have already been posted and are being worked on.
Maciej