[RFA] MIPS DWARF2 CFI support

[RFA] MIPS DWARF2 CFI support

[Martin M. Hunt]

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

This is really kevinb's work, with some minor tweaks.  I've tested this
on over 50 multilib combinations and it looks goood.

2004-05-26  Kevin Buettner: 

	* dwarf2-frame.c (execute_cfa_program): Fix typo in which the
	alignment was being added to the offset instead of multiplied.

	* dwarf2-frame.c (struct comp_unit): Add new field ``signed_addr_p''.
	(encoding_for_size): Add new parameter ``signed_addr_p''.  Adjust
	all callers.  Add code for handling signed encodings.
	(dwarf2_build_frame_info): Initialize ``unit.signed_addr_p''.

	* dwarf2-frame.c (dwarf2_build_frame_info): Set unit.addr_size.

	* config/mips/tm-mips.h (SP_REGNUM): Delete define.
	* mips-tdep.h (MIPS_SP_REGNUM): Define.
	* mips-tdep.c (mips_gdbarch_init): Set SP_REGNUM via call
	to set_gdbarch_sp_regnum().  Use cooked register number.
	(SP_REGNUM): Replace all occurrences with MIPS_SP_REGNUM.

	* mips-tdep.c (mips_gdbarch_init): Set PC_REGNUM (via
	call to set_gdbarch_pc_regnum()) to cooked register number.

	* mips-tdep.c (dwarf2-frame.h): Include.
	(mips_gdbarch_init): Enable DWARF2 CFI support.


-- 
Martin M. Hunt <hunt@redhat.com>
Red Hat Inc.

[-- Attachment #2: cfi.patch --]
[-- Type: text/x-patch, Size: 9328 bytes --]

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.34
diff -u -u -r1.34 dwarf2-frame.c
--- dwarf2-frame.c	1 May 2004 10:55:19 -0000	1.34
+++ dwarf2-frame.c	8 Jun 2004 03:48:13 -0000
@@ -426,7 +426,7 @@
 	    case DW_CFA_offset_extended_sf:
 	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &reg);
 	      insn_ptr = read_sleb128 (insn_ptr, insn_end, &offset);
-	      offset += fs->data_align;
+	      offset *= fs->data_align;
 	      dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
 	      fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_OFFSET;
 	      fs->regs.reg[reg].loc.offset = offset;
@@ -893,6 +893,9 @@
   /* Address size for this unit - from unit header.  */
   unsigned char addr_size;
 
+  /* Are addresses signed?  */
+  unsigned char signed_addr_p;
+
   /* Pointer to the .debug_frame section loaded into memory.  */
   char *dwarf_frame_buffer;
 
@@ -1021,16 +1024,25 @@
    should be dereferenced.  */
 
 static unsigned char
-encoding_for_size (unsigned int size)
+encoding_for_size (unsigned int size, int signed_addr_p)
 {
   switch (size)
     {
     case 2:
-      return DW_EH_PE_udata2;
+      if (signed_addr_p)
+	return DW_EH_PE_sdata2;
+      else
+	return DW_EH_PE_udata2;
     case 4:
-      return DW_EH_PE_udata4;
+      if (signed_addr_p)
+	return DW_EH_PE_sdata4;
+      else
+	return DW_EH_PE_udata4;
     case 8:
-      return DW_EH_PE_udata8;
+      if (signed_addr_p)
+	return DW_EH_PE_sdata8;
+      else
+	return DW_EH_PE_udata8;
     default:
       internal_error (__FILE__, __LINE__, "Unsupported address size");
     }
@@ -1110,7 +1122,7 @@
     }
 
   if ((encoding & 0x0f) == 0x00)
-    encoding |= encoding_for_size (ptr_len);
+    encoding |= encoding_for_size (ptr_len, unit->signed_addr_p);
 
   switch (encoding & 0x0f)
     {
@@ -1285,7 +1297,7 @@
       /* The encoding for FDE's in a normal .debug_frame section
          depends on the target address size as specified in the
          Compilation Unit Header.  */
-      cie->encoding = encoding_for_size (unit->addr_size);
+      cie->encoding = encoding_for_size (unit->addr_size, unit->signed_addr_p);
 
       /* Check version number.  */
       if (read_1_byte (unit->abfd, buf) != DW_CIE_VERSION)
@@ -1548,7 +1560,8 @@
   /* Build a minimal decoding of the DWARF2 compilation unit.  */
   unit.abfd = objfile->obfd;
   unit.objfile = objfile;
-  unit.addr_size = objfile->obfd->arch_info->bits_per_address / 8;
+  unit.addr_size = TYPE_LENGTH (builtin_type_void_data_ptr);
+  unit.signed_addr_p = bfd_get_sign_extend_vma (unit.abfd);
   unit.dbase = 0;
   unit.tbase = 0;
 
Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.295
diff -u -u -r1.295 mips-tdep.c
--- mips-tdep.c	7 Jun 2004 02:02:52 -0000	1.295
+++ mips-tdep.c	8 Jun 2004 03:48:15 -0000
@@ -54,6 +54,7 @@
 #include "frame-base.h"
 #include "trad-frame.h"
 #include "infcall.h"
+#include "dwarf2-frame.h"
 
 static const struct objfile_data *mips_pdr_data;
 
@@ -757,7 +758,7 @@
 static CORE_ADDR
 mips_read_sp (void)
 {
-  return read_signed_register (SP_REGNUM);
+  return read_signed_register (MIPS_SP_REGNUM);
 }
 
 /* Should the upper word of 64-bit addresses be zeroed? */
@@ -849,7 +850,7 @@
 static struct frame_id
 mips_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
 {
-  return frame_id_build (frame_unwind_register_signed (next_frame, NUM_REGS + SP_REGNUM),
+  return frame_id_build (frame_unwind_register_signed (next_frame, NUM_REGS + MIPS_SP_REGNUM),
 			 frame_pc_unwind (next_frame));
 }
 
@@ -880,7 +881,7 @@
     {
       /* If function is frameless, then we need to do it the hard way.  I
          strongly suspect that frameless always means prologueless... */
-      if (PROC_FRAME_REG (proc_desc) == SP_REGNUM
+      if (PROC_FRAME_REG (proc_desc) == MIPS_SP_REGNUM
 	  && PROC_FRAME_OFFSET (proc_desc) == 0)
 	return 0;
     }
@@ -1683,7 +1684,7 @@
   }
 
   /* SP_REGNUM, contains the value and not the address.  */
-  trad_frame_set_value (cache->saved_regs, NUM_REGS + SP_REGNUM, cache->base);
+  trad_frame_set_value (cache->saved_regs, NUM_REGS + MIPS_SP_REGNUM, cache->base);
 
   return (*this_cache);
 }
@@ -1756,8 +1757,8 @@
       regcache_cooked_read_signed (current_regcache, regno, &val);
       return val;
     }
-  else if ((regno % NUM_REGS) == SP_REGNUM)
-    /* The SP_REGNUM is special, its value is stored in saved_regs.
+  else if ((regno % NUM_REGS) == MIPS_SP_REGNUM)
+    /* MIPS_SP_REGNUM is special, its value is stored in saved_regs.
        In fact, it is so special that it can even only be fetched
        using a raw register number!  Once this code as been converted
        to frame-unwind the problem goes away.  */
@@ -2194,7 +2195,7 @@
 	  /* Old gcc frame, r30 is virtual frame pointer.  */
 	  if ((long) low_word != PROC_FRAME_OFFSET (&temp_proc_desc))
 	    frame_addr = sp + low_word;
-	  else if (PROC_FRAME_REG (&temp_proc_desc) == SP_REGNUM)
+	  else if (PROC_FRAME_REG (&temp_proc_desc) == MIPS_SP_REGNUM)
 	    {
 	      unsigned alloca_adjust;
 	      PROC_FRAME_REG (&temp_proc_desc) = 30;
@@ -2217,7 +2218,7 @@
       else if (inst == 0x03A0F021 || inst == 0x03a0f025 || inst == 0x03a0f02d)
 	{
 	  /* New gcc frame, virtual frame pointer is at r30 + frame_size.  */
-	  if (PROC_FRAME_REG (&temp_proc_desc) == SP_REGNUM)
+	  if (PROC_FRAME_REG (&temp_proc_desc) == MIPS_SP_REGNUM)
 	    {
 	      unsigned alloca_adjust;
 	      PROC_FRAME_REG (&temp_proc_desc) = 30;
@@ -2249,7 +2250,7 @@
   CORE_ADDR sp;
 
   if (cur_frame)
-    sp = read_next_frame_reg (next_frame, NUM_REGS + SP_REGNUM);
+    sp = read_next_frame_reg (next_frame, NUM_REGS + MIPS_SP_REGNUM);
   else
     sp = 0;
 
@@ -2259,7 +2260,7 @@
   temp_saved_regs = xrealloc (temp_saved_regs, SIZEOF_FRAME_SAVED_REGS);
   memset (temp_saved_regs, '\0', SIZEOF_FRAME_SAVED_REGS);
   PROC_LOW_ADDR (&temp_proc_desc) = start_pc;
-  PROC_FRAME_REG (&temp_proc_desc) = SP_REGNUM;
+  PROC_FRAME_REG (&temp_proc_desc) = MIPS_SP_REGNUM;
   PROC_PC_REG (&temp_proc_desc) = RA_REGNUM;
 
   if (start_pc + 200 < limit_pc)
@@ -3029,7 +3030,7 @@
 	fprintf_unfiltered (gdb_stdlog, "\n");
     }
 
-  regcache_cooked_write_signed (regcache, SP_REGNUM, sp);
+  regcache_cooked_write_signed (regcache, MIPS_SP_REGNUM, sp);
 
   /* Return adjusted stack pointer.  */
   return sp;
@@ -3315,7 +3316,7 @@
 	fprintf_unfiltered (gdb_stdlog, "\n");
     }
 
-  regcache_cooked_write_signed (regcache, SP_REGNUM, sp);
+  regcache_cooked_write_signed (regcache, MIPS_SP_REGNUM, sp);
 
   /* Return adjusted stack pointer.  */
   return sp;
@@ -3730,7 +3731,7 @@
 	fprintf_unfiltered (gdb_stdlog, "\n");
     }
 
-  regcache_cooked_write_signed (regcache, SP_REGNUM, sp);
+  regcache_cooked_write_signed (regcache, MIPS_SP_REGNUM, sp);
 
   /* Return adjusted stack pointer.  */
   return sp;
@@ -4184,7 +4185,7 @@
 	fprintf_unfiltered (gdb_stdlog, "\n");
     }
 
-  regcache_cooked_write_signed (regcache, SP_REGNUM, sp);
+  regcache_cooked_write_signed (regcache, MIPS_SP_REGNUM, sp);
 
   /* Return adjusted stack pointer.  */
   return sp;
@@ -5614,7 +5615,8 @@
       }
     /* FIXME: cagney/2003-11-15: For MIPS, hasn't PC_REGNUM been
        replaced by read_pc?  */
-    set_gdbarch_pc_regnum (gdbarch, regnum->pc);
+    set_gdbarch_pc_regnum (gdbarch, regnum->pc + num_regs);
+    set_gdbarch_sp_regnum (gdbarch, MIPS_SP_REGNUM + num_regs);
     set_gdbarch_fp0_regnum (gdbarch, regnum->fp0);
     set_gdbarch_num_regs (gdbarch, num_regs);
     set_gdbarch_num_pseudo_regs (gdbarch, num_regs);
@@ -5738,6 +5740,9 @@
   set_gdbarch_addr_bits_remove (gdbarch, mips_addr_bits_remove);
 
   /* Unwind the frame.  */
+  frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
+  frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer);
+
   set_gdbarch_unwind_pc (gdbarch, mips_unwind_pc);
   set_gdbarch_unwind_dummy_id (gdbarch, mips_unwind_dummy_id);
 
Index: mips-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.h,v
retrieving revision 1.7
diff -u -u -r1.7 mips-tdep.h
--- mips-tdep.h	29 Apr 2004 18:28:43 -0000	1.7
+++ mips-tdep.h	8 Jun 2004 03:48:15 -0000
@@ -62,6 +62,7 @@
 extern const struct mips_regnum *mips_regnum (struct gdbarch *gdbarch);
 
 enum {
+  MIPS_SP_REGNUM = 29,
   MIPS_EMBED_LO_REGNUM = 33,
   MIPS_EMBED_HI_REGNUM = 34,
   MIPS_EMBED_BADVADDR_REGNUM = 35,
Index: config/mips/tm-mips.h
===================================================================
RCS file: /cvs/src/src/gdb/config/mips/tm-mips.h,v
retrieving revision 1.59
diff -u -u -r1.59 tm-mips.h
--- config/mips/tm-mips.h	1 May 2004 00:34:55 -0000	1.59
+++ config/mips/tm-mips.h	8 Jun 2004 03:48:15 -0000
@@ -56,7 +56,6 @@
 #define V0_REGNUM 2		/* Function integer return value */
 #define A0_REGNUM 4		/* Loc of first arg during a subr call */
 #define T9_REGNUM 25		/* Contains address of callee in PIC */
-#define SP_REGNUM 29		/* Contains address of top of stack */
 #define RA_REGNUM 31		/* Contains return address value */
 #define PS_REGNUM 32		/* Contains processor status */
 #define	UNUSED_REGNUM 73	/* Never used, FIXME */

Re: [RFA] MIPS DWARF2 CFI support

[Daniel Jacobowitz]

On Mon, Jun 07, 2004 at 08:49:08PM -0700, Martin M. Hunt wrote:
> This is really kevinb's work, with some minor tweaks.  I've tested this
> on over 50 multilib combinations and it looks goood.
> 
> 2004-05-26  Kevin Buettner: 
> 
> 	* dwarf2-frame.c (execute_cfa_program): Fix typo in which the
> 	alignment was being added to the offset instead of multiplied.
> 
> 	* dwarf2-frame.c (struct comp_unit): Add new field ``signed_addr_p''.
> 	(encoding_for_size): Add new parameter ``signed_addr_p''.  Adjust
> 	all callers.  Add code for handling signed encodings.
> 	(dwarf2_build_frame_info): Initialize ``unit.signed_addr_p''.
> 
> 	* dwarf2-frame.c (dwarf2_build_frame_info): Set unit.addr_size.
> 
> 	* config/mips/tm-mips.h (SP_REGNUM): Delete define.
> 	* mips-tdep.h (MIPS_SP_REGNUM): Define.
> 	* mips-tdep.c (mips_gdbarch_init): Set SP_REGNUM via call
> 	to set_gdbarch_sp_regnum().  Use cooked register number.
> 	(SP_REGNUM): Replace all occurrences with MIPS_SP_REGNUM.
> 
> 	* mips-tdep.c (mips_gdbarch_init): Set PC_REGNUM (via
> 	call to set_gdbarch_pc_regnum()) to cooked register number.
> 
> 	* mips-tdep.c (dwarf2-frame.h): Include.
> 	(mips_gdbarch_init): Enable DWARF2 CFI support.

I've got three objections:
  - This is a whole lot of changes.  Please submit them independently,
    with some sort of attempt at explanation.

> -  unit.addr_size = objfile->obfd->arch_info->bits_per_address / 8;
> +  unit.addr_size = TYPE_LENGTH (builtin_type_void_data_ptr);
> +  unit.signed_addr_p = bfd_get_sign_extend_vma (unit.abfd);

  - This is wrong; please don't introduce dependencies on GDB's type
    system in dwarf2-frame.c.  I imagine what's there isn't right
    either because arch_info is completely unreliable on MIPS.  There
    is already a dwarf64_p where this is used; is that right?

    Actually, this sounds like a problem in the dwarf3 draft that
    someone (maybe Jim?) brought up on the dwarf2 list a few weeks ago.
    Jim, do you remember?

And somewhat theoretical:
  - I'd really rather not enable dwarf2 unwinding for MIPS until
    someone makes a go at fixing the MIPS prologue unwinder.  Either
    that or rip it out; because after this patch it will get almost
    zero coverage in the testsuite.  This is an ongoing problem.
    I know that the existing prologue unwinder is broken at the
    beginning of functions and in a number of other situations.


-- 
Daniel Jacobowitz

Re: [RFA] MIPS DWARF2 CFI support

[Andrew Cagney]

> On Mon, Jun 07, 2004 at 08:49:08PM -0700, Martin M. Hunt wrote:
> 
>>> This is really kevinb's work, with some minor tweaks.  I've tested this
>>> on over 50 multilib combinations and it looks goood.
>>> 
>>> 2004-05-26  Kevin Buettner: 
>>> 
>>> 	* dwarf2-frame.c (execute_cfa_program): Fix typo in which the
>>> 	alignment was being added to the offset instead of multiplied.
>>> 
>>> 	* dwarf2-frame.c (struct comp_unit): Add new field ``signed_addr_p''.
>>> 	(encoding_for_size): Add new parameter ``signed_addr_p''.  Adjust
>>> 	all callers.  Add code for handling signed encodings.
>>> 	(dwarf2_build_frame_info): Initialize ``unit.signed_addr_p''.
>>> 
>>> 	* dwarf2-frame.c (dwarf2_build_frame_info): Set unit.addr_size.
>>> 
>>> 	* config/mips/tm-mips.h (SP_REGNUM): Delete define.
>>> 	* mips-tdep.h (MIPS_SP_REGNUM): Define.
>>> 	* mips-tdep.c (mips_gdbarch_init): Set SP_REGNUM via call
>>> 	to set_gdbarch_sp_regnum().  Use cooked register number.
>>> 	(SP_REGNUM): Replace all occurrences with MIPS_SP_REGNUM.
>>> 
>>> 	* mips-tdep.c (mips_gdbarch_init): Set PC_REGNUM (via
>>> 	call to set_gdbarch_pc_regnum()) to cooked register number.
>>> 
>>> 	* mips-tdep.c (dwarf2-frame.h): Include.
>>> 	(mips_gdbarch_init): Enable DWARF2 CFI support.
> 
> 
> I've got three objections:
>   - This is a whole lot of changes.  Please submit them independently,
>     with some sort of attempt at explanation.

(Yes please, the SP_REGNUM looked to be an easy one to split out and the 
bulk of the change).

>>> -  unit.addr_size = objfile->obfd->arch_info->bits_per_address / 8;
>>> +  unit.addr_size = TYPE_LENGTH (builtin_type_void_data_ptr);
>>> +  unit.signed_addr_p = bfd_get_sign_extend_vma (unit.abfd);
> 
> 
>   - This is wrong; please don't introduce dependencies on GDB's type
>     system in dwarf2-frame.c.  I imagine what's there isn't right
>     either because arch_info is completely unreliable on MIPS.  There
>     is already a dwarf64_p where this is used; is that right?
> 
>     Actually, this sounds like a problem in the dwarf3 draft that
>     someone (maybe Jim?) brought up on the dwarf2 list a few weeks ago.
>     Jim, do you remember?
> 
> And somewhat theoretical:
>   - I'd really rather not enable dwarf2 unwinding for MIPS until
>     someone makes a go at fixing the MIPS prologue unwinder.  Either
>     that or rip it out; because after this patch it will get almost
>     zero coverage in the testsuite.  This is an ongoing problem.
>     I know that the existing prologue unwinder is broken at the
>     beginning of functions and in a number of other situations.

I don't think delaying CFI until the prologue unwinder is working is 
reasonable.  However, what would help is the addition of a new more 
specialized stub-unwinder (see hppa and s390) (perhaphs later we can 
return to cleaning up the existing prologue unwinder).  Patches for the 
former (the stub unwinder) more than welcome.

Andrew