[pushed] gdb: fix internal error in avr_frame_unwind_cache

Message ID 20210405023145.3744658-1-simon.marchi@polymtl.ca
State New
Headers show
Series
  • [pushed] gdb: fix internal error in avr_frame_unwind_cache
Related show

Commit Message

Keith Seitz via Gdb-patches April 5, 2021, 2:31 a.m.
I pushed this as obvious even though it's not necessarily obvious
because I assume that nobody really cares about AVR, and GDB isn't
of much use without that fix.

When trying to do pretty much anything that requires unwinding a frame
on AVR, we get

    /home/simark/src/wt/avr/gdb/trad-frame.h:143: internal-error: LONGEST trad_frame_saved_reg::addr() const: Assertion `m_kind == trad_frame_saved_reg_kind::ADDR' failed.

This is likely coming from the trad-frame refactor in 098caef485a4
("Refactor struct trad_frame_saved_regs").  Here's an example of how to
reproduce it:

In one terminal:

    $ cat test.c
    int foo(int x)
    {
      return x * 7;
    }

    int main() {
        return foo(2);
    }
    $ avr-gcc -gdwarf-4 -mmcu=atmega2560 test.c
    $ /tmp/simavr/bin/simavr --mcu atmega2560 -g a.out
    Loaded 330 .text at address 0x0
    Loaded 0 .data

And in another one:

    $ ./gdb -q -nx --data-directory=data-directory a.out -ex "tar rem :1234" -ex "b foo" -ex c -ex bt
    Reading symbols from a.out...
    Remote debugging using :1234
    0x00000000 in __vectors ()
    Breakpoint 1 at 0x110: file test.c, line 3.
    Note: automatically using hardware breakpoints for read-only addresses.
    Continuing.

    Breakpoint 1, foo (x=2) at test.c:3
    3         return x * 7;
    #0  foo (x=2) at test.c:3
    /home/simark/src/wt/avr/gdb/trad-frame.h:143: internal-error: LONGEST trad_frame_saved_reg::addr() const: Assertion `m_kind == trad_frame_saved_reg_kind::ADDR' failed.

What the AVR code does is:

1. In avr_scan_prologue, in the block that says "First stage of the
   prologue scanning.", look for "push rX" instructions and note that rX
   is saved on the stack.  But instead of putting the actual stack
   address directly, it puts an offset (from the previous frame's sp).
2. Back in avr_frame_unwind_cache, in the block that says "Adjust all
   the saved registers", adjust all these values to be real stack
   addresses.

To check whether a register was assigned an address (and therefore if it
needs adjustment), the code does:

    if (info->saved_regs[i].addr () > 0)

Since commit 098caef485a4, it's invalid to call the `addr` getter of
trad_frame_saved_reg if the register hasn't been assigned an address.
Instead, the code could use the `is_addr` getter to verify if the
register has been assigned an address.  This is what this patch does.

gdb/ChangeLog:

	* avr-tdep.c (avr_frame_unwind_cache): Use
	trad_frame_saved_reg::is_addr.

Change-Id: I5803089160b829400178746c5e3bca0c1cd11c00
---
 gdb/ChangeLog  | 5 +++++
 gdb/avr-tdep.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.30.1

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 21cd0f031c22..281c567a8d7f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2021-04-04  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* avr-tdep.c (avr_frame_unwind_cache): Use
+	trad_frame_saved_reg::is_addr.
+
 2021-04-02  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* objfiles.c (get_objfile_bfd_data): Remove objfile parameter,
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 018b18f3dfd3..5853e692ae46 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -1037,7 +1037,7 @@  avr_frame_unwind_cache (struct frame_info *this_frame,
   /* Adjust all the saved registers so that they contain addresses and not
      offsets.  */
   for (i = 0; i < gdbarch_num_regs (gdbarch) - 1; i++)
-    if (info->saved_regs[i].addr () > 0)
+    if (info->saved_regs[i].is_addr ())
       info->saved_regs[i].set_addr (info->prev_sp
 				    - info->saved_regs[i].addr ());