Re: a review and questions on avr_scan_prologue()

[Tristan Gingold <gingold@adacore.com>]

On Feb 14, 2010, at 12:56 AM, Petr Hluzín wrote:

> Hello

Hi,

> I took a look at avr-tdep.c [1] and I found some places which are
> either bug or are not clear to me. Here it goes:

Thanks for doing reviews!

>      else if (len >= sizeof (img) - 2
> 	       && memcmp (img + 2, prologue, sizeof (img) - 2) == 0)
> 	{
>          info->prologue_type = AVR_PROLOGUE_SIG;
>          vpc += sizeof (img) - 2;
>          info->saved_regs[AVR_SREG_REGNUM].addr = 3;
>          info->saved_regs[0].addr = 2;
>          info->saved_regs[1].addr = 1;
> -          info->size += 3;
> +          info->size += 2;
> 	}
> 
> Since the "img + 2" skips "push r1" I believe the scan should record
> smaller size.

Yes, you're right.  I will fix that.

>  if (vpc >= AVR_MAX_PROLOGUE_SIZE)
>     fprintf_unfiltered (gdb_stderr,
>                         _("Hit end of prologue while scanning pushes\n"));
> 
> This condition is never true due to a way `len' is calculated and
> `vpc' always being less than `len'. (This is not a bug but per se but
> the author might expected something what is not true.)

I will change that to an assert.

>      else if (insn == 0x920f)  /* push r0 */
>        {
>          info->size += 1;
>          vpc += 2;
>        }
> 
> The condition is never true because of the preceding "Scan pushes
> (saved registers)" loop's exit condition.

I don't think so.  You can have:
  rcall .+0
  push r0


> Also:
> The avr_scan_prologue()'s recognizes several well-known prologues. Is
> there a reason why it does not use the general prologue analysis
> algorithm as described in the documentation [2]?

Historical reason: it was written before the general prologue analysis.
Then, when AdaCore did its AVR port, I fixed all issues I found, but didn't rewrite it from scratch.

> I think universal prologue analysis is quite easy with AVR arch. The
> code might be shorter (though less clear).
> I might try to write the code if you are interested.
> (The current prologue scan code chokes on hand-crafted assembly.)

Feel free to work on that.  Improvements are always welcome!

Tristan.