a review and questions on avr_scan_prologue()

[Petr Hluzín]

Hello

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:


      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.

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

      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.

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]?

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

[1] http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/avr-tdep.c?cvsroot=src
[2] http://sources.redhat.com/gdb/current/onlinedocs/gdbint/Algorithms.html

-- 
Petr Hluzin