Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Alan Hayward <Alan.Hayward@arm.com>
To: "Maciej W. Rozycki" <macro@imgtec.com>
Cc: Yao Qi <qiyaoltc@gmail.com>,
	"gdb-patches@sourceware.org"	<gdb-patches@sourceware.org>,
	nd <nd@arm.com>
Subject: Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (4/4)
Date: Fri, 09 Jun 2017 10:31:00 -0000	[thread overview]
Message-ID: <D2365DAB-685B-40C6-923F-07504F10DD92@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1706082100310.21750@tp.orcam.me.uk>


> 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.

  reply	other threads:[~2017-06-09 10:31 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 10:12 [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE Alan Hayward
2017-04-04 17:19 ` John Baldwin
2017-04-05 10:27   ` Alan Hayward
2017-04-11 15:37 ` Yao Qi
2017-05-05  8:03   ` [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (1/4) Alan Hayward
2017-05-05 21:44     ` Yao Qi
2017-05-05  8:04   ` [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (4/4) Alan Hayward
     [not found]     ` <434A7317-C19A-4B53-8CB1-C7B4ACEC7D17@arm.com>
2017-06-08 20:27       ` Maciej W. Rozycki
2017-06-09 10:31         ` Alan Hayward [this message]
     [not found]           ` <ebebb23e-e61b-5eff-b666-f2632e554322@redhat.com>
2017-06-09 11:48             ` Maciej W. Rozycki
2017-06-09 12:05               ` Pedro Alves
2017-06-09 13:23                 ` Maciej W. Rozycki
2017-06-09 14:29                   ` Pedro Alves
2017-06-12  9:09                     ` Alan Hayward
2017-06-12 18:11                       ` Pedro Alves
2017-06-12 14:29                     ` Maciej W. Rozycki
2017-05-05  8:04   ` [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (3/4) Alan Hayward
2017-05-05 21:54     ` Yao Qi
2017-05-05  8:04   ` [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (2/4) Alan Hayward
2017-05-05 19:51     ` John Baldwin
2017-05-12  8:53     ` Yao Qi
2017-05-16 11:16       ` Alan Hayward
2017-05-22 12:07         ` Yao Qi
2017-05-22 16:05           ` Alan Hayward
2017-05-22 17:15             ` Pedro Alves
2017-05-23 17:49               ` Alan Hayward
2017-05-23 18:30                 ` Pedro Alves
2017-05-24  9:08                   ` Alan Hayward
2017-05-24  9:29                     ` Pedro Alves
     [not found]                     ` <40597975-9458-e9af-8915-9d303bb1ed98@redhat.com>
2017-05-24 10:20                       ` Alan Hayward
     [not found]                         ` <d32041ef-9df3-316a-68ca-3fe2a2797539@redhat.com>
2017-05-24 19:45                           ` Alan Hayward
2017-05-25 10:46                             ` Pedro Alves
2017-05-25 11:43                               ` Yao Qi
2017-05-25 11:48                                 ` Pedro Alves
2017-05-26  8:54                                   ` Alan Hayward
2017-05-26 10:26                                     ` Pedro Alves
2017-05-26 15:30                                       ` Alan Hayward
2017-05-26 15:49                                         ` Pedro Alves
2017-05-26 16:18                                           ` Alan Hayward
2017-05-26 16:00                                         ` John Baldwin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D2365DAB-685B-40C6-923F-07504F10DD92@arm.com \
    --to=alan.hayward@arm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=macro@imgtec.com \
    --cc=nd@arm.com \
    --cc=qiyaoltc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox