RFC: mips-linux software single step versus sigreturn

RFC: mips-linux software single step versus sigreturn

[Daniel Jacobowitz]

This patch lets mips32_next_pc call an architecure-specific hook to work out
the next PC for a SYSCALL instruction.  mips-linux supplies the hook,
and uses unwinding to find the resume address for a sigreturn syscall.
This prevents a bogus breakpoint from being set in the struct sigcontext
when single-stepping out of a trampoline; in the process, it fixes
all of the easily fixable sigstep.exp failures on mips-linux.

The remaining failures come from the fact that software single-step can't
be used to step into a signal handler.  We can work out in the target code
whether or not there is a signal handler, but not its address.  Kernel
changes to support this could be made; heck, we could theoretically call
sigaction () ourselves to find the address, but that's above and beyond the
call of duty - probably too complex to bother with given the current state
of GDB's execution control mechanisms.

Any comments?  I'll commit this in a few days if there are no objections to
the approach.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-04-02  Daniel Jacobowitz  <dan@codesourcery.com>

	* mips-linux-tdep.c (mips_linux_syscall_next_pc): New function.
	(mips_linux_os_methods): New variable.
	(mips_linux_init_abi): Call mips_set_os_methods.
	* mips-tdep.c (struct gdbarch_tdep): Add os_methods.
	(mips32_next_pc): Use the syscall_next_pc method.
	(mips_set_os_methods): New function.
	* mips-tdep.h (struct mips_os_methods): New.
	(mips_set_os_methods): New prototype.

Index: mips-linux-tdep.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/mips-linux-tdep.c,v
retrieving revision 1.39
diff -u -p -r1.39 mips-linux-tdep.c
--- mips-linux-tdep.c	31 Mar 2005 19:58:25 -0000	1.39
+++ mips-linux-tdep.c	3 Apr 2005 03:52:35 -0000
@@ -1151,6 +1168,26 @@ mips_linux_n32n64_sigframe_init (const s
 				     func));
 }
 
+/* If the current instruction is SYSCALL, return the PC of the next
+   instruction to be executed.  */
+CORE_ADDR
+mips_linux_syscall_next_pc (CORE_ADDR pc)
+{
+  CORE_ADDR v0 = read_register (MIPS_V0_REGNUM);
+
+  if (v0 == MIPS_NR_sigreturn || v0 == MIPS_NR_rt_sigreturn
+      || v0 == MIPS_NR_N64_rt_sigreturn
+      || v0 == MIPS_NR_N32_rt_sigreturn)
+    return frame_pc_unwind (get_current_frame ());
+
+  return pc + 4;
+}
+
+const struct mips_os_methods mips_linux_os_methods =
+{
+  mips_linux_syscall_next_pc
+};
+
 /* Initialize one of the GNU/Linux OS ABIs.  */
 
 static void
@@ -1199,6 +1236,8 @@ mips_linux_init_abi (struct gdbarch_info
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
+
+  mips_set_os_methods (gdbarch, &mips_linux_os_methods);
 }
 
 void
Index: mips-tdep.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/mips-tdep.c,v
retrieving revision 1.379
diff -u -p -r1.379 mips-tdep.c
--- mips-tdep.c	17 Mar 2005 18:07:46 -0000	1.379
+++ mips-tdep.c	3 Apr 2005 04:10:45 -0000
@@ -141,6 +141,9 @@ struct gdbarch_tdep
   const struct mips_regnum *regnum;
   /* Register names table for the current register set.  */
   const char **mips_processor_reg_names;
+
+  /* Methods provided by the OS/ABI.  */
+  const struct mips_os_methods *os_methods;
 };
 
 static int
@@ -948,6 +951,16 @@ mips32_next_pc (CORE_ADDR pc)
 	      /* Set PC to that address */
 	      pc = read_signed_register (rtype_rs (inst));
 	      break;
+	    case 12:		/* SYSCALL */
+	      {
+		const struct mips_os_methods *os_methods;
+		os_methods = gdbarch_tdep (current_gdbarch)->os_methods;
+		if (os_methods && os_methods->syscall_next_pc)
+		  pc = (*os_methods->syscall_next_pc) (pc);
+		else
+		  pc += 4;
+	      }
+	      break;
 	    default:
 	      pc += 4;
 	    }
@@ -4652,6 +4665,15 @@ global_mips_abi (void)
   internal_error (__FILE__, __LINE__, _("unknown ABI string"));
 }
 
+/* Set the OS/ABI specific support methods for this architecture.  */
+
+void
+mips_set_os_methods (struct gdbarch *gdbarch,
+		     const struct mips_os_methods *methods)
+{
+  gdbarch_tdep (gdbarch)->os_methods = methods;
+}
+
 static struct gdbarch *
 mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
Index: mips-tdep.h
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/mips-tdep.h,v
retrieving revision 1.17
diff -u -p -r1.17 mips-tdep.h
--- mips-tdep.h	30 Oct 2004 23:22:53 -0000	1.17
+++ mips-tdep.h	3 Apr 2005 03:50:50 -0000
@@ -1,6 +1,6 @@
 /* Target-dependent header for the MIPS architecture, for GDB, the GNU Debugger.
 
-   Copyright 2002, 2003 Free Software Foundation, Inc.
+   Copyright 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -102,6 +102,14 @@ enum
   MIPS_NUMREGS = 32
 };
 
+/* OS/ABI specific methods that a target can supply.  */
+struct mips_os_methods
+{
+  CORE_ADDR (*syscall_next_pc) (CORE_ADDR);
+};
+
+void mips_set_os_methods (struct gdbarch *, const struct mips_os_methods *);
+
 /* Single step based on where the current instruction will take us.  */
 extern void mips_software_single_step (enum target_signal, int);
 

Re: RFC: mips-linux software single step versus sigreturn

[Mark Kettenis]

   Date: Sat, 2 Apr 2005 23:25:54 -0500
   From: Daniel Jacobowitz <drow@false.org>

   Any comments?

Is the extra indirection via os_methods really necessary?

Mark

Re: RFC: mips-linux software single step versus sigreturn

[Daniel Jacobowitz]

On Sun, Apr 03, 2005 at 09:18:24PM +0200, Mark Kettenis wrote:
>    Date: Sat, 2 Apr 2005 23:25:54 -0500
>    From: Daniel Jacobowitz <drow@false.org>
> 
>    Any comments?
> 
> Is the extra indirection via os_methods really necessary?

I'm not thrilled with it either, but when Andrew maintained the MIPS
port, he expressed opposition to moving MIPS's gdbarch_tdep to
mips-tdep.h.  The original patch was here:
  http://sources.redhat.com/ml/gdb-patches/2002-10/msg00119.html
The objection was here:
  http://sources.redhat.com/ml/gdb-patches/2002-11/msg00038.html

If the consensus has swung the other way, I'll revise...

-- 
Daniel Jacobowitz
CodeSourcery, LLC