Re: [patch, avr] Fix incorrect argument register for push dummy call

[Luis Machado <lgustavo at codesourcery dot com>]

Hi,

On 02/25/2016 09:34 AM, Pitchumani Sivanupandi wrote:
> There are few failures for avr-gdb in callfuncs.exp. These failures are
> due to the incorrect argument passing in case of push dummy call. The
> last argument register is calculated incorrectly that caused this issue.
>
> Attached patch fixes the calculation so that the push dummy call aligns
> with the code generated by avr-gcc.
>
> No new regressions found when tested with internal simulator.
>
> If ok, could someone commit please?
>
> Regards,
> Pitchumani
>
> gdb/ChangeLog
>
> 2016-02-25  Pitchumani Sivanupandi<pitchumani.s@atmel.com>
>
> 	* avr-tdep.c (AVR_LAST_ARG_REGNUM): Define.
> 	(avr_push_dummy_call): Correct last needed argument register.
>
>
> avr-last-argument-register-fix.patch
>
>
> diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
> index 597cfb4..18ecfe4 100644
> --- a/gdb/avr-tdep.c
> +++ b/gdb/avr-tdep.c
> @@ -111,6 +111,7 @@ enum
>
>     AVR_ARG1_REGNUM = 24,         /* Single byte argument */
>     AVR_ARGN_REGNUM = 25,         /* Multi byte argments */
> +  AVR_LAST_ARG_REGNUM = 8,      /* Last argument register */
>
>     AVR_RET1_REGNUM = 24,         /* Single byte return value */
>     AVR_RETN_REGNUM = 25,         /* Multi byte return value */
> @@ -1298,12 +1299,14 @@ avr_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>         const bfd_byte *contents = value_contents (arg);
>         int len = TYPE_LENGTH (type);
>
> -      /* Calculate the potential last register needed.  */
> -      last_regnum = regnum - (len + (len & 1));
> +      /* Calculate the potential last register needed.
> +         E.g. For length 2, registers regnum and regnum-1 (say 25 and 24)

Is that how the code works? It seems a bit confusing, because we want to 
start in even-aligned registers, so we'd use r25:r24, r23:r22 etc.

So last_regnum always ends up pointing at an odd-numbered register, so 
the code can decrement the odd-numbered register if needed (in case we 
have data of length 1/3/...).

          /* Skip a register for odd length args.  */
          if (len & 1)
            regnum--;

> +         shall be used. So, last needed register will be regnum-1(24).  */
> +      last_regnum = regnum - (len + (len & 1)) + 1;

Isn't this going to be incorrect for, say, data of length 3?

last_regnum = 25 - (3 + (3 & 1)) + 1 -> 25 - (3 + 1) + 1 -> 22

But it should've been 21, because data would occupy registers 25:24 and 
23:22 and then we'd need an even-aligned register to start with?

>         /* If there are registers available, use them.  Once we start putting
>            stuff on the stack, all subsequent args go on stack.  */
> -      if ((si == NULL) && (last_regnum >= 8))
> +      if ((si == NULL) && (last_regnum >= AVR_LAST_ARG_REGNUM))
>           {
>             ULONGEST val;