RFC: Updates support for breakpoints that generate SIGILL

[Daniel Jacobowitz <dan at codesourcery dot com>]

GDB has several bits of code which claim to handle breakpoints that
generate SIGILL, SIGEMT (?), or SIGSEGV.  I needed to use this code
for testing; I have a patch coming up which requires a new breakpoint
instruction on ARM targets, and existing kernels report SIGILL
on that instruction.

I've submitted a kernel patch to add the new breakpoint to the list
that generate SIGTRAP.  So, eventually, I won't care about this patch
any more.  But I think it's worthwhile anyway, unless there's a
problem with it I haven't found.

I didn't touch linux-nat.  It will probably have similar problems to
gdbserver for multi-threaded programs, but I can't test it.  I claim
that this patch is strictly an improvement over the status quo.

Any thoughts about this patch, or shall I check it in?  Tested on
arm-linux-gnueabi using gdbserver, both with and without SIGILLs.

-- 
Daniel Jacobowitz
CodeSourcery

2010-01-28  Daniel Jacobowitz  <dan@codesourcery.com>

	gdb/
	* infrun.c (prepare_to_proceed): Handle other signals which might
	match a breakpoint.
	(handle_inferior_event): Move the check for unusual breakpoint
	signals earlier.

	gdb/gdbserver/
	* linux-low.c (get_stop_pc): Check for SIGTRAP.
	(linux_wait_for_event_1): Handle SIGILL and SIGSEGV as possible
	breakpoints.

---
 gdb/gdbserver/linux-low.c |   21 ++++++++++++++-----
 gdb/infrun.c              |   50 ++++++++++++++++++++++++++--------------------
 2 files changed, 45 insertions(+), 26 deletions(-)

Index: gdb-mainline/gdb/infrun.c
===================================================================
--- gdb-mainline.orig/gdb/infrun.c	2010-01-28 13:30:18.000000000 -0800
+++ gdb-mainline/gdb/infrun.c	2010-01-28 13:31:22.000000000 -0800
@@ -1642,7 +1642,10 @@ prepare_to_proceed (int step)
 
   /* Make sure we were stopped at a breakpoint.  */
   if (wait_status.kind != TARGET_WAITKIND_STOPPED
-      || wait_status.value.sig != TARGET_SIGNAL_TRAP)
+      || (wait_status.value.sig != TARGET_SIGNAL_TRAP
+	  && wait_status.value.sig != TARGET_SIGNAL_ILL
+	  && wait_status.value.sig != TARGET_SIGNAL_SEGV
+	  && wait_status.value.sig != TARGET_SIGNAL_EMT))
     {
       return 0;
     }
@@ -2673,6 +2676,7 @@ handle_inferior_event (struct execution_
 {
   struct frame_info *frame;
   struct gdbarch *gdbarch;
+  struct regcache *regcache;
   int sw_single_step_trap_p = 0;
   int stopped_by_watchpoint;
   int stepped_after_stopped_by_watchpoint = 0;
@@ -2733,6 +2737,30 @@ handle_inferior_event (struct execution_
 
   breakpoint_retire_moribund ();
 
+  /* First, distinguish signals caused by the debugger from signals
+     that have to do with the program's own actions.  Note that
+     breakpoint insns may cause SIGTRAP or SIGILL or SIGEMT, depending
+     on the operating system version.  Here we detect when a SIGILL or
+     SIGEMT is really a breakpoint and change it to SIGTRAP.  We do
+     something similar for SIGSEGV, since a SIGSEGV will be generated
+     when we're trying to execute a breakpoint instruction on a
+     non-executable stack.  This happens for call dummy breakpoints
+     for architectures like SPARC that place call dummies on the
+     stack.  */
+  regcache = get_thread_regcache (ecs->ptid);
+  if (ecs->ws.kind == TARGET_WAITKIND_STOPPED
+      && (ecs->ws.value.sig == TARGET_SIGNAL_ILL
+	  || ecs->ws.value.sig == TARGET_SIGNAL_SEGV
+	  || ecs->ws.value.sig == TARGET_SIGNAL_EMT)
+      && breakpoint_inserted_here_p (get_regcache_aspace (regcache),
+				     regcache_read_pc (regcache)))
+    {
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: Treating signal as SIGTRAP\n");
+      ecs->ws.value.sig = TARGET_SIGNAL_TRAP;
+    }
+
   /* Mark the non-executing threads accordingly.  In all-stop, all
      threads of all processes are stopped when we get any event
      reported.  In non-stop mode, only the event thread stops.  If
@@ -3512,27 +3540,7 @@ targets should add new threads to the th
      3) set ecs->random_signal to 1, and the decision between 1 and 2
      will be made according to the signal handling tables.  */
 
-  /* First, distinguish signals caused by the debugger from signals
-     that have to do with the program's own actions.  Note that
-     breakpoint insns may cause SIGTRAP or SIGILL or SIGEMT, depending
-     on the operating system version.  Here we detect when a SIGILL or
-     SIGEMT is really a breakpoint and change it to SIGTRAP.  We do
-     something similar for SIGSEGV, since a SIGSEGV will be generated
-     when we're trying to execute a breakpoint instruction on a
-     non-executable stack.  This happens for call dummy breakpoints
-     for architectures like SPARC that place call dummies on the
-     stack.
-
-     If we're doing a displaced step past a breakpoint, then the
-     breakpoint is always inserted at the original instruction;
-     non-standard signals can't be explained by the breakpoint.  */
   if (ecs->event_thread->stop_signal == TARGET_SIGNAL_TRAP
-      || (! ecs->event_thread->trap_expected
-          && breakpoint_inserted_here_p (get_regcache_aspace (get_current_regcache ()),
-					 stop_pc)
-	  && (ecs->event_thread->stop_signal == TARGET_SIGNAL_ILL
-	      || ecs->event_thread->stop_signal == TARGET_SIGNAL_SEGV
-	      || ecs->event_thread->stop_signal == TARGET_SIGNAL_EMT))
       || stop_soon == STOP_QUIETLY || stop_soon == STOP_QUIETLY_NO_SIGSTOP
       || stop_soon == STOP_QUIETLY_REMOTE)
     {
Index: gdb-mainline/gdb/gdbserver/linux-low.c
===================================================================
--- gdb-mainline.orig/gdb/gdbserver/linux-low.c	2010-01-28 12:21:54.000000000 -0800
+++ gdb-mainline/gdb/gdbserver/linux-low.c	2010-01-28 13:31:39.000000000 -0800
@@ -448,7 +448,8 @@ get_stop_pc (void)
 {
   CORE_ADDR stop_pc = (*the_low_target.get_pc) ();
 
-  if (! get_thread_lwp (current_inferior)->stepping)
+  if (! get_thread_lwp (current_inferior)->stepping
+      && WSTOPSIG (get_thread_lwp (current_inferior)->last_status) == SIGTRAP)
     stop_pc -= the_low_target.decr_pc_after_break;
 
   if (debug_threads)
@@ -1233,18 +1234,28 @@ linux_wait_for_event_1 (ptid_t ptid, int
 	  continue;
 	}
 
-      /* If this event was not handled above, and is not a SIGTRAP, report
-	 it.  */
-      if (!WIFSTOPPED (*wstat) || WSTOPSIG (*wstat) != SIGTRAP)
+      /* If this event was not handled above, and is not a SIGTRAP,
+	 report it.  SIGILL and SIGSEGV are also treated as traps in case
+	 a breakpoint is inserted at the current PC.  */
+      if (!WIFSTOPPED (*wstat)
+	  || (WSTOPSIG (*wstat) != SIGTRAP && WSTOPSIG (*wstat) != SIGILL
+	      && WSTOPSIG (*wstat) != SIGSEGV))
 	return lwpid_of (event_child);
 
       /* If this target does not support breakpoints, we simply report the
-	 SIGTRAP; it's of no concern to us.  */
+	 signal; it's of no concern to us.  */
       if (the_low_target.get_pc == NULL)
 	return lwpid_of (event_child);
 
       stop_pc = get_stop_pc ();
 
+      /* Only handle SIGILL or SIGSEGV if we've hit a recognized
+	 breakpoint.  */
+      if (WSTOPSIG (*wstat) != SIGTRAP
+	  && (event_child->stepping
+	      || ! (*the_low_target.breakpoint_at) (stop_pc)))
+	return lwpid_of (event_child);
+
       /* bp_reinsert will only be set if we were single-stepping.
 	 Notice that we will resume the process after hitting
 	 a gdbserver breakpoint; single-stepping to/over one