[PATCH]: Handle sparc skipping unimp instruction with dwarf2

[PATCH]: Handle sparc skipping unimp instruction with dwarf2

[David S. Miller]

This is a followon to my previous posting entitled:

[PATCH]: Setup sparc32 to support dwarf2 unwind sniffer

and this patch is relative to that one.

If you actually enable the dwarf2 unwind sniffer on sparc 32-bit
you'll hit a bunch of regressions in gdb.base/struct.exp, and
the reason is simple.  The dwarf2 unwinder doesn't know about the
convention where we should skip the caller's unimp instruction
when returning from a function that returns a structure.

This is very easy to fix with just a little bit of infrastructure
adjustments.  What we do is we pass in the fs->pc to the dwarf2 reg
init function.  We use this PC to find out if the function we are
analyzing returns a struct or not.  If so, we adjust the PC and NPC
by 4.

Sparc64 doesn't have this funny convention so all of this is not
relevant there.

I tested using the dwarf2 unwinder for sparc32 Linux/Sparc and there
are no regressions, and using dwarf2 in fact fixes a few of the
remaining threading testcase failures (there are still some more to
tackle alas :).

Comments?

2006-04-04  David S. Miller  <davem@sunset.davemloft.net>

	* dwarf2-frame.c (dwarf2_frame_ops init_reg): Add CORE_ADDR "pc"
	argument.
	(dwarf2_frame_default_init_reg): Likewise.
	(dwarf2_frame_set_init_reg): Update init_reg arg.
	(dwarf2_frame_init_reg): Take "pc" and pass it to ops->init_reg().
	(dwarf2_frame_cache): Pass fs->pc to dwarf2_frame_init_reg.
	* dwarf2-frame.h (dwarf2-frame_set_init_reg): Update declaration.
	* cris-tdep.c (cris_dwarf2_frame_init_reg): Add "pc" argument.
	* s390-tdep.c (s390_dwarf2_frame_init_reg): Likewise.
	* sh-tdep.c (sh_dwarf2_frame_init_reg): Likewise.
	* sparc64-tdep.c (sparc64_dwarf2_frame_init_reg): Likewise.
	* sparc-tdep.c (sparc_dwarf2_frame_init_reg): Likewise, and use
	it to determine if the function returns a structure and thus we
	have to indicate the return PC and NPC are 4 bytes later than
	usual.
	
--- dwarf2-frame.h.~1~	2006-02-25 15:04:58.000000000 -0800
+++ dwarf2-frame.h	2006-04-04 21:35:50.000000000 -0700
@@ -79,7 +79,8 @@ struct dwarf2_frame_state_reg
 
 extern void dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
 				       void (*init_reg) (struct gdbarch *, int,
-					     struct dwarf2_frame_state_reg *));
+					     struct dwarf2_frame_state_reg *,
+					     CORE_ADDR));
 
 /* Set the architecture-specific signal trampoline recognition
    function for GDBARCH to SIGNAL_FRAME_P.  */
--- dwarf2-frame.c.~1~	2006-02-25 15:04:58.000000000 -0800
+++ dwarf2-frame.c	2006-04-04 21:36:40.000000000 -0700
@@ -506,7 +506,8 @@ static struct gdbarch_data *dwarf2_frame
 struct dwarf2_frame_ops
 {
   /* Pre-initialize the register state REG for register REGNUM.  */
-  void (*init_reg) (struct gdbarch *, int, struct dwarf2_frame_state_reg *);
+  void (*init_reg) (struct gdbarch *, int, struct dwarf2_frame_state_reg *,
+		    CORE_ADDR);
 
   /* Check whether the frame preceding NEXT_FRAME will be a signal
      trampoline.  */
@@ -518,7 +519,8 @@ struct dwarf2_frame_ops
 
 static void
 dwarf2_frame_default_init_reg (struct gdbarch *gdbarch, int regnum,
-			       struct dwarf2_frame_state_reg *reg)
+			       struct dwarf2_frame_state_reg *reg,
+			       CORE_ADDR pc)
 {
   /* If we have a register that acts as a program counter, mark it as
      a destination for the return address.  If we have a register that
@@ -570,7 +572,8 @@ dwarf2_frame_init (struct obstack *obsta
 void
 dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
 			   void (*init_reg) (struct gdbarch *, int,
-					     struct dwarf2_frame_state_reg *))
+					     struct dwarf2_frame_state_reg *,
+					     CORE_ADDR))
 {
   struct dwarf2_frame_ops *ops = gdbarch_data (gdbarch, dwarf2_frame_data);
 
@@ -581,11 +584,12 @@ dwarf2_frame_set_init_reg (struct gdbarc
 
 static void
 dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-		       struct dwarf2_frame_state_reg *reg)
+		       struct dwarf2_frame_state_reg *reg,
+		       CORE_ADDR pc)
 {
   struct dwarf2_frame_ops *ops = gdbarch_data (gdbarch, dwarf2_frame_data);
 
-  ops->init_reg (gdbarch, regnum, reg);
+  ops->init_reg (gdbarch, regnum, reg, pc);
 }
 
 /* Set the architecture-specific signal trampoline recognition
@@ -713,7 +717,7 @@ dwarf2_frame_cache (struct frame_info *n
     int regnum;
 
     for (regnum = 0; regnum < num_regs; regnum++)
-      dwarf2_frame_init_reg (gdbarch, regnum, &cache->reg[regnum]);
+      dwarf2_frame_init_reg (gdbarch, regnum, &cache->reg[regnum], fs->pc);
   }
 
   /* Go through the DWARF2 CFI generated table and save its register
--- cris-tdep.c.~1~	2006-02-25 15:04:58.000000000 -0800
+++ cris-tdep.c	2006-04-04 21:36:50.000000000 -0700
@@ -1852,7 +1852,8 @@ cris_dwarf2_reg_to_regnum (int reg)
 
 static void
 cris_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-                            struct dwarf2_frame_state_reg *reg)
+                            struct dwarf2_frame_state_reg *reg,
+			    CORE_ADDR pc)
 {
   /* The return address column.  */
   if (regnum == PC_REGNUM)
--- s390-tdep.c.~1~	2006-02-25 15:05:02.000000000 -0800
+++ s390-tdep.c	2006-04-04 21:36:56.000000000 -0700
@@ -2279,7 +2279,8 @@ s390_unwind_sp (struct gdbarch *gdbarch,
 
 static void
 s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-                            struct dwarf2_frame_state_reg *reg)
+                            struct dwarf2_frame_state_reg *reg,
+			    CORE_ADDR pc)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
--- sh-tdep.c.~1~	2006-04-04 14:52:27.000000000 -0700
+++ sh-tdep.c	2006-04-04 21:37:07.000000000 -0700
@@ -2123,7 +2123,8 @@ sh_sh2a_register_sim_regno (int nr)
 
 static void
 sh_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-                          struct dwarf2_frame_state_reg *reg)
+                          struct dwarf2_frame_state_reg *reg,
+			  CORE_ADDR pc)
 {
   /* Mark the PC as the destination for the return address.  */
   if (regnum == PC_REGNUM)
--- sparc64-tdep.c.~1~	2006-02-25 15:05:03.000000000 -0800
+++ sparc64-tdep.c	2006-04-04 21:37:16.000000000 -0700
@@ -1106,7 +1106,8 @@ sparc64_return_value (struct gdbarch *gd
 
 static void
 sparc64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-			       struct dwarf2_frame_state_reg *reg)
+			       struct dwarf2_frame_state_reg *reg,
+			       CORE_ADDR pc)
 {
   switch (regnum)
     {
--- sparc-tdep.c.~2~	2006-04-04 20:19:46.000000000 -0700
+++ sparc-tdep.c	2006-04-04 21:35:29.000000000 -0700
@@ -995,9 +995,33 @@
 	  || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16));
 }
 
+static int
+sparc32_dwarf2_struct_return_p (CORE_ADDR pc)
+{
+  struct symbol *sym;
+
+  sym = find_pc_function (pc);
+  if (sym)
+    {
+      struct type *type = check_typedef (SYMBOL_TYPE (sym));
+      enum type_code code = TYPE_CODE (type);
+
+      if (code == TYPE_CODE_FUNC || code == TYPE_CODE_METHOD)
+	{
+	  type = check_typedef (TYPE_TARGET_TYPE (type));
+	  if (sparc_structure_or_union_p (type)
+	      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
+	    return 1;
+	}
+    }
+
+  return 0;
+}
+
 static void
 sparc32_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-			       struct dwarf2_frame_state_reg *reg)
+			       struct dwarf2_frame_state_reg *reg,
+			       CORE_ADDR pc)
 {
   switch (regnum)
     {
@@ -1012,11 +1036,11 @@
       break;
     case SPARC32_PC_REGNUM:
       reg->how = DWARF2_FRAME_REG_RA_OFFSET;
-      reg->loc.offset = 8;
+      reg->loc.offset = (sparc32_dwarf2_struct_return_p (pc) ? 12 : 8);
       break;
     case SPARC32_NPC_REGNUM:
       reg->how = DWARF2_FRAME_REG_RA_OFFSET;
-      reg->loc.offset = 12;
+      reg->loc.offset = (sparc32_dwarf2_struct_return_p (pc) ? 16 : 12);
       break;
     }
 }

Re: [PATCH]: Handle sparc skipping unimp instruction with dwarf2

[Mark Kettenis]

>
>  This is a followon to my previous posting entitled:
>
>  [PATCH]: Setup sparc32 to support dwarf2 unwind sniffer
>
>  and this patch is relative to that one.
>
>  If you actually enable the dwarf2 unwind sniffer on sparc 32-bit
>  you'll hit a bunch of regressions in gdb.base/struct.exp, and
>  the reason is simple.  The dwarf2 unwinder doesn't know about the
>  convention where we should skip the caller's unimp instruction
>  when returning from a function that returns a structure.

Oh, it's coming back to me now.  That was another reason why I didn't
bother with the dwarf2 unwinder on 32-bit sparc yet.  Guess you don't
want to enable the dwarf2 unwinder for 32-bit Linux just yet ;-).

>  This is very easy to fix with just a little bit of infrastructure
>  adjustments.  What we do is we pass in the fs->pc to the dwarf2 reg
>  init function.  We use this PC to find out if the function we are
>  analyzing returns a struct or not.  If so, we adjust the PC and NPC
>  by 4.

Rather than passing the PC, I think you should actually pass down
NEXT_FRAME.  Other targets might have different requirements, and you
can unwind the PC from NEXT_FRAME in the sparc code.

Oh, and could you rename sparc32_dwarf2_struct_return_p() to
sparc32_struct_return_p() and make sparc32_frame_cache() use that function
too?  The check isn't really dwarf2-specific and duplication of code
is bad!

Thanks,

Mark

>  2006-04-04  David S. Miller  <davem@sunset.davemloft.net>
>
>  	* dwarf2-frame.c (dwarf2_frame_ops init_reg): Add CORE_ADDR "pc"
>  	argument.
>  	(dwarf2_frame_default_init_reg): Likewise.
>  	(dwarf2_frame_set_init_reg): Update init_reg arg.
>  	(dwarf2_frame_init_reg): Take "pc" and pass it to ops->init_reg().
>  	(dwarf2_frame_cache): Pass fs->pc to dwarf2_frame_init_reg.
>  	* dwarf2-frame.h (dwarf2-frame_set_init_reg): Update declaration.
>  	* cris-tdep.c (cris_dwarf2_frame_init_reg): Add "pc" argument.
>  	* s390-tdep.c (s390_dwarf2_frame_init_reg): Likewise.
>  	* sh-tdep.c (sh_dwarf2_frame_init_reg): Likewise.
>  	* sparc64-tdep.c (sparc64_dwarf2_frame_init_reg): Likewise.
>  	* sparc-tdep.c (sparc_dwarf2_frame_init_reg): Likewise, and use
>  	it to determine if the function returns a structure and thus we
>  	have to indicate the return PC and NPC are 4 bytes later than
>  	usual.
>
>  --- dwarf2-frame.h.~1~	2006-02-25 15:04:58.000000000 -0800
>  +++ dwarf2-frame.h	2006-04-04 21:35:50.000000000 -0700
>  @@ -79,7 +79,8 @@ struct dwarf2_frame_state_reg
>
>   extern void dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
>   				       void (*init_reg) (struct gdbarch *, int,
>  -					     struct dwarf2_frame_state_reg *));
>  +					     struct dwarf2_frame_state_reg *,
>  +					     CORE_ADDR));
>
>   /* Set the architecture-specific signal trampoline recognition
>      function for GDBARCH to SIGNAL_FRAME_P.  */
>  --- dwarf2-frame.c.~1~	2006-02-25 15:04:58.000000000 -0800
>  +++ dwarf2-frame.c	2006-04-04 21:36:40.000000000 -0700
>  @@ -506,7 +506,8 @@ static struct gdbarch_data *dwarf2_frame
>   struct dwarf2_frame_ops
>   {
>     /* Pre-initialize the register state REG for register REGNUM.  */
>  -  void (*init_reg) (struct gdbarch *, int, struct dwarf2_frame_state_reg
>  *);
>  +  void (*init_reg) (struct gdbarch *, int, struct dwarf2_frame_state_reg
>  *,
>  +		    CORE_ADDR);
>
>     /* Check whether the frame preceding NEXT_FRAME will be a signal
>        trampoline.  */
>  @@ -518,7 +519,8 @@ struct dwarf2_frame_ops
>
>   static void
>   dwarf2_frame_default_init_reg (struct gdbarch *gdbarch, int regnum,
>  -			       struct dwarf2_frame_state_reg *reg)
>  +			       struct dwarf2_frame_state_reg *reg,
>  +			       CORE_ADDR pc)
>   {
>     /* If we have a register that acts as a program counter, mark it as
>        a destination for the return address.  If we have a register that
>  @@ -570,7 +572,8 @@ dwarf2_frame_init (struct obstack *obsta
>   void
>   dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
>   			   void (*init_reg) (struct gdbarch *, int,
>  -					     struct dwarf2_frame_state_reg *))
>  +					     struct dwarf2_frame_state_reg *,
>  +					     CORE_ADDR))
>   {
>     struct dwarf2_frame_ops *ops = gdbarch_data (gdbarch,
>  dwarf2_frame_data);
>
>  @@ -581,11 +584,12 @@ dwarf2_frame_set_init_reg (struct gdbarc
>
>   static void
>   dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>  -		       struct dwarf2_frame_state_reg *reg)
>  +		       struct dwarf2_frame_state_reg *reg,
>  +		       CORE_ADDR pc)
>   {
>     struct dwarf2_frame_ops *ops = gdbarch_data (gdbarch,
>  dwarf2_frame_data);
>
>  -  ops->init_reg (gdbarch, regnum, reg);
>  +  ops->init_reg (gdbarch, regnum, reg, pc);
>   }
>
>   /* Set the architecture-specific signal trampoline recognition
>  @@ -713,7 +717,7 @@ dwarf2_frame_cache (struct frame_info *n
>       int regnum;
>
>       for (regnum = 0; regnum < num_regs; regnum++)
>  -      dwarf2_frame_init_reg (gdbarch, regnum, &cache->reg[regnum]);
>  +      dwarf2_frame_init_reg (gdbarch, regnum, &cache->reg[regnum],
>  fs->pc);
>     }
>
>     /* Go through the DWARF2 CFI generated table and save its register
>  --- cris-tdep.c.~1~	2006-02-25 15:04:58.000000000 -0800
>  +++ cris-tdep.c	2006-04-04 21:36:50.000000000 -0700
>  @@ -1852,7 +1852,8 @@ cris_dwarf2_reg_to_regnum (int reg)
>
>   static void
>   cris_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>  -                            struct dwarf2_frame_state_reg *reg)
>  +                            struct dwarf2_frame_state_reg *reg,
>  +			    CORE_ADDR pc)
>   {
>     /* The return address column.  */
>     if (regnum == PC_REGNUM)
>  --- s390-tdep.c.~1~	2006-02-25 15:05:02.000000000 -0800
>  +++ s390-tdep.c	2006-04-04 21:36:56.000000000 -0700
>  @@ -2279,7 +2279,8 @@ s390_unwind_sp (struct gdbarch *gdbarch,
>
>   static void
>   s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>  -                            struct dwarf2_frame_state_reg *reg)
>  +                            struct dwarf2_frame_state_reg *reg,
>  +			    CORE_ADDR pc)
>   {
>     struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
>  --- sh-tdep.c.~1~	2006-04-04 14:52:27.000000000 -0700
>  +++ sh-tdep.c	2006-04-04 21:37:07.000000000 -0700
>  @@ -2123,7 +2123,8 @@ sh_sh2a_register_sim_regno (int nr)
>
>   static void
>   sh_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>  -                          struct dwarf2_frame_state_reg *reg)
>  +                          struct dwarf2_frame_state_reg *reg,
>  +			  CORE_ADDR pc)
>   {
>     /* Mark the PC as the destination for the return address.  */
>     if (regnum == PC_REGNUM)
>  --- sparc64-tdep.c.~1~	2006-02-25 15:05:03.000000000 -0800
>  +++ sparc64-tdep.c	2006-04-04 21:37:16.000000000 -0700
>  @@ -1106,7 +1106,8 @@ sparc64_return_value (struct gdbarch *gd
>
>   static void
>   sparc64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>  -			       struct dwarf2_frame_state_reg *reg)
>  +			       struct dwarf2_frame_state_reg *reg,
>  +			       CORE_ADDR pc)
>   {
>     switch (regnum)
>       {
>  --- sparc-tdep.c.~2~	2006-04-04 20:19:46.000000000 -0700
>  +++ sparc-tdep.c	2006-04-04 21:35:29.000000000 -0700
>  @@ -995,9 +995,33 @@
>   	  || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16));
>   }
>
>  +static int
>  +sparc32_dwarf2_struct_return_p (CORE_ADDR pc)
>  +{
>  +  struct symbol *sym;
>  +
>  +  sym = find_pc_function (pc);
>  +  if (sym)
>  +    {
>  +      struct type *type = check_typedef (SYMBOL_TYPE (sym));
>  +      enum type_code code = TYPE_CODE (type);
>  +
>  +      if (code == TYPE_CODE_FUNC || code == TYPE_CODE_METHOD)
>  +	{
>  +	  type = check_typedef (TYPE_TARGET_TYPE (type));
>  +	  if (sparc_structure_or_union_p (type)
>  +	      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
>  +	    return 1;
>  +	}
>  +    }
>  +
>  +  return 0;
>  +}
>  +
>   static void
>   sparc32_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>  -			       struct dwarf2_frame_state_reg *reg)
>  +			       struct dwarf2_frame_state_reg *reg,
>  +			       CORE_ADDR pc)
>   {
>     switch (regnum)
>       {
>  @@ -1012,11 +1036,11 @@
>         break;
>       case SPARC32_PC_REGNUM:
>         reg->how = DWARF2_FRAME_REG_RA_OFFSET;
>  -      reg->loc.offset = 8;
>  +      reg->loc.offset = (sparc32_dwarf2_struct_return_p (pc) ? 12 : 8);
>         break;
>       case SPARC32_NPC_REGNUM:
>         reg->how = DWARF2_FRAME_REG_RA_OFFSET;
>  -      reg->loc.offset = 12;
>  +      reg->loc.offset = (sparc32_dwarf2_struct_return_p (pc) ? 16 : 12);
>         break;
>       }
>   }
>

Re: [PATCH]: Handle sparc skipping unimp instruction with dwarf2

[David S. Miller]

From: "Mark Kettenis" <mark.kettenis@xs4all.nl>
Date: Wed, 5 Apr 2006 11:30:07 +0200 (CEST)

> >  This is very easy to fix with just a little bit of infrastructure
> >  adjustments.  What we do is we pass in the fs->pc to the dwarf2 reg
> >  init function.  We use this PC to find out if the function we are
> >  analyzing returns a struct or not.  If so, we adjust the PC and NPC
> >  by 4.
> 
> Rather than passing the PC, I think you should actually pass down
> NEXT_FRAME.  Other targets might have different requirements, and you
> can unwind the PC from NEXT_FRAME in the sparc code.
> 
> Oh, and could you rename sparc32_dwarf2_struct_return_p() to
> sparc32_struct_return_p() and make sparc32_frame_cache() use that function
> too?  The check isn't really dwarf2-specific and duplication of code
> is bad!

Easier said than done :-) The else block of that code can't be used in
the dwarf2 case, because when we can't find the "sym" for the function
it wants to have the sparc frame cache information, such as whether
this is a frameless function or not.  It uses this to determine
whether to look at O7 or I7 as the return address register.

I tried to consolidate as much as I could.  In the dwarf2 case,
if we have dwarf2 unwind information, and we can't find a symbol
for the same function, we've got other problems right? :-)  The
only other option I can think of is to pass in the retaddr_column
to the ->init_reg() method but that seems unnecessary.

Anyways, how does this look?  Ok to commit?

2006-04-05  David S. Miller  <davem@sunset.davemloft.net>

	* dwarf2-frame.c (dwarf2_frame_ops init_reg): Add "next_frame"
	argument.
	(dwarf2_frame_default_init_reg): Likewise.
	(dwarf2_frame_set_init_reg): Update init_reg arg.
	(dwarf2_frame_init_reg): Take "next_frame" and pass it to
	ops->init_reg().
	(dwarf2_frame_cache): Pass next_frame to dwarf2_frame_init_reg.
	* dwarf2-frame.h (dwarf2-frame_set_init_reg): Update declaration.
	* cris-tdep.c (cris_dwarf2_frame_init_reg): Add next_frame arg.
	* s390-tdep.c (s390_dwarf2_frame_init_reg): Likewise.
	* sh-tdep.c (sh_dwarf2_frame_init_reg): Likewise.
	* sparc64-tdep.c (sparc64_dwarf2_frame_init_reg): Likewise.
	* sparc-tdep.c (sparc32_struct_return_from_sym): New function.
	(sparc32_frame_cache): Call it.
	(sparc32_dwarf2_struct_return_p): New function.
	(sparc_dwarf2_frame_init_reg): Use it to determine if the function
	returns a structure and thus we have to indicate the return PC and
	NPC are 4 bytes later than usual.

--- ./dwarf2-frame.h.~1~	2006-04-05 11:32:55.000000000 -0700
+++ ./dwarf2-frame.h	2006-04-05 11:57:19.000000000 -0700
@@ -79,7 +79,8 @@ struct dwarf2_frame_state_reg
 
 extern void dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
 				       void (*init_reg) (struct gdbarch *, int,
-					     struct dwarf2_frame_state_reg *));
+					     struct dwarf2_frame_state_reg *,
+					     struct frame_info *));
 
 /* Set the architecture-specific signal trampoline recognition
    function for GDBARCH to SIGNAL_FRAME_P.  */
--- ./dwarf2-frame.c.~1~	2006-04-05 11:32:55.000000000 -0700
+++ ./dwarf2-frame.c	2006-04-05 11:58:38.000000000 -0700
@@ -506,7 +506,8 @@ static struct gdbarch_data *dwarf2_frame
 struct dwarf2_frame_ops
 {
   /* Pre-initialize the register state REG for register REGNUM.  */
-  void (*init_reg) (struct gdbarch *, int, struct dwarf2_frame_state_reg *);
+  void (*init_reg) (struct gdbarch *, int, struct dwarf2_frame_state_reg *,
+		    struct frame_info *);
 
   /* Check whether the frame preceding NEXT_FRAME will be a signal
      trampoline.  */
@@ -518,7 +519,8 @@ struct dwarf2_frame_ops
 
 static void
 dwarf2_frame_default_init_reg (struct gdbarch *gdbarch, int regnum,
-			       struct dwarf2_frame_state_reg *reg)
+			       struct dwarf2_frame_state_reg *reg,
+			       struct frame_info *next_frame)
 {
   /* If we have a register that acts as a program counter, mark it as
      a destination for the return address.  If we have a register that
@@ -570,7 +572,8 @@ dwarf2_frame_init (struct obstack *obsta
 void
 dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
 			   void (*init_reg) (struct gdbarch *, int,
-					     struct dwarf2_frame_state_reg *))
+					     struct dwarf2_frame_state_reg *,
+					     struct frame_info *))
 {
   struct dwarf2_frame_ops *ops = gdbarch_data (gdbarch, dwarf2_frame_data);
 
@@ -581,11 +584,12 @@ dwarf2_frame_set_init_reg (struct gdbarc
 
 static void
 dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-		       struct dwarf2_frame_state_reg *reg)
+		       struct dwarf2_frame_state_reg *reg,
+		       struct frame_info *next_frame)
 {
   struct dwarf2_frame_ops *ops = gdbarch_data (gdbarch, dwarf2_frame_data);
 
-  ops->init_reg (gdbarch, regnum, reg);
+  ops->init_reg (gdbarch, regnum, reg, next_frame);
 }
 
 /* Set the architecture-specific signal trampoline recognition
@@ -713,7 +717,7 @@ dwarf2_frame_cache (struct frame_info *n
     int regnum;
 
     for (regnum = 0; regnum < num_regs; regnum++)
-      dwarf2_frame_init_reg (gdbarch, regnum, &cache->reg[regnum]);
+      dwarf2_frame_init_reg (gdbarch, regnum, &cache->reg[regnum], next_frame);
   }
 
   /* Go through the DWARF2 CFI generated table and save its register
--- ./cris-tdep.c.~1~	2006-04-05 11:32:55.000000000 -0700
+++ ./cris-tdep.c	2006-04-05 11:59:07.000000000 -0700
@@ -1852,7 +1852,8 @@ cris_dwarf2_reg_to_regnum (int reg)
 
 static void
 cris_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-                            struct dwarf2_frame_state_reg *reg)
+                            struct dwarf2_frame_state_reg *reg,
+			    struct frame_info *next_frame)
 {
   /* The return address column.  */
   if (regnum == PC_REGNUM)
--- ./s390-tdep.c.~1~	2006-04-05 11:32:55.000000000 -0700
+++ ./s390-tdep.c	2006-04-05 11:59:20.000000000 -0700
@@ -2279,7 +2279,8 @@ s390_unwind_sp (struct gdbarch *gdbarch,
 
 static void
 s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-                            struct dwarf2_frame_state_reg *reg)
+                            struct dwarf2_frame_state_reg *reg,
+			    struct frame_info *next_frame)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
--- ./sh-tdep.c.~1~	2006-04-05 11:32:55.000000000 -0700
+++ ./sh-tdep.c	2006-04-05 11:59:32.000000000 -0700
@@ -2123,7 +2123,8 @@ sh_sh2a_register_sim_regno (int nr)
 
 static void
 sh_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-                          struct dwarf2_frame_state_reg *reg)
+                          struct dwarf2_frame_state_reg *reg,
+			  struct frame_info *next_frame)
 {
   /* Mark the PC as the destination for the return address.  */
   if (regnum == PC_REGNUM)
--- ./sparc64-tdep.c.~1~	2006-04-05 11:32:55.000000000 -0700
+++ ./sparc64-tdep.c	2006-04-05 11:59:49.000000000 -0700
@@ -1106,7 +1106,8 @@ sparc64_return_value (struct gdbarch *gd
 
 static void
 sparc64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-			       struct dwarf2_frame_state_reg *reg)
+			       struct dwarf2_frame_state_reg *reg,
+			       struct frame_info *next_frame)
 {
   switch (regnum)
     {
--- ./sparc-tdep.c.~1~	2006-04-05 11:34:52.000000000 -0700
+++ ./sparc-tdep.c	2006-04-05 12:07:20.000000000 -0700
@@ -680,6 +680,23 @@ sparc_frame_cache (struct frame_info *ne
   return cache;
 }
 
+static int
+sparc32_struct_return_from_sym (struct symbol *sym)
+{
+  struct type *type = check_typedef (SYMBOL_TYPE (sym));
+  enum type_code code = TYPE_CODE (type);
+
+  if (code == TYPE_CODE_FUNC || code == TYPE_CODE_METHOD)
+    {
+      type = check_typedef (TYPE_TARGET_TYPE (type));
+      if (sparc_structure_or_union_p (type)
+	  || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
+	return 1;
+    }
+
+  return 0;
+}
+
 struct sparc_frame_cache *
 sparc32_frame_cache (struct frame_info *next_frame, void **this_cache)
 {
@@ -694,16 +711,7 @@ sparc32_frame_cache (struct frame_info *
   sym = find_pc_function (cache->pc);
   if (sym)
     {
-      struct type *type = check_typedef (SYMBOL_TYPE (sym));
-      enum type_code code = TYPE_CODE (type);
-
-      if (code == TYPE_CODE_FUNC || code == TYPE_CODE_METHOD)
-	{
-	  type = check_typedef (TYPE_TARGET_TYPE (type));
-	  if (sparc_structure_or_union_p (type)
-	      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
-	    cache->struct_return_p = 1;
-	}
+      cache->struct_return_p = sparc32_struct_return_from_sym (sym);
     }
   else
     {
@@ -995,10 +1003,24 @@ sparc32_stabs_argument_has_addr (struct 
 	  || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16));
 }
 
+static int
+sparc32_dwarf2_struct_return_p (struct frame_info *next_frame)
+{
+  CORE_ADDR pc = frame_unwind_address_in_block (next_frame);
+  struct symbol *sym = find_pc_function (pc);
+
+  if (sym)
+    return sparc32_struct_return_from_sym (sym);
+  return 0;
+}
+
 static void
 sparc32_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-			       struct dwarf2_frame_state_reg *reg)
+			       struct dwarf2_frame_state_reg *reg,
+			       struct frame_info *next_frame)
 {
+  int off;
+
   switch (regnum)
     {
     case SPARC_G0_REGNUM:
@@ -1011,12 +1033,14 @@ sparc32_dwarf2_frame_init_reg (struct gd
       reg->how = DWARF2_FRAME_REG_CFA;
       break;
     case SPARC32_PC_REGNUM:
-      reg->how = DWARF2_FRAME_REG_RA_OFFSET;
-      reg->loc.offset = 8;
-      break;
     case SPARC32_NPC_REGNUM:
       reg->how = DWARF2_FRAME_REG_RA_OFFSET;
-      reg->loc.offset = 12;
+      off = 8;
+      if (sparc32_dwarf2_struct_return_p (next_frame))
+	off += 4;
+      if (regnum == SPARC32_NPC_REGNUM)
+	off += 4;
+      reg->loc.offset = off;
       break;
     }
 }

Re: [PATCH]: Handle sparc skipping unimp instruction with dwarf2

[Mark Kettenis]

> Date: Wed, 05 Apr 2006 12:17:39 -0700 (PDT)
> From: "David S. Miller" <davem@davemloft.net>
> 
> > Oh, and could you rename sparc32_dwarf2_struct_return_p() to
> > sparc32_struct_return_p() and make sparc32_frame_cache() use that function
> > too?  The check isn't really dwarf2-specific and duplication of code
> > is bad!
> 
> Easier said than done :-) The else block of that code can't be used in
> the dwarf2 case, because when we can't find the "sym" for the function
> it wants to have the sparc frame cache information, such as whether
> this is a frameless function or not.  It uses this to determine
> whether to look at O7 or I7 as the return address register.

Ah yes, I spotted the duplicated logic, but didn't look much further.

> I tried to consolidate as much as I could.  In the dwarf2 case,
> if we have dwarf2 unwind information, and we can't find a symbol
> for the same function, we've got other problems right? :-)  The
> only other option I can think of is to pass in the retaddr_column
> to the ->init_reg() method but that seems unnecessary.

The consolidation is better I think, since now all the logic that
determines the struct_return stuff from the type is in one place.

We're not completely safe if we rely on always having the type info if
we have unwind info, since gdb also pays attention to .eh_frame
sections.  But let's worry about that if we see any problems.

> Anyways, how does this look?  Ok to commit?

ok

> 2006-04-05  David S. Miller  <davem@sunset.davemloft.net>
> 
> 	* dwarf2-frame.c (dwarf2_frame_ops init_reg): Add "next_frame"
> 	argument.
> 	(dwarf2_frame_default_init_reg): Likewise.
> 	(dwarf2_frame_set_init_reg): Update init_reg arg.
> 	(dwarf2_frame_init_reg): Take "next_frame" and pass it to
> 	ops->init_reg().
> 	(dwarf2_frame_cache): Pass next_frame to dwarf2_frame_init_reg.
> 	* dwarf2-frame.h (dwarf2-frame_set_init_reg): Update declaration.
> 	* cris-tdep.c (cris_dwarf2_frame_init_reg): Add next_frame arg.
> 	* s390-tdep.c (s390_dwarf2_frame_init_reg): Likewise.
> 	* sh-tdep.c (sh_dwarf2_frame_init_reg): Likewise.
> 	* sparc64-tdep.c (sparc64_dwarf2_frame_init_reg): Likewise.
> 	* sparc-tdep.c (sparc32_struct_return_from_sym): New function.
> 	(sparc32_frame_cache): Call it.
> 	(sparc32_dwarf2_struct_return_p): New function.
> 	(sparc_dwarf2_frame_init_reg): Use it to determine if the function
> 	returns a structure and thus we have to indicate the return PC and
> 	NPC are 4 bytes later than usual.

Re: [PATCH]: Handle sparc skipping unimp instruction with dwarf2

[David S. Miller]

From: Mark Kettenis <mark.kettenis@xs4all.nl>
Date: Wed, 5 Apr 2006 21:37:06 +0200 (CEST)

> > Date: Wed, 05 Apr 2006 12:17:39 -0700 (PDT)
> > From: "David S. Miller" <davem@davemloft.net>
> > 
> > Anyways, how does this look?  Ok to commit?
> 
> ok

Done, thanks for reviewing.