[PATCH]: Fix signal handler stepping on Linux/Sparc

[PATCH]: Fix signal handler stepping on Linux/Sparc

[David S. Miller]

When we step over the system call trap instruction, GDB does
not currently step correctly if this for a sigreturn system
call on Linux/SPARC.

Thankfully the sparc generic tdep code has a way to support
this cleanly, by providing a software single step handler for
trap instructions that the target can override.

It seems that perhaps some other Linux targets don't handle this right
too.

MIPS is the most likely platform that gets this wrong in the same
exact way, as I don't see any handling of system call instructions in
the software single-step handler which MIPS/Linux uses.

Alpha I think is in the same boat as MIPS and would need handling in
the software single-step code to get this right.  It just assumes that
the next PC is PC + 4 if it isn't looking at a branch or a jump.

Other platforms, such as i386, s390, and I believe ppc, have hardware
single stepping so don't need explicit support like this for stepping
over a sigreturn() system call properly.

Anyways, this gets most of gdb.base/sigstep.exp to pass on
Linux/Sparc.  There are still a few failures in that testcase, which I
need to look into.

Tested sparc-linux-gnu and sparc64-linux-gnu.

Ok to apply?

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

	* sparc-linux-tdep.c (sparc32_linux_step_trap): New.
	(sparc32_linux_init_abi): Hook it into tdep->step_trap.
	* sparc64-linux-tdep.c (sparc64_linux_step_trap): New.
	(sparc64_linux_init_abi): Hook it into tdep->step_trap.
	* Makefile.in: Update dependencies.

--- ./sparc-linux-tdep.c.~1~	2006-04-05 13:27:08.000000000 -0700
+++ ./sparc-linux-tdep.c	2006-04-07 22:48:00.000000000 -0700
@@ -126,6 +126,49 @@ sparc32_linux_sigframe_init (const struc
   trad_frame_set_id (this_cache, frame_id_build (base, func));
 }
 \f
+/* Return the address of a system call's alternative return
+   address.  */
+
+static CORE_ADDR
+sparc32_linux_step_trap (unsigned long insn)
+{
+  if (insn == 0x91d02010)
+    {
+      ULONGEST sc_num;
+
+      regcache_cooked_read_unsigned (current_regcache,
+				     SPARC_G1_REGNUM, &sc_num);
+
+      /* __NR_rt_sigreturn is 101 and __NR_sigreturn is 216  */
+      if (sc_num == 101 || sc_num == 216)
+	{
+	  ULONGEST sp, pc_offset;
+
+	  regcache_cooked_read_unsigned (current_regcache,
+					 SPARC_SP_REGNUM, &sp);
+
+	  /* The kernel puts the sigreturn registers on the stack,
+	     and this is where the signal unwinding state is take from
+	     when returning from a signal.
+
+	     For __NR_sigreturn, this register area sits 96 bytes from
+	     the base of the stack.  The saved PC sits 4 bytes into the
+	     sigreturn register save area.
+
+	     For __NR_rt_sigreturn a siginfo_t, which is 128 bytes, sits
+	     right before the sigreturn register save area.  */
+
+	  pc_offset = 96 + 4;
+	  if (sc_num == 101)
+	    pc_offset += 128;
+
+	  return read_memory_unsigned_integer (sp + pc_offset, 4);
+	}
+    }
+
+  return 0;
+}
+\f
 
 static void
 sparc32_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
@@ -152,6 +195,9 @@ sparc32_linux_init_abi (struct gdbarch_i
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
 
+  /* Make sure we can single-step over signal return system calls.  */
+  tdep->step_trap = sparc32_linux_step_trap;
+
   /* Hook in the DWARF CFI frame unwinder.  */
   frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
 }
--- ./sparc64-linux-tdep.c.~1~	2006-04-05 12:19:12.000000000 -0700
+++ ./sparc64-linux-tdep.c	2006-04-07 23:03:14.000000000 -0700
@@ -22,7 +22,9 @@
 #include "defs.h"
 #include "frame.h"
 #include "frame-unwind.h"
+#include "regcache.h"
 #include "gdbarch.h"
+#include "gdbcore.h"
 #include "osabi.h"
 #include "solib-svr4.h"
 #include "symtab.h"
@@ -98,6 +100,36 @@ sparc64_linux_sigframe_init (const struc
   trad_frame_set_id (this_cache, frame_id_build (base, func));
 }
 \f
+/* Return the address of a system call's alternative return
+   address.  */
+
+static CORE_ADDR
+sparc64_linux_step_trap (unsigned long insn)
+{
+  if (insn == 0x91d0206d)
+    {
+      ULONGEST sp;
+
+      regcache_cooked_read_unsigned (current_regcache,
+				     SPARC_SP_REGNUM, &sp);
+      if (sp & 1)
+	sp += BIAS;
+
+      /* The kernel puts the sigreturn registers on the stack,
+	 and this is where the signal unwinding state is take from
+	 when returning from a signal.
+
+	 A siginfo_t sits 192 bytes from the base of the stack.  This
+	 siginfo_t is 128 bytes, and is followed by the sigreturn
+	 register save area.  The saved PC sits at a 136 byte offset
+	 into there.  */
+
+      return read_memory_unsigned_integer (sp + 192 + 128 + 136, 8);
+    }
+
+  return 0;
+}
+\f
 
 static void
 sparc64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
@@ -120,6 +152,9 @@ sparc64_linux_init_abi (struct gdbarch_i
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
+
+  /* Make sure we can single-step over signal return system calls.  */
+  tdep->step_trap = sparc64_linux_step_trap;
 }
 \f
 
--- ./Makefile.in.~1~	2006-04-05 14:51:11.000000000 -0700
+++ ./Makefile.in	2006-04-07 23:03:32.000000000 -0700
@@ -2616,8 +2616,9 @@ sparc64-linux-nat.o: sparc64-linux-nat.c
 	$(gregset_h) $(sparc64_tdep_h) $(sparc_tdep_h) \
 	$(sparc_nat_h) $(inferior_h) $(target_h) $(linux_nat_h)
 sparc64-linux-tdep.o: sparc64-linux-tdep.c $(defs_h) $(frame_h) \
-	$(frame_unwind_h) $(gdbarch_h) $(osabi_h) $(solib_svr4_h) \
-	$(symtab_h) $(trad_frame_h) $(tramp_frame_h) $(sparc64_tdep_h)
+	$(frame_unwind_h) $(regcache_h) $(gdbarch_h) $(gdbcore_h) \
+	$(osabi_h) $(solib_svr4_h) $(symtab_h) $(trad_frame_h) \
+	$(tramp_frame_h) $(sparc64_tdep_h)
 sparc64-nat.o: sparc64-nat.c $(defs_h) $(gdbarch_h) $(sparc64_tdep_h) \
 	$(sparc_nat_h)
 sparc64nbsd-nat.o: sparc64nbsd-nat.c $(defs_h) $(regcache_h) $(target_h) \

Re: [PATCH]: Fix signal handler stepping on Linux/Sparc

[Daniel Jacobowitz]

On Fri, Apr 07, 2006 at 11:18:43PM -0700, David S. Miller wrote:
> It seems that perhaps some other Linux targets don't handle this right
> too.
> 
> MIPS is the most likely platform that gets this wrong in the same
> exact way, as I don't see any handling of system call instructions in
> the software single-step handler which MIPS/Linux uses.

I know that I have fixed this for MIPS before.  I may have not followed
through on the patch, though.  ARM has a similar problem.  Those two
I'll fix at some point; Alpha I'm probably going to leave for someone
who has more need for the port...

> 2006-04-07  David S. Miller  <davem@sunset.davemloft.net>
> 
> 	* sparc-linux-tdep.c (sparc32_linux_step_trap): New.
> 	(sparc32_linux_init_abi): Hook it into tdep->step_trap.
> 	* sparc64-linux-tdep.c (sparc64_linux_step_trap): New.
> 	(sparc64_linux_init_abi): Hook it into tdep->step_trap.
> 	* Makefile.in: Update dependencies.

Yes, this is fine.  Thanks.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH]: Fix signal handler stepping on Linux/Sparc

[David S. Miller]

From: Daniel Jacobowitz <drow@false.org>
Date: Sat, 8 Apr 2006 16:29:00 -0400

> On Fri, Apr 07, 2006 at 11:18:43PM -0700, David S. Miller wrote:
> > It seems that perhaps some other Linux targets don't handle this right
> > too.
> > 
> > MIPS is the most likely platform that gets this wrong in the same
> > exact way, as I don't see any handling of system call instructions in
> > the software single-step handler which MIPS/Linux uses.
> 
> I know that I have fixed this for MIPS before.  I may have not followed
> through on the patch, though.  ARM has a similar problem.  Those two
> I'll fix at some point; Alpha I'm probably going to leave for someone
> who has more need for the port...

Someone like Richard Henderson should be able to cook up a fix for
Alpha without much effort, so hopefully someone like him will get
around to it at some point.

> > 2006-04-07  David S. Miller  <davem@sunset.davemloft.net>
> > 
> > 	* sparc-linux-tdep.c (sparc32_linux_step_trap): New.
> > 	(sparc32_linux_init_abi): Hook it into tdep->step_trap.
> > 	* sparc64-linux-tdep.c (sparc64_linux_step_trap): New.
> > 	(sparc64_linux_init_abi): Hook it into tdep->step_trap.
> > 	* Makefile.in: Update dependencies.
> 
> Yes, this is fine.  Thanks.

Thanks a lot for reviewing, committed.