[PATCH][SH] Signal handler unwinding for SH-Linux

[PATCH][SH] Signal handler unwinding for SH-Linux

[Chung-Lin Tang]

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

Hi,

This patch adds support for unwinding across signal handlers for
SH/Linux. The implementation is quite straightforward, following the
overall pattern for recognizing signal frames using the 'tramp_frame'
facilities. Currently, only 32-bit SH support is implemented.

The test results show improvement on many sighandler related tests, with
no regressions. Some gdb.base/sigbpt.exp tests change from FAIL->KFAIL,
though I'm not completely sure what that indicates.

Thanks,
Chung-Lin

2012-04-29  Chung-Lin Tang  <cltang@codesourcery.com>

	* sh-linux-tdep.c: Include trad-frame.h and tramp-frame.h.
	(sh_linux_sigtramp_cache): New function.
	(sh_linux_sigreturn_init): New function.
	(sh_linux_rt_sigreturn_init): New function.
	(SH_MOVW,SH_TRAP,SH_OR_R0_R0): New symbols for instruction
	patterns.
	(SH_NR_SIGRETURN,SH_NR_RT_SIGRETURN): New symbols for sigreturn
	syscall codes.
	(sh_linux_sigreturn_tramp_frame): New tramp_frame definition.
	(sh_linux_rt_sigreturn_tramp_frame): Likewise.
	(sh_linux_init_abi): Add init calls to register new tramp_frame
	definitions under 32-bit SH, update comments.

[-- Attachment #2: sh-sighandler-unwind.patch --]
[-- Type: text/plain, Size: 4837 bytes --]

Index: gdb/sh-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-linux-tdep.c,v
retrieving revision 1.13
diff -u -p -r1.13 sh-linux-tdep.c
--- gdb/sh-linux-tdep.c	4 Jan 2012 08:17:10 -0000	1.13
+++ gdb/sh-linux-tdep.c	29 Apr 2012 07:45:43 -0000
@@ -23,6 +23,9 @@
 #include "solib-svr4.h"
 #include "symtab.h"
 
+#include "trad-frame.h"
+#include "tramp-frame.h"
+
 #include "glibc-tdep.h"
 #include "sh-tdep.h"
 #include "linux-tdep.h"
@@ -70,6 +73,110 @@ static const struct sh_corefile_regmap f
   {-1 /* Terminator.  */, 0}
 };
 
+/* SH signal handler frame support.  */
+
+static void
+sh_linux_sigtramp_cache (struct frame_info *this_frame,
+			 struct trad_frame_cache *this_cache,
+			 CORE_ADDR func, int regs_offset)
+{
+  int i;
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  CORE_ADDR base = get_frame_register_unsigned (this_frame,
+						gdbarch_sp_regnum (gdbarch));
+  CORE_ADDR regs = base + regs_offset;
+
+  for (i = 0; i < 18; i++)
+    trad_frame_set_reg_addr (this_cache, i, regs + i * 4);
+
+  trad_frame_set_reg_addr (this_cache, SR_REGNUM, regs + 18 * 4);
+  trad_frame_set_reg_addr (this_cache, GBR_REGNUM, regs + 19 * 4);
+  trad_frame_set_reg_addr (this_cache, MACH_REGNUM, regs + 20 * 4);
+  trad_frame_set_reg_addr (this_cache, MACL_REGNUM, regs + 21 * 4);
+
+  /* Restore FP state if we have an FPU.  */
+  if (gdbarch_fp0_regnum (gdbarch) != -1)
+    {
+      CORE_ADDR fpregs = regs + 22 * 4;
+      for (i = FR0_REGNUM; i <= FP_LAST_REGNUM; i++)
+	trad_frame_set_reg_addr (this_cache, i, fpregs + i * 4);
+      trad_frame_set_reg_addr (this_cache, FPSCR_REGNUM, fpregs + 32 * 4);
+      trad_frame_set_reg_addr (this_cache, FPUL_REGNUM, fpregs + 33 * 4);
+    }
+
+  /* Save a frame ID.  */
+  trad_frame_set_id (this_cache, frame_id_build (base, func));
+}
+
+static void
+sh_linux_sigreturn_init (const struct tramp_frame *self,
+			 struct frame_info *this_frame,
+			 struct trad_frame_cache *this_cache,
+			 CORE_ADDR func)
+{
+  /* SH 32-bit sigframe: sigcontext at start of sigframe,
+     registers start after a single 'oldmask' word.  */
+  sh_linux_sigtramp_cache (this_frame, this_cache, func, 4);
+}
+
+static void
+sh_linux_rt_sigreturn_init (const struct tramp_frame *self,
+			    struct frame_info *this_frame,
+			    struct trad_frame_cache *this_cache,
+			    CORE_ADDR func)
+{
+  /* SH 32-bit rt_sigframe: starts with a siginfo (128 bytes), then
+     we can find sigcontext embedded within a ucontext (offset 20 bytes).
+     Then registers start after a single 'oldmask' word.  */
+  sh_linux_sigtramp_cache (this_frame, this_cache, func,
+			   128 /* sizeof (struct siginfo)  */
+			   + 20 /* offsetof (struct ucontext, uc_mcontext) */
+			   + 4 /* oldmask word at start of sigcontext */);
+}
+
+/* Instruction patterns.  */
+#define SH_MOVW     0x9305
+#define SH_TRAP     0xc300
+#define SH_OR_R0_R0 0x200b       
+
+/* SH sigreturn syscall numbers.  */
+#define SH_NR_SIGRETURN 0x0077
+#define SH_NR_RT_SIGRETURN 0x00ad
+
+static struct tramp_frame sh_linux_sigreturn_tramp_frame = {
+  SIGTRAMP_FRAME,
+  2,
+  {
+    { SH_MOVW, 0xffff },
+    { SH_TRAP, 0xff00 }, /* #imm argument part filtered out.  */
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_NR_SIGRETURN, 0xffff },
+    { TRAMP_SENTINEL_INSN }
+  },
+  sh_linux_sigreturn_init
+};
+
+static struct tramp_frame sh_linux_rt_sigreturn_tramp_frame = {
+  SIGTRAMP_FRAME,
+  2,
+  {
+    { SH_MOVW, 0xffff },
+    { SH_TRAP, 0xff00 }, /* #imm argument part filtered out.  */
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_NR_RT_SIGRETURN, 0xffff },
+    { TRAMP_SENTINEL_INSN }
+  },
+  sh_linux_rt_sigreturn_init
+};
+
 static void
 sh_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -84,13 +191,17 @@ sh_linux_init_abi (struct gdbarch_info i
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
 
-  /* Core files are supported for 32-bit SH only, at present.  */
+  /* Core files and signal handler frame unwinding are supported for
+     32-bit SH only, at present.  */
   if (info.bfd_arch_info->mach != bfd_mach_sh5)
     {
       struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
       tdep->core_gregmap = (struct sh_corefile_regmap *)gregs_table;
       tdep->core_fpregmap = (struct sh_corefile_regmap *)fpregs_table;
+
+      tramp_frame_prepend_unwinder (gdbarch, &sh_linux_sigreturn_tramp_frame);
+      tramp_frame_prepend_unwinder (gdbarch, &sh_linux_rt_sigreturn_tramp_frame);
     }
 }
 

Re: [PATCH][SH] Signal handler unwinding for SH-Linux

[Yao Qi]

On 04/29/2012 04:05 PM, Chung-Lin Tang wrote:
> The test results show improvement on many sighandler related tests, with
> no regressions. Some gdb.base/sigbpt.exp tests change from FAIL->KFAIL,
> though I'm not completely sure what that indicates.
Chung-Lin,
Not sure the comment at the beginning of gdb.base/sigbpt.exp is relevant
to your KFAILs.

# This test is known to tickle the following problems: kernel letting
# the inferior execute both the system call, and the instruction
# following, when single-stepping a system call; kernel failing to
# propogate the single-step state when single-stepping the sigreturn
# system call, instead resuming the inferior at full speed; GDB
# doesn't know how to software single-step across a sigreturn
# instruction.  Since the kernel problems can be "fixed" using
# software single-step this is KFAILed rather than XFAILed.

If this comment is related to your KFAILs, this can be handled by
computing the "next pc" of sigreturn or rt_sigreturn.  Please have a
look at functions {arm,mips,tic6x}_linux_syscall_next_pc.  You can do
something similar in SH backend.  Hope it helps.

-- 
Yao (齐尧)

Re: [PATCH][SH] Signal handler unwinding for SH-Linux

[Joel Brobecker]

Chung-Lin,
> 2012-04-29  Chung-Lin Tang  <cltang@codesourcery.com>
> 
> 	* sh-linux-tdep.c: Include trad-frame.h and tramp-frame.h.
> 	(sh_linux_sigtramp_cache): New function.
> 	(sh_linux_sigreturn_init): New function.
> 	(sh_linux_rt_sigreturn_init): New function.
> 	(SH_MOVW,SH_TRAP,SH_OR_R0_R0): New symbols for instruction
> 	patterns.
> 	(SH_NR_SIGRETURN,SH_NR_RT_SIGRETURN): New symbols for sigreturn
> 	syscall codes.
> 	(sh_linux_sigreturn_tramp_frame): New tramp_frame definition.
> 	(sh_linux_rt_sigreturn_tramp_frame): Likewise.
> 	(sh_linux_init_abi): Add init calls to register new tramp_frame
> 	definitions under 32-bit SH, update comments.

Overall, this looks good to me.  I do not know about SH specifically,
so I will take your word on the specific location and layout of the
sigcontext structure.

Just a small minor comment:  All new functions and globals should
be documented.  Usually, when we implement a "virtual method" (a
function destined to be used as a pointer in one of our generic
structures; Eg. sh_linux_sigreturn_init), we do not repeat description.
We just say that this function implements such and such callback
in struct bla bla for such and such situation. The actual documentation
should already be provided at the same location the field itself
is declared.

For instance, one possible description of sh_linux_sigreturn_init
is:

/* Implement struct tramp_frame's "init" callback for signal
   trampolines on 32-bit SH.  */

-- 
Joel

Re: [PATCH][SH] Signal handler unwinding for SH-Linux

[Chung-Lin Tang]

On 2012/5/3 05:51 AM, Joel Brobecker wrote:
> Overall, this looks good to me.  I do not know about SH specifically,
> so I will take your word on the specific location and layout of the
> sigcontext structure.
> 
> Just a small minor comment:  All new functions and globals should
> be documented.  Usually, when we implement a "virtual method" (a
> function destined to be used as a pointer in one of our generic
> structures; Eg. sh_linux_sigreturn_init), we do not repeat description.
> We just say that this function implements such and such callback
> in struct bla bla for such and such situation. The actual documentation
> should already be provided at the same location the field itself
> is declared.
> 
> For instance, one possible description of sh_linux_sigreturn_init
> is:
> 
> /* Implement struct tramp_frame's "init" callback for signal
>    trampolines on 32-bit SH.  */

Hi Joel, thanks for reviewing. I've committed the patch with minor
additional comments like you suggested.

Chung-Lin

Re: [PATCH][SH] Signal handler unwinding for SH-Linux

[Joel Brobecker]

> Hi Joel, thanks for reviewing. I've committed the patch with minor
> additional comments like you suggested.

Can you post the patch that you committed? We ask that the final version
always be posted, to keep a record of th epatch on the list, withouth
having to go dig for the patch is CVS...

Thanks,
-- 
Joel

Re: [PATCH][SH] Signal handler unwinding for SH-Linux

[Chung-Lin Tang]

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

On 2012/5/7 11:35 PM, Joel Brobecker wrote:
>> Hi Joel, thanks for reviewing. I've committed the patch with minor
>> additional comments like you suggested.
> 
> Can you post the patch that you committed? We ask that the final version
> always be posted, to keep a record of th epatch on the list, withouth
> having to go dig for the patch is CVS...
> 
> Thanks,

As attached, the output of cvs diff -u -p -r1.13 -r1.14 sh-linux-tdep.c

[-- Attachment #2: sh-sigh-unwind.diff --]
[-- Type: text/plain, Size: 4942 bytes --]

Index: sh-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-linux-tdep.c,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -p -r1.13 -r1.14
--- sh-linux-tdep.c	4 Jan 2012 08:17:10 -0000	1.13
+++ sh-linux-tdep.c	7 May 2012 13:17:29 -0000	1.14
@@ -23,6 +23,9 @@
 #include "solib-svr4.h"
 #include "symtab.h"
 
+#include "trad-frame.h"
+#include "tramp-frame.h"
+
 #include "glibc-tdep.h"
 #include "sh-tdep.h"
 #include "linux-tdep.h"
@@ -70,6 +73,113 @@ static const struct sh_corefile_regmap f
   {-1 /* Terminator.  */, 0}
 };
 
+/* SH signal handler frame support.  */
+
+static void
+sh_linux_sigtramp_cache (struct frame_info *this_frame,
+			 struct trad_frame_cache *this_cache,
+			 CORE_ADDR func, int regs_offset)
+{
+  int i;
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  CORE_ADDR base = get_frame_register_unsigned (this_frame,
+						gdbarch_sp_regnum (gdbarch));
+  CORE_ADDR regs = base + regs_offset;
+
+  for (i = 0; i < 18; i++)
+    trad_frame_set_reg_addr (this_cache, i, regs + i * 4);
+
+  trad_frame_set_reg_addr (this_cache, SR_REGNUM, regs + 18 * 4);
+  trad_frame_set_reg_addr (this_cache, GBR_REGNUM, regs + 19 * 4);
+  trad_frame_set_reg_addr (this_cache, MACH_REGNUM, regs + 20 * 4);
+  trad_frame_set_reg_addr (this_cache, MACL_REGNUM, regs + 21 * 4);
+
+  /* Restore FP state if we have an FPU.  */
+  if (gdbarch_fp0_regnum (gdbarch) != -1)
+    {
+      CORE_ADDR fpregs = regs + 22 * 4;
+      for (i = FR0_REGNUM; i <= FP_LAST_REGNUM; i++)
+	trad_frame_set_reg_addr (this_cache, i, fpregs + i * 4);
+      trad_frame_set_reg_addr (this_cache, FPSCR_REGNUM, fpregs + 32 * 4);
+      trad_frame_set_reg_addr (this_cache, FPUL_REGNUM, fpregs + 33 * 4);
+    }
+
+  /* Save a frame ID.  */
+  trad_frame_set_id (this_cache, frame_id_build (base, func));
+}
+
+/* Implement struct tramp_frame "init" callbacks for signal
+   trampolines on 32-bit SH.  */
+
+static void
+sh_linux_sigreturn_init (const struct tramp_frame *self,
+			 struct frame_info *this_frame,
+			 struct trad_frame_cache *this_cache,
+			 CORE_ADDR func)
+{
+  /* SH 32-bit sigframe: sigcontext at start of sigframe,
+     registers start after a single 'oldmask' word.  */
+  sh_linux_sigtramp_cache (this_frame, this_cache, func, 4);
+}
+
+static void
+sh_linux_rt_sigreturn_init (const struct tramp_frame *self,
+			    struct frame_info *this_frame,
+			    struct trad_frame_cache *this_cache,
+			    CORE_ADDR func)
+{
+  /* SH 32-bit rt_sigframe: starts with a siginfo (128 bytes), then
+     we can find sigcontext embedded within a ucontext (offset 20 bytes).
+     Then registers start after a single 'oldmask' word.  */
+  sh_linux_sigtramp_cache (this_frame, this_cache, func,
+			   128 /* sizeof (struct siginfo)  */
+			   + 20 /* offsetof (struct ucontext, uc_mcontext) */
+			   + 4 /* oldmask word at start of sigcontext */);
+}
+
+/* Instruction patterns.  */
+#define SH_MOVW     0x9305
+#define SH_TRAP     0xc300
+#define SH_OR_R0_R0 0x200b       
+
+/* SH sigreturn syscall numbers.  */
+#define SH_NR_SIGRETURN 0x0077
+#define SH_NR_RT_SIGRETURN 0x00ad
+
+static struct tramp_frame sh_linux_sigreturn_tramp_frame = {
+  SIGTRAMP_FRAME,
+  2,
+  {
+    { SH_MOVW, 0xffff },
+    { SH_TRAP, 0xff00 }, /* #imm argument part filtered out.  */
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_NR_SIGRETURN, 0xffff },
+    { TRAMP_SENTINEL_INSN }
+  },
+  sh_linux_sigreturn_init
+};
+
+static struct tramp_frame sh_linux_rt_sigreturn_tramp_frame = {
+  SIGTRAMP_FRAME,
+  2,
+  {
+    { SH_MOVW, 0xffff },
+    { SH_TRAP, 0xff00 }, /* #imm argument part filtered out.  */
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_OR_R0_R0, 0xffff },
+    { SH_NR_RT_SIGRETURN, 0xffff },
+    { TRAMP_SENTINEL_INSN }
+  },
+  sh_linux_rt_sigreturn_init
+};
+
 static void
 sh_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -84,13 +194,17 @@ sh_linux_init_abi (struct gdbarch_info i
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
 
-  /* Core files are supported for 32-bit SH only, at present.  */
+  /* Core files and signal handler frame unwinding are supported for
+     32-bit SH only, at present.  */
   if (info.bfd_arch_info->mach != bfd_mach_sh5)
     {
       struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
       tdep->core_gregmap = (struct sh_corefile_regmap *)gregs_table;
       tdep->core_fpregmap = (struct sh_corefile_regmap *)fpregs_table;
+
+      tramp_frame_prepend_unwinder (gdbarch, &sh_linux_sigreturn_tramp_frame);
+      tramp_frame_prepend_unwinder (gdbarch, &sh_linux_rt_sigreturn_tramp_frame);
     }
 }