Re: [PATCH] avr-tdep.c (avr_scan_prologue): minor fix in prologue scanning

[Joel Brobecker <brobecker at adacore dot com>]

Hello Cristiano,

On Sat, Aug 22, 2015 at 12:47:51AM +0200, Cristiano wrote:
> I use GDB from Eclipse with Avarice to debug AVR microcontrollers.
> While single stepping (stepi) through the instructions of a normal
> function prologue, stepping over the sbiw r28, 0x0f instruction, which
> is near the end of the prologue, causes the following change in the
> output of backtrace:
> 
> Before:
> #0 0x000042b0 in HID_Device_USBTask (HIDInterfaceInfo=0x80b8b8) at
> ../../LUFA/Drivers/USB/Class/Device/HIDClassDevice.c:157
> #1 0x000002e0 in main () at HIDRadio.c:83
> 
> After:
> #0 0x000042b2 in HID_Device_USBTask (HIDInterfaceInfo=0x8000ff) at
> ../../LUFA/Drivers/USB/Class/Device/HIDClassDevice.c:157
> #1 0x00017170 in ?? ()
> 
> Note that the frame #1 is inconsistent.
> 
> If you now return from frame #0 with the command finish, the target does
> not stop at the return address at main. Instead, the target starts
> running and it's necessary to stop it.
> 
> This is not a big issue and I'm not an expert of the gdb internals but
> the fix seems easy enough.
> 
> 
> 2015-08-20 Cristiano De Alti <cristiano_dealti@hotmail.com>
> 
> * avr-tdep.c (avr_scan_prologue): minor fix in prologue scanning.

I really apologize for letting this patch stall for 3 months without
a reply. We appreciate patches like these! The unfortunate reality
is just that I don't think anyone really maintains that port anymore...

I would accept your patch based on your AVR-specific expertise, but
it looks like all leading spaces got slowed by either your mailer
or your email server.

If you were to rebase your patch, and git-sendemail it to me (Cc:
this list), I can review it and then apply it if it looks OK to me.

Thanks!

> 
> 
> diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
> index 3b99bd2..32a6540 100644
> --- a/gdb/avr-tdep.c
> +++ b/gdb/avr-tdep.c
> @@ -832,25 +832,26 @@ avr_scan_prologue (struct gdbarch *gdbarch,
> CORE_ADDR pc_beg, CORE_ADDR pc_end,
> or signal handler functions, which is why we set the prologue type
> when we saw the beginning of the prologue previously. */
> 
> - if (vpc + sizeof (img_sig) < len
> + if (vpc + sizeof (img_sig) <= len
> && memcmp (prologue + vpc, img_sig, sizeof (img_sig)) == 0)
> {
> + info->size += locals_size;
> vpc += sizeof (img_sig);
> }
> - else if (vpc + sizeof (img_int) < len
> + else if (vpc + sizeof (img_int) <= len
> && memcmp (prologue + vpc, img_int, sizeof (img_int)) == 0)
> {
> + info->size += locals_size;
> vpc += sizeof (img_int);
> }
> - if (vpc + sizeof (img) < len
> + if (vpc + sizeof (img) <= len
> && memcmp (prologue + vpc, img, sizeof (img)) == 0)
> {
> + info->size += locals_size;
> info->prologue_type = AVR_PROLOGUE_NORMAL;
> vpc += sizeof (img);
> }
> 
> - info->size += locals_size;
> -
> /* Fall through. */
> }

-- 
Joel