[patch] [v2] Enable dwarf unwind for AVR target

[patch] [v2] Enable dwarf unwind for AVR target

[Sivanupandi, Pitchumani]

[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]

Previous patch and discussion is here:
https://sourceware.org/ml/gdb-patches/2016-01/msg00027.html

Test case: step over on function call statements (e.g. break.exp)

Need for dwarf unwind:
Current AVR frame unwind analyzes only the prologue and stack unwind becomes
unreliable. CFA info from dwarf debug information can be used to unwind the
stack pointer and PC reliably.

Attached patch (updated, v2) enables the dwarf unwinder for avr target.

Fix:
Dwarf debug info generated by avr-gcc denotes the return address by register
36 which is not an actual register.
e.g. .debug_frame
(--snip--)
00000000 00000010 ffffffff CIE
  Version:               1
  Augmentation:          ""
  Code alignment factor: 2
  Data alignment factor: -1
  Return address column: 36

  DW_CFA_def_cfa: r32 ofs 3
  DW_CFA_offset: r36 at cfa-2
(--snip--)

The fix is to add a pseudo register (36 - AVR_DWARF2_PC_REGNUM/LR) to gdb to
map return address register. Register name is "LR" (link register). When dwarf
frame unwind asks for PC, target function will read return address value from
AVR_DWARF2_PC_REGNUM's CFA address.

Target function avr_dwarf2_prev_register implementation is similar to existing
avr_frame_prev_register function.

Note:
* AVR_DWARF2_PC_REGNUM is meant only to unwind PC. Also we can't expect stack
at all times (e.g. startup code) to read/write into that pseudo register. So,
the pseudo register read will return that register unavailable and write will
not do anything.
* Added extern function dwarf2_frame_addr to dwarf2-frame.c to find the frame
address for argument register from dwarf frame cache.
* Dwarf2 address size set to 4 (Ref: DWARF2_ADDR_SIZE from avr-gcc).

Ran GDB regression tests with Atmel internal simulator (atmega2560). No new
regressions found.

Is this patch OK?

Regards,
Pitchumani

gdb/ChangeLog
* avr-tdep.c: Include dwarf2-frame.h
  (enum): Add new pseudo register AVR_DWARF2_PC_REGNUM (36).
  Update number of pseudo registers (AVR_NUM_PSEUDO_REGS).
  (avr_register_name): Add LR as register name for new pseudo register.
  (avr_register_type): return pc type for new register.
  (avr_pseudo_register_read): return that register unavailable for new
  pseudo register.
  (avr_pseudo_register_write): do nothing as new pseudo register is read-only.
  (avr_dwarf2_prev_register): New function to unwind prev register.
  (avr_dwarf_reg_to_regnum): Allow all valid pseudo registers.
  (avr_dwarf2_frame_init_reg): Initialize pseudo registers handler.
  (avr_gdbarch_init): Set dwarf2 address size.
  Set register state init function.
  Add dwarf2 unwinders to the unwinders list.
* dwarf2-frame.c (dwarf2_frame_addr): New function that returns CFA for
  argument register.
* dwarf2-frame.h: Add prototype for new function.


[-- Attachment #2: avr-enable-dwarf-unwind-v2.patch --]
[-- Type: application/octet-stream, Size: 6412 bytes --]

diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index ccc8b45..faa6a77 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -29,6 +29,7 @@
 #include "trad-frame.h"
 #include "gdbcmd.h"
 #include "gdbcore.h"
+#include "dwarf2-frame.h"
 #include "gdbtypes.h"
 #include "inferior.h"
 #include "symfile.h"
@@ -97,7 +98,8 @@ enum
 
   /* Pseudo registers.  */
   AVR_PSEUDO_PC_REGNUM = 35,
-  AVR_NUM_PSEUDO_REGS = 1,
+  AVR_DWARF2_PC_REGNUM = 36 /*LR*/,
+  AVR_NUM_PSEUDO_REGS = 2,
 
   AVR_PC_REG_INDEX = 35,	/* index into array of registers */
 
@@ -211,7 +213,7 @@ avr_register_name (struct gdbarch *gdbarch, int regnum)
     "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
     "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
     "SREG", "SP", "PC2",
-    "pc"
+    "pc", "LR"
   };
   if (regnum < 0)
     return NULL;
@@ -230,6 +232,8 @@ avr_register_type (struct gdbarch *gdbarch, int reg_nr)
     return builtin_type (gdbarch)->builtin_uint32;
   if (reg_nr == AVR_PSEUDO_PC_REGNUM)
     return gdbarch_tdep (gdbarch)->pc_type;
+  if (reg_nr == AVR_DWARF2_PC_REGNUM)
+    return gdbarch_tdep (gdbarch)->pc_type;
   if (reg_nr == AVR_SP_REGNUM)
     return builtin_type (gdbarch)->builtin_data_ptr;
   return builtin_type (gdbarch)->builtin_uint8;
@@ -395,6 +399,10 @@ avr_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
       val >>= 1;
       store_unsigned_integer (buf, 4, gdbarch_byte_order (gdbarch), val);
       return status;
+    case AVR_DWARF2_PC_REGNUM:
+      /* return that register is unavailable as we can't expect stack at all
+         times to read CFA, e.g. startup code.  */
+      return REG_UNAVAILABLE;
     default:
       internal_error (__FILE__, __LINE__, _("invalid regnum"));
     }
@@ -413,6 +421,10 @@ avr_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
       val <<= 1;
       regcache_raw_write_unsigned (regcache, AVR_PC_REGNUM, val);
       break;
+    case AVR_DWARF2_PC_REGNUM:
+      /* Do nothing. It is read-only; also we can't expect stack at all
+         times to write, e.g. startup code.  */
+      break;
     default:
       internal_error (__FILE__, __LINE__, _("invalid regnum"));
     }
@@ -1355,6 +1367,38 @@ avr_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   return sp + call_length;
 }
 
+static struct value *
+avr_dwarf2_prev_register (struct frame_info *this_frame, void **this_cache,
+			  int regnum)
+{
+  struct gdbarch * gdbarch = get_frame_arch (this_frame);
+  struct gdbarch_tdep * tdep = gdbarch_tdep (gdbarch);
+  CORE_ADDR addr;
+  ULONGEST pc;
+  int i;
+  gdb_byte buf[3];
+
+  switch (regnum)
+    {
+    case AVR_PC_REGNUM:
+    case AVR_PSEUDO_PC_REGNUM:
+      addr = dwarf2_frame_addr (this_cache, AVR_DWARF2_PC_REGNUM);
+      read_memory (addr, buf, tdep->call_length);
+      pc = 0;
+      for (i = 0; i < tdep->call_length; i++)
+        pc = (pc << 8) | buf[i];
+      /* Change it to byte address. */
+      if (regnum == AVR_PC_REGNUM)
+        pc <<= 1;
+      return frame_unwind_got_constant (this_frame, regnum, pc);
+//    case AVR_PSEUDO_PC_REGNUM:
+//      internal_error (__FILE__, __LINE__, "I'm here");
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Unexpected register %d"), regnum);
+    }
+}
+
 /* Unfortunately dwarf2 register for SP is 32.  */
 
 static int
@@ -1362,11 +1406,38 @@ avr_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
 {
   if (reg >= 0 && reg < 32)
     return reg;
-  if (reg == 32)
-    return AVR_SP_REGNUM;
+  switch (reg)
+  {
+    case 32:
+      return AVR_SP_REGNUM;
+    case 34: // AVR_PC_REGNUM
+    case 35: // AVR_PSEUDO_PC_REGNUM
+    case 36: // AVR_DWARF2_PC_REGNUM
+      return reg;
+  }
   return -1;
 }
 
+static void
+avr_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+                           struct dwarf2_frame_state_reg *reg,
+                           struct frame_info *this_frame)
+{
+  switch (regnum)
+  {
+    /* Handle PC and PSEUDO PC registers by target function.  */
+    case AVR_PC_REGNUM:
+    case AVR_PSEUDO_PC_REGNUM:
+      reg->how = DWARF2_FRAME_REG_FN;
+      reg->loc.fn = avr_dwarf2_prev_register;
+      break;
+    /*  Use CFA to handle SP register.  */
+    case AVR_SP_REGNUM:
+      reg->how = DWARF2_FRAME_REG_CFA;
+      break;
+  }
+}
+
 /* Implementation of `address_class_type_flags' gdbarch method.
 
    This method maps DW_AT_address_class attributes to a
@@ -1478,6 +1549,9 @@ avr_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
   set_gdbarch_addr_bit (gdbarch, 32);
 
+  /* Dwarf address is 4 bytes. Ref: DWARF2_ADDR_SIZE in avr-gcc.  */
+  set_gdbarch_dwarf2_addr_size (gdbarch, 4);
+
   set_gdbarch_float_bit (gdbarch, 4 * TARGET_CHAR_BIT);
   set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
   set_gdbarch_long_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
@@ -1517,6 +1591,9 @@ avr_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   set_gdbarch_breakpoint_from_pc (gdbarch, avr_breakpoint_from_pc);
 
+  dwarf2_frame_set_init_reg (gdbarch, avr_dwarf2_frame_init_reg);
+  dwarf2_append_unwinders (gdbarch);
+
   frame_unwind_append_unwinder (gdbarch, &avr_frame_unwind);
   frame_base_set_default (gdbarch, &avr_frame_base);
 
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index a640c26..54bab17 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -1524,6 +1524,21 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
 
   return get_frame_base (this_frame);
 }
+
+/* Compute the frame address.  */
+CORE_ADDR
+dwarf2_frame_addr (void ** this_cache, int regnum)
+{
+  struct dwarf2_frame_cache *cache = *this_cache;
+  CORE_ADDR addr;
+
+  gdb_assert (cache != NULL);
+  gdb_assert (cache->reg[regnum].how == DWARF2_FRAME_REG_SAVED_OFFSET);
+
+  addr = cache->cfa + cache->reg[regnum].loc.offset;
+
+  return addr;
+}
 \f
 const struct objfile_data *dwarf2_frame_objfile_data;
 
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index b739744..cde88e5 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -119,6 +119,7 @@ extern const struct frame_base *
 /* Compute the DWARF CFA for a frame.  */
 
 CORE_ADDR dwarf2_frame_cfa (struct frame_info *this_frame);
+CORE_ADDR dwarf2_frame_addr (void ** this_cache, int regnum);
 
 /* Find the CFA information for PC.
 

Re: [patch] [v2] Enable dwarf unwind for AVR target

[Kevin Buettner]

On Fri, 8 Jan 2016 08:52:33 +0000
"Sivanupandi, Pitchumani" <Pitchumani.Sivanupandi@atmel.com> wrote:

> Dwarf debug info generated by avr-gcc denotes the return address by register
> 36 which is not an actual register.
> e.g. .debug_frame
> (--snip--)
> 00000000 00000010 ffffffff CIE
>   Version:               1
>   Augmentation:          ""
>   Code alignment factor: 2
>   Data alignment factor: -1
>   Return address column: 36
> 
>   DW_CFA_def_cfa: r32 ofs 3
>   DW_CFA_offset: r36 at cfa-2
> (--snip--)
> 
> The fix is to add a pseudo register (36 - AVR_DWARF2_PC_REGNUM/LR) to gdb to
> map return address register. Register name is "LR" (link register). When dwarf
> frame unwind asks for PC, target function will read return address value from
> AVR_DWARF2_PC_REGNUM's CFA address.

The usual way to handle this problem is to define a dwarf2_reg_to_regnum
method which maps the index of DWARF's return address column to GDB's PC 
register.  So, for the AVR, you'd map 36 to 35.

If you do this, I think you can dispense with all of the stuff
which adds and manipulates the pseudo-register.

Here's an example (from rx-tdep.c) where this is done:

static int
rx_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
{
  if (0 <= reg && reg <= 15)
    return reg;
  else if (reg == 16)
    return RX_PSW_REGNUM;
  else if (reg == 17)
    return RX_PC_REGNUM;
  else
    return -1;
}

Then, in rx_gdbarch_init, this function is registered as follows:

  set_gdbarch_dwarf2_reg_to_regnum (gdbarch, rx_dwarf_reg_to_regnum);

Hope this helps...

Kevin

Re: [patch] [v2] Enable dwarf unwind for AVR target

[Pitchumani Sivanupandi]

On Wed, Feb 03, 2016 at 09:45:34AM -0700, Kevin Buettner wrote:
> On Fri, 8 Jan 2016 08:52:33 +0000
> "Sivanupandi, Pitchumani" <Pitchumani.Sivanupandi@atmel.com> wrote:
> 
> > Dwarf debug info generated by avr-gcc denotes the return address by register
> > 36 which is not an actual register.
> > e.g. .debug_frame
> > (--snip--)
> > 00000000 00000010 ffffffff CIE
> >   Version:               1
> >   Augmentation:          ""
> >   Code alignment factor: 2
> >   Data alignment factor: -1
> >   Return address column: 36
> > 
> >   DW_CFA_def_cfa: r32 ofs 3
> >   DW_CFA_offset: r36 at cfa-2
> > (--snip--)
> > 
> > The fix is to add a pseudo register (36 - AVR_DWARF2_PC_REGNUM/LR) to gdb to
> > map return address register. Register name is "LR" (link register). When dwarf
> > frame unwind asks for PC, target function will read return address value from
> > AVR_DWARF2_PC_REGNUM's CFA address.
> 
> The usual way to handle this problem is to define a dwarf2_reg_to_regnum
> method which maps the index of DWARF's return address column to GDB's PC 
> register.  So, for the AVR, you'd map 36 to 35.
> 
> If you do this, I think you can dispense with all of the stuff
> which adds and manipulates the pseudo-register.
> 
> Here's an example (from rx-tdep.c) where this is done:
> 
> static int
> rx_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> {
>   if (0 <= reg && reg <= 15)
>     return reg;
>   else if (reg == 16)
>     return RX_PSW_REGNUM;
>   else if (reg == 17)
>     return RX_PC_REGNUM;
>   else
>     return -1;
> }
> 
> Then, in rx_gdbarch_init, this function is registered as follows:
> 
>   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, rx_dwarf_reg_to_regnum);
> 
> Hope this helps...

Sorry for delayed response.

Thanks for the comments. It is working, however I found few regressions
when running the gdb tests. I'll check those and post the updated patch.

Regards,
Pitchumani

RE: [patch] [v2] Enable dwarf unwind for AVR target

[Sivanupandi, Pitchumani]

Ping!

> -----Original Message-----
> From: Sivanupandi, Pitchumani
> Sent: 08 January 2016 14:23
> To: 'brobecker@adacore.com' <brobecker@adacore.com>; 'Pedro Alves'
> <palves@redhat.com>
> Cc: 'troth@openavr.org' <troth@openavr.org>; 'gdb-patches@sourceware.org'
> <gdb-patches@sourceware.org>
> Subject: [patch] [v2] Enable dwarf unwind for AVR target
> 
> Previous patch and discussion is here:
> https://sourceware.org/ml/gdb-patches/2016-01/msg00027.html
> 
> Test case: step over on function call statements (e.g. break.exp)
> 
> Need for dwarf unwind:
> Current AVR frame unwind analyzes only the prologue and stack unwind becomes
> unreliable. CFA info from dwarf debug information can be used to unwind the
> stack pointer and PC reliably.
> 
> Attached patch (updated, v2) enables the dwarf unwinder for avr target.
> 
> Fix:
> Dwarf debug info generated by avr-gcc denotes the return address by register
> 36 which is not an actual register.
> e.g. .debug_frame
> (--snip--)
> 00000000 00000010 ffffffff CIE
>   Version:               1
>   Augmentation:          ""
>   Code alignment factor: 2
>   Data alignment factor: -1
>   Return address column: 36
> 
>   DW_CFA_def_cfa: r32 ofs 3
>   DW_CFA_offset: r36 at cfa-2
> (--snip--)
> 
> The fix is to add a pseudo register (36 - AVR_DWARF2_PC_REGNUM/LR) to gdb to
> map return address register. Register name is "LR" (link register). When
> dwarf frame unwind asks for PC, target function will read return address
> value from AVR_DWARF2_PC_REGNUM's CFA address.
> 
> Target function avr_dwarf2_prev_register implementation is similar to
> existing avr_frame_prev_register function.
> 
> Note:
> * AVR_DWARF2_PC_REGNUM is meant only to unwind PC. Also we can't expect
> stack at all times (e.g. startup code) to read/write into that pseudo
> register. So, the pseudo register read will return that register unavailable
> and write will not do anything.
> * Added extern function dwarf2_frame_addr to dwarf2-frame.c to find the
> frame address for argument register from dwarf frame cache.
> * Dwarf2 address size set to 4 (Ref: DWARF2_ADDR_SIZE from avr-gcc).
> 
> Ran GDB regression tests with Atmel internal simulator (atmega2560). No new
> regressions found.
> 
> Is this patch OK?
> 
> Regards,
> Pitchumani
> 
> gdb/ChangeLog
> * avr-tdep.c: Include dwarf2-frame.h
>   (enum): Add new pseudo register AVR_DWARF2_PC_REGNUM (36).
>   Update number of pseudo registers (AVR_NUM_PSEUDO_REGS).
>   (avr_register_name): Add LR as register name for new pseudo register.
>   (avr_register_type): return pc type for new register.
>   (avr_pseudo_register_read): return that register unavailable for new
>   pseudo register.
>   (avr_pseudo_register_write): do nothing as new pseudo register is read-
> only.
>   (avr_dwarf2_prev_register): New function to unwind prev register.
>   (avr_dwarf_reg_to_regnum): Allow all valid pseudo registers.
>   (avr_dwarf2_frame_init_reg): Initialize pseudo registers handler.
>   (avr_gdbarch_init): Set dwarf2 address size.
>   Set register state init function.
>   Add dwarf2 unwinders to the unwinders list.
> * dwarf2-frame.c (dwarf2_frame_addr): New function that returns CFA for
>   argument register.
> * dwarf2-frame.h: Add prototype for new function.