[rfc][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[rfc][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Hello,

several targets today implement a performance optimization where some signals
are not reported back to the GDB core, but instead are directly delivered to
the target; this can be done either by not even trapping those signals (e.g.
procfs targets), by restarting the inferior directly in target code (e.g.
linux-nat), or by restarting the inferior in the remote stub.

This works via a target callback to_notice_signal, which is called whenever
the list of signals to be handled by GDB changes.  Target code is then
expected to itself detect which signals are safe to short-circuit (by calling
signal_pass_state) etc.  Target code must also handle (implicitly defined)
special cases, e.g. that signals must always be reported during startup
or when single-stepping.

It is this last condition that causes problems on architectures that use
software single-step, because target code isn't even aware of that
condition in general.  This causes failures on ARM, see the discussion here:
http://sourceware.org/ml/gdb-patches/2011-01/msg00398.html

The solution that was discussed there is basically to make the implicit
rules of signal passing shortcuts explicit.  The patch below is an
attempt to implement this idea.  The basic concept is to remove the
to_notice_signals callback and replace it by a to_pass_signals callback,
which explicitly gets a list of signals that can be safely bypassed
at this point.

Common code then calls this new target callback as appropriate, in
particular to mark all signals non-bypassable when single-stepping.

All platforms using to_notice_signals (procfs, nto-procfs, remote)
or directly accessing signal_pass_state etc (linux-nat) are updated
to instead use the to_pass_signals callback.

Tested on i386-linux, both native and gdbserver, with no regressions.
Fixes the same set of failures on ARM as the original patch.

Any thoughts on this approach?

I'm unable to test on any procfs or nto-procfs target; I'd much
appreciate if anyone with access to those targets could help me out here ...

Thanks,
Ulrich


ChangeLog:

	* target.h (struct target_ops): Remove to_notice_signals;
	add to_pass_signals.
	(target_notice_signals): Remove.
	(target_pass_signals): Add prototype.
	* target.c (update_current_target): Remove to_notice_signals;
	mention to_pass_signals.
	(target_pass_signals): New function.
	(debug_to_notice_signals): Remove.
	(setup_target_debug): Do not install debug_to_notice_signals.

	* infrun.c (signal_pass): New global.
	(resume): Call target_pass_signals.
	(signal_cache_update): New function.
	(signal_stop_update): Call it.
	(signal_print_update): Likewise.
	(signal_pass_update): Likewise.
	(handle_command): Call signal_cache_update and target_pass_signals
	instead of target_notice_signals.
	(_initialize_infrun): Initialize signal_pass.

	* linux-nat.c (pass_mask): New global.
	(linux_nat_pass_signals): New function.
	(linux_nat_create_inferior): Report all signals initially.
	(linux_nat_attach): Likewise.
	(linux_nat_resume): Use pass_mask to decide whether to directly
	handle an inferior signal.
	(linux_nat_wait_1): Likewise.
	(linux_nat_add_target): Install to_pass_signals callback.

	* nto-procfs.c (notice_signals): Remove.
	(procfs_resume): Do not call notice_signals.
	(procfs_notice_signals): Remove.
	(procfs_pass_signals): New function.
	(init_procfs_ops): Install to_pass_signals callback instead of
	to_notice_signals callback.
	(_initialize_procfs): Report all signals initially.

	* procfs.c (procfs_notice_signals): Remove.
	(procfs_pass_signals): New function.
	(procfs_target): Install to_pass_signals callback instead of
	to_notice_signals callback.
	(register_gdb_signals): Remove.
	(procfs_debug_inferior): Report all signals initially.
	(procfs_init_inferior): Remove redundant register_gdb_signals call.

	* remote.c (remote_pass_signals): Add numsigs and pass_signals
	parameters; use them instead of calling signal_..._state routines.
	(remote_notice_signals): Remove.
	(remote_start_remote): Report all signals initially.
	(remote_resume): Do not call remote_pass_signals.
	(_initialize_remote): Install to_pass_signals callback instead of
	to_notice_signals callback.


Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.472
diff -u -p -r1.472 infrun.c
--- gdb/infrun.c	7 Mar 2011 16:03:00 -0000	1.472
+++ gdb/infrun.c	18 Mar 2011 17:23:18 -0000
@@ -276,6 +276,11 @@ static unsigned char *signal_stop;
 static unsigned char *signal_print;
 static unsigned char *signal_program;
 
+/* Table of signals that the target may silently handle.
+   This is automatically determined from the flags above,
+   and simply cached here.  */
+static unsigned char *signal_pass;
+
 #define SET_SIGS(nsigs,sigs,flags) \
   do { \
     int signum = (nsigs); \
@@ -1787,6 +1792,13 @@ a command like `return' or `jump' to con
 	 happens to apply to another thread.  */
       tp->suspend.stop_signal = TARGET_SIGNAL_0;
 
+      /* Advise target which signals may be handled silently.  If we are
+	 currently stepping, we need to receive all signals.  */
+      if (step || singlestep_breakpoints_inserted_p)
+	target_pass_signals (0, NULL);
+      else
+	target_pass_signals ((int) TARGET_SIGNAL_LAST, signal_pass);
+
       target_resume (resume_ptid, step, sig);
     }
 
@@ -5853,12 +5865,29 @@ signal_pass_state (int signo)
   return signal_program[signo];
 }
 
+static void
+signal_cache_update (int signo)
+{
+  if (signo == -1)
+    {
+      for (signo = 0; signo < (int) TARGET_SIGNAL_LAST; signo++)
+	signal_cache_update (signo);
+
+      return;
+    }
+
+  signal_pass[signo] = (signal_stop[signo] == 0
+			&& signal_print[signo] == 0
+			&& signal_program[signo] == 1);
+}
+
 int
 signal_stop_update (int signo, int state)
 {
   int ret = signal_stop[signo];
 
   signal_stop[signo] = state;
+  signal_cache_update (signo);
   return ret;
 }
 
@@ -5868,6 +5897,7 @@ signal_print_update (int signo, int stat
   int ret = signal_print[signo];
 
   signal_print[signo] = state;
+  signal_cache_update (signo);
   return ret;
 }
 
@@ -5877,6 +5907,7 @@ signal_pass_update (int signo, int state
   int ret = signal_program[signo];
 
   signal_program[signo] = state;
+  signal_cache_update (signo);
   return ret;
 }
 
@@ -6068,7 +6099,8 @@ Are you sure you want to change it? "),
   for (signum = 0; signum < nsigs; signum++)
     if (sigs[signum])
       {
-	target_notice_signals (inferior_ptid);
+	signal_cache_update (-1);
+	target_pass_signals ((int) TARGET_SIGNAL_LAST, signal_pass);
 
 	if (from_tty)
 	  {
@@ -6925,6 +6957,8 @@ leave it stopped or free to run as neede
     xmalloc (sizeof (signal_print[0]) * numsigs);
   signal_program = (unsigned char *)
     xmalloc (sizeof (signal_program[0]) * numsigs);
+  signal_pass = (unsigned char *)
+    xmalloc (sizeof (signal_program[0]) * numsigs);
   for (i = 0; i < numsigs; i++)
     {
       signal_stop[i] = 1;
@@ -6968,6 +7002,9 @@ leave it stopped or free to run as neede
   signal_stop[TARGET_SIGNAL_CANCEL] = 0;
   signal_print[TARGET_SIGNAL_CANCEL] = 0;
 
+  /* Update cached state.  */
+  signal_cache_update (-1);
+
   add_setshow_zinteger_cmd ("stop-on-solib-events", class_support,
 			    &stop_on_solib_events, _("\
 Set stopping for shared library events."), _("\
Index: gdb/linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.199
diff -u -p -r1.199 linux-nat.c
--- gdb/linux-nat.c	9 Mar 2011 12:48:55 -0000	1.199
+++ gdb/linux-nat.c	18 Mar 2011 17:23:19 -0000
@@ -1078,6 +1078,26 @@ restore_child_signals_mask (sigset_t *pr
 {
   sigprocmask (SIG_SETMASK, prev_mask, NULL);
 }
+
+/* Mask of signals to pass directly to the inferior.  */
+static sigset_t pass_mask;
+
+/* Update signals to pass to the inferior.  */
+static void
+linux_nat_pass_signals (int numsigs, unsigned char *pass_signals)
+{
+  int signo;
+
+  sigemptyset (&pass_mask);
+
+  for (signo = 1; signo < NSIG; signo++)
+    {
+      int target_signo = target_signal_from_host (signo);
+      if (target_signo < numsigs && pass_signals[target_signo])
+        sigaddset (&pass_mask, signo);
+    }
+}
+
 \f
 
 /* Prototypes for local functions.  */
@@ -1543,6 +1563,9 @@ linux_nat_create_inferior (struct target
     }
 #endif /* HAVE_PERSONALITY */
 
+  /* Make sure we report all signals during startup.  */
+  linux_nat_pass_signals (0, NULL);
+
   linux_ops->to_create_inferior (ops, exec_file, allargs, env, from_tty);
 
 #ifdef HAVE_PERSONALITY
@@ -1564,6 +1587,9 @@ linux_nat_attach (struct target_ops *ops
   int status;
   ptid_t ptid;
 
+  /* Make sure we report all signals during attach.  */
+  linux_nat_pass_signals (0, NULL);
+
   linux_ops->to_attach (ops, args, from_tty);
 
   /* The ptrace base target adds the main thread with (pid,0,0)
@@ -1922,19 +1948,8 @@ linux_nat_resume (struct target_ops *ops
 
   if (lp->status && WIFSTOPPED (lp->status))
     {
-      enum target_signal saved_signo;
-      struct inferior *inf;
-
-      inf = find_inferior_pid (ptid_get_pid (lp->ptid));
-      gdb_assert (inf);
-      saved_signo = target_signal_from_host (WSTOPSIG (lp->status));
-
-      /* Defer to common code if we're gaining control of the
-	 inferior.  */
-      if (inf->control.stop_soon == NO_STOP_QUIETLY
-	  && signal_stop_state (saved_signo) == 0
-	  && signal_print_state (saved_signo) == 0
-	  && signal_pass_state (saved_signo) == 1)
+      if (WSTOPSIG (lp->status)
+	  && sigismember (&pass_mask, WSTOPSIG (lp->status)))
 	{
 	  if (debug_linux_nat)
 	    fprintf_unfiltered (gdb_stdlog,
@@ -1944,7 +1959,7 @@ linux_nat_resume (struct target_ops *ops
 	  /* FIXME: What should we do if we are supposed to continue
 	     this thread with a signal?  */
 	  gdb_assert (signo == TARGET_SIGNAL_0);
-	  signo = saved_signo;
+	  signo = target_signal_from_host (WSTOPSIG (lp->status));
 	  lp->status = 0;
 	}
     }
@@ -3590,20 +3605,8 @@ retry:
   if (WIFSTOPPED (status))
     {
       enum target_signal signo = target_signal_from_host (WSTOPSIG (status));
-      struct inferior *inf;
-
-      inf = find_inferior_pid (ptid_get_pid (lp->ptid));
-      gdb_assert (inf);
 
-      /* Defer to common code if we get a signal while
-	 single-stepping, since that may need special care, e.g. to
-	 skip the signal handler, or, if we're gaining control of the
-	 inferior.  */
-      if (!lp->step
-	  && inf->control.stop_soon == NO_STOP_QUIETLY
-	  && signal_stop_state (signo) == 0
-	  && signal_print_state (signo) == 0
-	  && signal_pass_state (signo) == 1)
+      if (WSTOPSIG (status) && sigismember (&pass_mask, WSTOPSIG (status)))
 	{
 	  /* FIMXE: kettenis/2001-06-06: Should we resume all threads
 	     here?  It is not clear we should.  GDB may not expect
@@ -5712,6 +5715,7 @@ linux_nat_add_target (struct target_ops 
   t->to_detach = linux_nat_detach;
   t->to_resume = linux_nat_resume;
   t->to_wait = linux_nat_wait;
+  t->to_pass_signals = linux_nat_pass_signals;
   t->to_xfer_partial = linux_nat_xfer_partial;
   t->to_kill = linux_nat_kill;
   t->to_mourn_inferior = linux_nat_mourn_inferior;
Index: gdb/nto-procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/nto-procfs.c,v
retrieving revision 1.57
diff -u -p -r1.57 nto-procfs.c
--- gdb/nto-procfs.c	11 Jan 2011 15:10:01 -0000	1.57
+++ gdb/nto-procfs.c	18 Mar 2011 17:23:19 -0000
@@ -64,8 +64,6 @@ static int procfs_xfer_memory (CORE_ADDR
 			       struct mem_attrib *attrib,
 			       struct target_ops *);
 
-static void notice_signals (void);
-
 static void init_procfs_ops (void);
 
 static ptid_t do_attach (ptid_t ptid);
@@ -983,8 +981,6 @@ procfs_resume (struct target_ops *ops,
 
   run.flags |= _DEBUG_RUN_ARM;
 
-  sigemptyset (&run.trace);
-  notice_signals ();
   signal_to_pass = target_signal_to_host (signo);
 
   if (signal_to_pass)
@@ -1332,34 +1328,23 @@ procfs_store_registers (struct target_op
     }
 }
 
+/* Set list of signals to be handled in the target.  */
+
 static void
-notice_signals (void)
+procfs_pass_signals (int numsigs, unsigned char *pass_signals)
 {
   int signo;
 
+  sigfillset (&run.trace);
+
   for (signo = 1; signo < NSIG; signo++)
     {
-      if (signal_stop_state (target_signal_from_host (signo)) == 0
-	  && signal_print_state (target_signal_from_host (signo)) == 0
-	  && signal_pass_state (target_signal_from_host (signo)) == 1)
-	sigdelset (&run.trace, signo);
-      else
-	sigaddset (&run.trace, signo);
+      int target_signo = target_signal_from_host (signo);
+      if (target_signo < numsigs && pass_signals[target_signo])
+        sigdelset (&run.trace, signo);
     }
 }
 
-/* When the user changes the state of gdb's signal handling via the
-   "handle" command, this function gets called to see if any change
-   in the /proc interface is required.  It is also called internally
-   by other /proc interface functions to initialize the state of
-   the traced signal set.  */
-static void
-procfs_notice_signals (ptid_t ptid)
-{
-  sigemptyset (&run.trace);
-  notice_signals ();
-}
-
 static struct tidinfo *
 procfs_thread_info (pid_t pid, short tid)
 {
@@ -1424,7 +1409,7 @@ init_procfs_ops (void)
   procfs_ops.to_create_inferior = procfs_create_inferior;
   procfs_ops.to_mourn_inferior = procfs_mourn_inferior;
   procfs_ops.to_can_run = procfs_can_run;
-  procfs_ops.to_notice_signals = procfs_notice_signals;
+  procfs_ops.to_pass_signals = procfs_pass_signals;
   procfs_ops.to_thread_alive = procfs_thread_alive;
   procfs_ops.to_find_new_threads = procfs_find_new_threads;
   procfs_ops.to_pid_to_str = procfs_pid_to_str;
@@ -1456,8 +1441,8 @@ _initialize_procfs (void)
   sigaddset (&set, SIGUSR1);
   sigprocmask (SIG_BLOCK, &set, NULL);
 
-  /* Set up trace and fault sets, as gdb expects them.  */
-  sigemptyset (&run.trace);
+  /* Initially, make sure all signals are reported.  */
+  sigfillset (&run.trace);
 
   /* Stuff some information.  */
   nto_cpuinfo_flags = SYSPAGE_ENTRY (cpuinfo)->flags;
Index: gdb/procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/procfs.c,v
retrieving revision 1.143
diff -u -p -r1.143 procfs.c
--- gdb/procfs.c	9 Mar 2011 12:48:55 -0000	1.143
+++ gdb/procfs.c	18 Mar 2011 17:23:20 -0000
@@ -120,7 +120,7 @@ static void procfs_fetch_registers (stru
 				    struct regcache *, int);
 static void procfs_store_registers (struct target_ops *,
 				    struct regcache *, int);
-static void procfs_notice_signals (ptid_t);
+static void procfs_pass_signals (ptid_t, int, unsigned char *);
 static void procfs_kill_inferior (struct target_ops *ops);
 static void procfs_mourn_inferior (struct target_ops *ops);
 static void procfs_create_inferior (struct target_ops *, char *,
@@ -201,7 +201,7 @@ procfs_target (void)
   t->to_store_registers = procfs_store_registers;
   t->to_xfer_partial = procfs_xfer_partial;
   t->deprecated_xfer_memory = procfs_xfer_memory;
-  t->to_notice_signals = procfs_notice_signals;
+  t->to_pass_signals = procfs_pass_signals;
   t->to_files_info = procfs_files_info;
   t->to_stop = procfs_stop;
 
@@ -3147,7 +3147,6 @@ proc_iterate_over_threads (procinfo *pi,
 
 static ptid_t do_attach (ptid_t ptid);
 static void do_detach (int signo);
-static int register_gdb_signals (procinfo *, gdb_sigset_t *);
 static void proc_trace_syscalls_1 (procinfo *pi, int syscallnum,
 				   int entry_or_exit, int mode, int from_tty);
 
@@ -3186,9 +3185,9 @@ procfs_debug_inferior (procinfo *pi)
   if (!proc_set_traced_faults  (pi, &traced_faults))
     return __LINE__;
 
-  /* Register to trace selected signals in the child.  */
-  premptyset (&traced_signals);
-  if (!register_gdb_signals (pi, &traced_signals))
+  /* Initially, register to trace all signals in the child.  */
+  prfillset (&traced_signals);
+  if (!proc_set_traced_signals (pi, &traced_signals))
     return __LINE__;
 
 
@@ -4464,40 +4463,26 @@ procfs_resume (struct target_ops *ops,
     }
 }
 
-/* Traverse the list of signals that GDB knows about (see "handle"
-   command), and arrange for the target to be stopped or not,
-   according to these settings.  Returns non-zero for success, zero
-   for failure.  */
-
-static int
-register_gdb_signals (procinfo *pi, gdb_sigset_t *signals)
-{
-  int signo;
-
-  for (signo = 0; signo < NSIG; signo ++)
-    if (signal_stop_state  (target_signal_from_host (signo)) == 0 &&
-	signal_print_state (target_signal_from_host (signo)) == 0 &&
-	signal_pass_state  (target_signal_from_host (signo)) == 1)
-      gdb_prdelset (signals, signo);
-    else
-      gdb_praddset (signals, signo);
-
-  return proc_set_traced_signals (pi, signals);
-}
-
 /* Set up to trace signals in the child process.  */
 
 static void
-procfs_notice_signals (ptid_t ptid)
+procfs_pass_signals (int numsigs, unsigned char *pass_signals)
 {
   gdb_sigset_t signals;
-  procinfo *pi = find_procinfo_or_die (PIDGET (ptid), 0);
+  procinfo *pi = find_procinfo_or_die (PIDGET (inferior_ptid), 0);
+  int signo;
 
-  if (proc_get_traced_signals (pi, &signals) &&
-      register_gdb_signals    (pi, &signals))
-    return;
-  else
-    proc_error (pi, "notice_signals", __LINE__);
+  prfillset (&signals);
+
+  for (signo = 0; signo < NSIG; signo++)
+    {
+      int target_signo = target_signal_from_host (signo);
+      if (target_signo < numsigs && pass_signals[target_signo])
+	gdb_prdelset (&signals, signo);
+    }
+
+  if (!proc_set_traced_signals (pi, &signals))
+    proc_error (pi, "pass_signals", __LINE__);
 }
 
 /* Print status information about the child process.  */
@@ -4679,11 +4664,6 @@ procfs_init_inferior (struct target_ops 
   if (!proc_get_traced_sysexit  (pi, pi->saved_exitset))
     proc_error (pi, "init_inferior, get_traced_sysexit", __LINE__);
 
-  /* Register to trace selected signals in the child.  */
-  prfillset (&signals);
-  if (!register_gdb_signals (pi, &signals))
-    proc_error (pi, "init_inferior, register_signals", __LINE__);
-
   if ((fail = procfs_debug_inferior (pi)) != 0)
     proc_error (pi, "init_inferior (procfs_debug_inferior)", fail);
 
Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.440
diff -u -p -r1.440 remote.c
--- gdb/remote.c	16 Mar 2011 17:59:02 -0000	1.440
+++ gdb/remote.c	18 Mar 2011 17:23:21 -0000
@@ -1559,20 +1559,17 @@ static char *last_pass_packet;
    it can simply pass through to the inferior without reporting.  */
 
 static void
-remote_pass_signals (void)
+remote_pass_signals (int numsigs, unsigned char *pass_signals)
 {
   if (remote_protocol_packets[PACKET_QPassSignals].support != PACKET_DISABLE)
     {
       char *pass_packet, *p;
-      int numsigs = (int) TARGET_SIGNAL_LAST;
       int count = 0, i;
 
       gdb_assert (numsigs < 256);
       for (i = 0; i < numsigs; i++)
 	{
-	  if (signal_stop_state (i) == 0
-	      && signal_print_state (i) == 0
-	      && signal_pass_state (i) == 1)
+	  if (pass_signals[i])
 	    count++;
 	}
       pass_packet = xmalloc (count * 3 + strlen ("QPassSignals:") + 1);
@@ -1580,9 +1577,7 @@ remote_pass_signals (void)
       p = pass_packet + strlen (pass_packet);
       for (i = 0; i < numsigs; i++)
 	{
-	  if (signal_stop_state (i) == 0
-	      && signal_print_state (i) == 0
-	      && signal_pass_state (i) == 1)
+	  if (pass_signals[i])
 	    {
 	      if (i >= 16)
 		*p++ = tohex (i >> 4);
@@ -1612,14 +1607,6 @@ remote_pass_signals (void)
     }
 }
 
-static void
-remote_notice_signals (ptid_t ptid)
-{
-  /* Update the remote on signals to silently pass, if they've
-     changed.  */
-  remote_pass_signals ();
-}
-
 /* If PTID is MAGIC_NULL_PTID, don't set any thread.  If PTID is
    MINUS_ONE_PTID, set the thread to -1, so the stub returns the
    thread.  If GEN is set, set the general thread, if not, then set
@@ -3355,10 +3342,8 @@ remote_start_remote (struct ui_out *uiou
 	 the stop reply queue.  */
       gdb_assert (wait_status == NULL);
 
-      /* Update the remote on signals to silently pass, or more
-	 importantly, which to not ignore, in case a previous session
-	 had set some different set of signals to be ignored.  */
-      remote_pass_signals ();
+      /* Report all signals during attach/startup.  */
+      remote_pass_signals (0, NULL);
     }
 
   /* If we connected to a live target, do some additional setup.  */
@@ -4528,9 +4513,6 @@ remote_resume (struct target_ops *ops,
   last_sent_signal = siggnal;
   last_sent_step = step;
 
-  /* Update the inferior on signals to silently pass, if they've changed.  */
-  remote_pass_signals ();
-
   /* The vCont packet doesn't need to specify threads via Hc.  */
   /* No reverse support (yet) for vCont.  */
   if (execution_direction != EXEC_REVERSE)
@@ -10272,7 +10254,7 @@ Specify the serial device it is connecte
   remote_ops.to_kill = remote_kill;
   remote_ops.to_load = generic_load;
   remote_ops.to_mourn_inferior = remote_mourn;
-  remote_ops.to_notice_signals = remote_notice_signals;
+  remote_ops.to_pass_signals = remote_pass_signals;
   remote_ops.to_thread_alive = remote_thread_alive;
   remote_ops.to_find_new_threads = remote_threads_info;
   remote_ops.to_pid_to_str = remote_pid_to_str;
Index: gdb/target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.279
diff -u -p -r1.279 target.c
--- gdb/target.c	17 Mar 2011 13:19:24 -0000	1.279
+++ gdb/target.c	18 Mar 2011 17:23:22 -0000
@@ -149,8 +149,6 @@ static void debug_to_load (char *, int);
 
 static int debug_to_can_run (void);
 
-static void debug_to_notice_signals (ptid_t);
-
 static void debug_to_stop (ptid_t);
 
 /* Pointer to array of target architecture structures; the size of the
@@ -624,7 +622,7 @@ update_current_target (void)
       INHERIT (to_has_exited, t);
       /* Do not inherit to_mourn_inferior.  */
       INHERIT (to_can_run, t);
-      INHERIT (to_notice_signals, t);
+      /* Do not inherit to_pass_signals.  */
       /* Do not inherit to_thread_alive.  */
       /* Do not inherit to_find_new_threads.  */
       /* Do not inherit to_pid_to_str.  */
@@ -792,9 +790,6 @@ update_current_target (void)
 	    return_zero);
   de_fault (to_can_run,
 	    return_zero);
-  de_fault (to_notice_signals,
-	    (void (*) (ptid_t))
-	    target_ignore);
   de_fault (to_extra_thread_info,
 	    (char *(*) (struct thread_info *))
 	    return_zero);
@@ -2586,6 +2581,37 @@ target_resume (ptid_t ptid, int step, en
 
   noprocess ();
 }
+
+void
+target_pass_signals (int numsigs, unsigned char *pass_signals)
+{
+  struct target_ops *t;
+
+  for (t = current_target.beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_pass_signals != NULL)
+	{
+	  if (targetdebug)
+	    {
+	      int i;
+
+	      fprintf_unfiltered (gdb_stdlog, "target_pass_signals (%d, {",
+				  numsigs);
+
+	      for (i = 0; i < numsigs; i++)
+		if (pass_signals[i])
+		  fprintf_unfiltered (gdb_stdlog, " %s",
+				      target_signal_to_name (i));
+
+	      fprintf_unfiltered (gdb_stdlog, " })\n");
+	    }
+
+	  (*t->to_pass_signals) (numsigs, pass_signals);
+	  return;
+	}
+    }
+}
+
 /* Look through the list of possible targets for a target that can
    follow forks.  */
 
@@ -3898,15 +3924,6 @@ debug_to_can_run (void)
   return retval;
 }
 
-static void
-debug_to_notice_signals (ptid_t ptid)
-{
-  debug_target.to_notice_signals (ptid);
-
-  fprintf_unfiltered (gdb_stdlog, "target_notice_signals (%d)\n",
-                      PIDGET (ptid));
-}
-
 static struct gdbarch *
 debug_to_thread_architecture (struct target_ops *ops, ptid_t ptid)
 {
@@ -3994,7 +4011,6 @@ setup_target_debug (void)
   current_target.to_remove_exec_catchpoint = debug_to_remove_exec_catchpoint;
   current_target.to_has_exited = debug_to_has_exited;
   current_target.to_can_run = debug_to_can_run;
-  current_target.to_notice_signals = debug_to_notice_signals;
   current_target.to_stop = debug_to_stop;
   current_target.to_rcmd = debug_to_rcmd;
   current_target.to_pid_to_exec_file = debug_to_pid_to_exec_file;
Index: gdb/target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.202
diff -u -p -r1.202 target.h
--- gdb/target.h	17 Mar 2011 13:19:24 -0000	1.202
+++ gdb/target.h	18 Mar 2011 17:23:22 -0000
@@ -493,7 +493,7 @@ struct target_ops
     int (*to_has_exited) (int, int, int *);
     void (*to_mourn_inferior) (struct target_ops *);
     int (*to_can_run) (void);
-    void (*to_notice_signals) (ptid_t ptid);
+    void (*to_pass_signals) (int, unsigned char *);
     int (*to_thread_alive) (struct target_ops *, ptid_t ptid);
     void (*to_find_new_threads) (struct target_ops *);
     char *(*to_pid_to_str) (struct target_ops *, ptid_t);
@@ -1119,10 +1119,9 @@ void target_mourn_inferior (void);
 #define target_can_run(t) \
      ((t)->to_can_run) ()
 
-/* post process changes to signal handling in the inferior.  */
+/* Set list of signals to be handled in the target.  */
 
-#define target_notice_signals(ptid) \
-     (*current_target.to_notice_signals) (ptid)
+extern void target_pass_signals (int nsig, unsigned char *pass_signals);
 
 /* Check to see if a thread is still alive.  */
 
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Pedro Alves]

On Friday 18 March 2011 20:20:49, Ulrich Weigand wrote:
> Hello,
> 
> several targets today implement a performance optimization where some signals
> are not reported back to the GDB core, but instead are directly delivered to
> the target; this can be done either by not even trapping those signals (e.g.
> procfs targets), by restarting the inferior directly in target code (e.g.
> linux-nat), or by restarting the inferior in the remote stub.
> 
> This works via a target callback to_notice_signal, which is called whenever
> the list of signals to be handled by GDB changes.  Target code is then
> expected to itself detect which signals are safe to short-circuit (by calling
> signal_pass_state) etc.  Target code must also handle (implicitly defined)
> special cases, e.g. that signals must always be reported during startup
> or when single-stepping.
> 
> It is this last condition that causes problems on architectures that use
> software single-step, because target code isn't even aware of that
> condition in general.  This causes failures on ARM, see the discussion here:
> http://sourceware.org/ml/gdb-patches/2011-01/msg00398.html
> 
> The solution that was discussed there is basically to make the implicit
> rules of signal passing shortcuts explicit.  The patch below is an
> attempt to implement this idea.  The basic concept is to remove the
> to_notice_signals callback and replace it by a to_pass_signals callback,
> which explicitly gets a list of signals that can be safely bypassed
> at this point.
> 
> Common code then calls this new target callback as appropriate, in
> particular to mark all signals non-bypassable when single-stepping.
> 
> All platforms using to_notice_signals (procfs, nto-procfs, remote)
> or directly accessing signal_pass_state etc (linux-nat) are updated
> to instead use the to_pass_signals callback.
> 
> Tested on i386-linux, both native and gdbserver, with no regressions.
> Fixes the same set of failures on ARM as the original patch.
> 
> Any thoughts on this approach?

As discussed before, I like the approach, but I have a couple of remarks
to the implementation:

 - There are more calls to target_resume in infrun.c.  Don't we need
to consider signals around those?

 - In non-stop, if you have this sequence:

     - step thread 1
     - continue thread 2 (imediately afterwards)
     - thread 1 gets signal FOO (which is normally pass-able).

   when handling the latter event, since the "signal_pass" set is
   global, and has been filled by the previous continue, the target
   will think it doesn't need to report the signal back to core gdb.

You've said before about this when I raised the issue before:

 "If the implementation is conservative in the right direction,
 the worst thing that could happen is that a signal is reported that
 might have gotten short-circuited .."

Might be we just need to check if _any_ thread is stepping, when
deciding whether to tell the target to report back all signals?

-- 
Pedro Alves

Re: [rfc][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Tom Tromey]

>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:

Ulrich> -/* post process changes to signal handling in the inferior.  */
Ulrich> +/* Set list of signals to be handled in the target.  */
 
Ulrich> -#define target_notice_signals(ptid) \
Ulrich> -     (*current_target.to_notice_signals) (ptid)
Ulrich> +extern void target_pass_signals (int nsig, unsigned char *pass_signals);

I would like it if there were more documentation about what this method
means and how it is used.  Actually, I suppose I would prefer it on the
field in struct target_ops, to be read by people porting gdb.

Tom

Re: [rfc][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Pedro Alves wrote:

> As discussed before, I like the approach, but I have a couple of remarks
> to the implementation:

Thanks for the review!

>  - There are more calls to target_resume in infrun.c.  Don't we need
> to consider signals around those?

Hmm, that's true.

>  - In non-stop, if you have this sequence:
> 
>      - step thread 1
>      - continue thread 2 (imediately afterwards)
>      - thread 1 gets signal FOO (which is normally pass-able).
> 
>    when handling the latter event, since the "signal_pass" set is
>    global, and has been filled by the previous continue, the target
>    will think it doesn't need to report the signal back to core gdb.
> 
> You've said before about this when I raised the issue before:
> 
>  "If the implementation is conservative in the right direction,
>  the worst thing that could happen is that a signal is reported that
>  might have gotten short-circuited .."
> 
> Might be we just need to check if _any_ thread is stepping, when
> deciding whether to tell the target to report back all signals?

Right, that seems correct to me.

Tom Tromey wrote:

> I would like it if there were more documentation about what this method
> means and how it is used.  Actually, I suppose I would prefer it on the
> field in struct target_ops, to be read by people porting gdb.

Good point, agreed.


I'll work on another update of the patch ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Pedro Alves wrote:

> Might be we just need to check if _any_ thread is stepping, when
> deciding whether to tell the target to report back all signals?

I've thought about this a bit more.  The primary cause why we need
to tell the target to report back all signals is because we have
pulled out breakpoints to step over one -- this is why running
arbitrary signal handlers would be dangerous.

Now, whether we have pulled out breakpoints or not is actually
already global state, of course -- they're either in or out.
In non-stop mode, we therefore have to use displaced stepping
anyway -- and if we do that, there's no need to disable the
signal reporting shortcuts.

So I've changed the code to only disable those shortcuts while
breakpoints are pulled out, and enable them otherwise, regardless
of the stepping state of any threads.

However, this exposed a second reason why targets currently
report back all signals when single-stepping: if the target
uses hardware single-stepping, and short-circuits signal
delivery, the kernel will actually single-step to the first
instruction of the handler and stop there.  If this happens
while we're stepping, GDB gets confused about where we are
now in relation to the stepping range, and just stops.

Apparently, in the past GDB used to attempt to recognize
this situation and handle it similarly to stepping into
function calls.  However, this was pulled out by Andrew Cagney
in 2004 and replaced by the current method of setting a step-
resume breakpoint *before* delivering the signal.  This helped
work around a number of problems as described here:
http://sourceware.org/ml/gdb-patches/2004-05/msg00273.html

For this approach to work, however, we do need to get hold of
every signal that's being delivered while in *hardware*
single-step mode.

Now, a lot of GDB infrastructure has improved since 2004, and
maybe we could go back to recognizing signal handler calls
after they've happened.  However, the situation remains complex
(some OSes call a signal trampoline which calls the handler,
some call the handler directly, on some we need the breakpoint
in the caller to work around the "sticky" trap flag problem
described in Andrew's mail ...)

So I thought that for now we might simply keep the implementation
in the linux and remote targets as-is today, and simply always
report signals for threads that are being single-stepped at the
target layer (i.e. hardware single-step).  There is no race since
the information is per-thread, and it is available to the target
anyway (as it works today).

So with the combination of the two rules:
- GDB uses target_pass_signals to disable signal bypassing
  globally whenever breakpoints are pulled out
- The targets skip bypassing locally on threads that are
  hardware single-stepped

we can actually fix both software and hardware single-step
targets.  The patch below implements this approach.

>  - There are more calls to target_resume in infrun.c.  Don't we need
> to consider signals around those?

Since we now only disable signal bypassing when breakpoints are
pulled out, which never happens when using displaced stepping,
we don't need to do anything around those target_resume calls
that are part of the displaced stepping implementation.

The call in handle_inferior_event's new_thread_event case is also
fine since no breakpoints are removed or inserted in this case.

The call in handle_inferior_event when stepping across (non-)
steppable watchpoints is a problem, however: this may actually
remove breakpoints as well.  I've added code to disable signal
bypassing during infwait_nonstep_watch_state as well.

There are no further target_resume calls to consider.

As a final change, I noticed a pre-existing bug in
handle_inferior_event:

      if (ecs->event_thread->prev_pc == stop_pc
          && ecs->event_thread->control.trap_expected
          && ecs->event_thread->control.step_resume_breakpoint == NULL)
        {
          /* We were just starting a new sequence, attempting to
             single-step off of a breakpoint and expecting a SIGTRAP.
             Instead this signal arrives.  This signal will take us out
             of the stepping range so GDB needs to remember to, when
             the signal handler returns, resume stepping off that
             breakpoint.  */
          /* To simplify things, "continue" is forced to use the same
             code paths as single-step - set a breakpoint at the
             signal return address and then, once hit, step off that
             breakpoint.  */
          if (debug_infrun)
            fprintf_unfiltered (gdb_stdlog,
                                "infrun: signal arrived while stepping over "
                                "breakpoint\n");

          insert_step_resume_breakpoint_at_frame (frame);
          ecs->event_thread->step_after_step_resume_breakpoint = 1;
          keep_going (ecs);
          return;
        }

This adds a step-resume breakpoint, but then goes on to call keep_going with
ecs->event_thread->control.trap_expected still set, which means that it assumes
breakpoints should remain removed and we step over a breakpoint ...  This isxi
kind of pointless in any case (causes an additional ptrace intercept), but it
is actually buggy in the software single-step case, since it confuses resume
into thinking signal bypasses are OK when they're not.

I've fixed this by resetting trap_expected to zero.  With this change, this
patch alone actually fixes all sigstep.exp failures on ARM.

Tested on i386-linux and armv7l-unknown-linux-gnueabi with no regressions.

Thoughts?

Bye,
Ulrich


ChangeLog:

	* target.h (struct target_ops): Remove to_notice_signals;
	add to_pass_signals.
	(target_notice_signals): Remove.
	(target_pass_signals): Add prototype.
	* target.c (update_current_target): Remove to_notice_signals;
	mention to_pass_signals.
	(target_pass_signals): New function.
	(debug_to_notice_signals): Remove.
	(setup_target_debug): Do not install debug_to_notice_signals.

	* infrun.c (signal_pass): New global.
	(resume): Call target_pass_signals.
	(handle_inferior_event): Report all signals while stepping over
	non-steppable watchpoint.  Reset trap_expected to ensure breakpoints
	are re-inserted when stepping over a signal handler.
	(signal_cache_update): New function.
	(signal_stop_update): Call it.
	(signal_print_update): Likewise.
	(signal_pass_update): Likewise.
	(handle_command): Call signal_cache_update and target_pass_signals
	instead of target_notice_signals.
	(_initialize_infrun): Initialize signal_pass.

	* linux-nat.c (pass_mask): New global.
	(linux_nat_pass_signals): New function.
	(linux_nat_create_inferior): Report all signals initially.
	(linux_nat_attach): Likewise.
	(linux_nat_resume): Use pass_mask to decide whether to directly
	handle an inferior signal.
	(linux_nat_wait_1): Likewise.
	(linux_nat_add_target): Install to_pass_signals callback.

	* nto-procfs.c (notice_signals): Remove.
	(procfs_resume): Do not call notice_signals.
	(procfs_notice_signals): Remove.
	(procfs_pass_signals): New function.
	(init_procfs_ops): Install to_pass_signals callback instead of
	to_notice_signals callback.
	(_initialize_procfs): Report all signals initially.

	* procfs.c (procfs_notice_signals): Remove.
	(procfs_pass_signals): New function.
	(procfs_target): Install to_pass_signals callback instead of
	to_notice_signals callback.
	(register_gdb_signals): Remove.
	(procfs_debug_inferior): Report all signals initially.
	(procfs_init_inferior): Remove redundant register_gdb_signals call.

	* remote.c (remote_pass_signals): Add numsigs and pass_signals
	parameters; use them instead of calling signal_..._state routines.
	(remote_notice_signals): Remove.
	(remote_start_remote): Report all signals initially.
	(remote_resume): Do not call remote_pass_signals.
	(_initialize_remote): Install to_pass_signals callback instead of
	to_notice_signals callback.


Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.472
diff -u -p -r1.472 infrun.c
--- gdb/infrun.c	7 Mar 2011 16:03:00 -0000	1.472
+++ gdb/infrun.c	31 Mar 2011 16:31:06 -0000
@@ -276,6 +276,11 @@ static unsigned char *signal_stop;
 static unsigned char *signal_print;
 static unsigned char *signal_program;
 
+/* Table of signals that the target may silently handle.
+   This is automatically determined from the flags above,
+   and simply cached here.  */
+static unsigned char *signal_pass;
+
 #define SET_SIGS(nsigs,sigs,flags) \
   do { \
     int signum = (nsigs); \
@@ -1787,6 +1792,18 @@ a command like `return' or `jump' to con
 	 happens to apply to another thread.  */
       tp->suspend.stop_signal = TARGET_SIGNAL_0;
 
+      /* Advise target which signals may be handled silently.  If we have
+	 removed breakpoints because we are stepping over one (which can
+	 happen only if we are not using displaced stepping), we need to
+	 receive all signals to avoid accidentally skipping a breakpoint
+	 during execution of a signal handler.  */
+      if ((step || singlestep_breakpoints_inserted_p)
+	  && tp->control.trap_expected
+	  && !use_displaced_stepping (gdbarch))
+	target_pass_signals (0, NULL);
+      else
+	target_pass_signals ((int) TARGET_SIGNAL_LAST, signal_pass);
+
       target_resume (resume_ptid, step, sig);
     }
 
@@ -3818,7 +3835,12 @@ handle_inferior_event (struct execution_
       int hw_step = 1;
 
       if (!target_have_steppable_watchpoint)
-	remove_breakpoints ();
+	{
+	  remove_breakpoints ();
+	  /* See comment in resume why we need to stop bypassing signals
+	     while breakpoints have been removed.  */
+	  target_pass_signals (0, NULL);
+	}
 	/* Single step */
       hw_step = maybe_software_singlestep (gdbarch, stop_pc);
       target_resume (ecs->ptid, hw_step, TARGET_SIGNAL_0);
@@ -4090,6 +4112,8 @@ process_event_stop_test:
 
 	  insert_step_resume_breakpoint_at_frame (frame);
 	  ecs->event_thread->step_after_step_resume_breakpoint = 1;
+	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
+	  ecs->event_thread->control.trap_expected = 0;
 	  keep_going (ecs);
 	  return;
 	}
@@ -4117,6 +4141,8 @@ process_event_stop_test:
                                 "single-step range\n");
 
 	  insert_step_resume_breakpoint_at_frame (frame);
+	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
+	  ecs->event_thread->control.trap_expected = 0;
 	  keep_going (ecs);
 	  return;
 	}
@@ -5853,12 +5879,29 @@ signal_pass_state (int signo)
   return signal_program[signo];
 }
 
+static void
+signal_cache_update (int signo)
+{
+  if (signo == -1)
+    {
+      for (signo = 0; signo < (int) TARGET_SIGNAL_LAST; signo++)
+	signal_cache_update (signo);
+
+      return;
+    }
+
+  signal_pass[signo] = (signal_stop[signo] == 0
+			&& signal_print[signo] == 0
+			&& signal_program[signo] == 1);
+}
+
 int
 signal_stop_update (int signo, int state)
 {
   int ret = signal_stop[signo];
 
   signal_stop[signo] = state;
+  signal_cache_update (signo);
   return ret;
 }
 
@@ -5868,6 +5911,7 @@ signal_print_update (int signo, int stat
   int ret = signal_print[signo];
 
   signal_print[signo] = state;
+  signal_cache_update (signo);
   return ret;
 }
 
@@ -5877,6 +5921,7 @@ signal_pass_update (int signo, int state
   int ret = signal_program[signo];
 
   signal_program[signo] = state;
+  signal_cache_update (signo);
   return ret;
 }
 
@@ -6068,7 +6113,8 @@ Are you sure you want to change it? "),
   for (signum = 0; signum < nsigs; signum++)
     if (sigs[signum])
       {
-	target_notice_signals (inferior_ptid);
+	signal_cache_update (-1);
+	target_pass_signals ((int) TARGET_SIGNAL_LAST, signal_pass);
 
 	if (from_tty)
 	  {
@@ -6925,6 +6971,8 @@ leave it stopped or free to run as neede
     xmalloc (sizeof (signal_print[0]) * numsigs);
   signal_program = (unsigned char *)
     xmalloc (sizeof (signal_program[0]) * numsigs);
+  signal_pass = (unsigned char *)
+    xmalloc (sizeof (signal_program[0]) * numsigs);
   for (i = 0; i < numsigs; i++)
     {
       signal_stop[i] = 1;
@@ -6968,6 +7016,9 @@ leave it stopped or free to run as neede
   signal_stop[TARGET_SIGNAL_CANCEL] = 0;
   signal_print[TARGET_SIGNAL_CANCEL] = 0;
 
+  /* Update cached state.  */
+  signal_cache_update (-1);
+
   add_setshow_zinteger_cmd ("stop-on-solib-events", class_support,
 			    &stop_on_solib_events, _("\
 Set stopping for shared library events."), _("\
Index: gdb/linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.199
diff -u -p -r1.199 linux-nat.c
--- gdb/linux-nat.c	9 Mar 2011 12:48:55 -0000	1.199
+++ gdb/linux-nat.c	31 Mar 2011 16:31:08 -0000
@@ -1078,6 +1078,26 @@ restore_child_signals_mask (sigset_t *pr
 {
   sigprocmask (SIG_SETMASK, prev_mask, NULL);
 }
+
+/* Mask of signals to pass directly to the inferior.  */
+static sigset_t pass_mask;
+
+/* Update signals to pass to the inferior.  */
+static void
+linux_nat_pass_signals (int numsigs, unsigned char *pass_signals)
+{
+  int signo;
+
+  sigemptyset (&pass_mask);
+
+  for (signo = 1; signo < NSIG; signo++)
+    {
+      int target_signo = target_signal_from_host (signo);
+      if (target_signo < numsigs && pass_signals[target_signo])
+        sigaddset (&pass_mask, signo);
+    }
+}
+
 \f
 
 /* Prototypes for local functions.  */
@@ -1543,6 +1563,9 @@ linux_nat_create_inferior (struct target
     }
 #endif /* HAVE_PERSONALITY */
 
+  /* Make sure we report all signals during startup.  */
+  linux_nat_pass_signals (0, NULL);
+
   linux_ops->to_create_inferior (ops, exec_file, allargs, env, from_tty);
 
 #ifdef HAVE_PERSONALITY
@@ -1564,6 +1587,9 @@ linux_nat_attach (struct target_ops *ops
   int status;
   ptid_t ptid;
 
+  /* Make sure we report all signals during attach.  */
+  linux_nat_pass_signals (0, NULL);
+
   linux_ops->to_attach (ops, args, from_tty);
 
   /* The ptrace base target adds the main thread with (pid,0,0)
@@ -1922,19 +1948,9 @@ linux_nat_resume (struct target_ops *ops
 
   if (lp->status && WIFSTOPPED (lp->status))
     {
-      enum target_signal saved_signo;
-      struct inferior *inf;
-
-      inf = find_inferior_pid (ptid_get_pid (lp->ptid));
-      gdb_assert (inf);
-      saved_signo = target_signal_from_host (WSTOPSIG (lp->status));
-
-      /* Defer to common code if we're gaining control of the
-	 inferior.  */
-      if (inf->control.stop_soon == NO_STOP_QUIETLY
-	  && signal_stop_state (saved_signo) == 0
-	  && signal_print_state (saved_signo) == 0
-	  && signal_pass_state (saved_signo) == 1)
+      if (!lp->step
+	  && WSTOPSIG (lp->status)
+	  && sigismember (&pass_mask, WSTOPSIG (lp->status)))
 	{
 	  if (debug_linux_nat)
 	    fprintf_unfiltered (gdb_stdlog,
@@ -1944,7 +1960,7 @@ linux_nat_resume (struct target_ops *ops
 	  /* FIXME: What should we do if we are supposed to continue
 	     this thread with a signal?  */
 	  gdb_assert (signo == TARGET_SIGNAL_0);
-	  signo = saved_signo;
+	  signo = target_signal_from_host (WSTOPSIG (lp->status));
 	  lp->status = 0;
 	}
     }
@@ -3590,20 +3606,11 @@ retry:
   if (WIFSTOPPED (status))
     {
       enum target_signal signo = target_signal_from_host (WSTOPSIG (status));
-      struct inferior *inf;
-
-      inf = find_inferior_pid (ptid_get_pid (lp->ptid));
-      gdb_assert (inf);
 
-      /* Defer to common code if we get a signal while
-	 single-stepping, since that may need special care, e.g. to
-	 skip the signal handler, or, if we're gaining control of the
-	 inferior.  */
+      /* When using hardware single-step, we need to report every signal.
+	 Otherwise, signals in pass_mask may be short-circuited.  */
       if (!lp->step
-	  && inf->control.stop_soon == NO_STOP_QUIETLY
-	  && signal_stop_state (signo) == 0
-	  && signal_print_state (signo) == 0
-	  && signal_pass_state (signo) == 1)
+	  && WSTOPSIG (status) && sigismember (&pass_mask, WSTOPSIG (status)))
 	{
 	  /* FIMXE: kettenis/2001-06-06: Should we resume all threads
 	     here?  It is not clear we should.  GDB may not expect
@@ -5712,6 +5719,7 @@ linux_nat_add_target (struct target_ops 
   t->to_detach = linux_nat_detach;
   t->to_resume = linux_nat_resume;
   t->to_wait = linux_nat_wait;
+  t->to_pass_signals = linux_nat_pass_signals;
   t->to_xfer_partial = linux_nat_xfer_partial;
   t->to_kill = linux_nat_kill;
   t->to_mourn_inferior = linux_nat_mourn_inferior;
Index: gdb/nto-procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/nto-procfs.c,v
retrieving revision 1.57
diff -u -p -r1.57 nto-procfs.c
--- gdb/nto-procfs.c	11 Jan 2011 15:10:01 -0000	1.57
+++ gdb/nto-procfs.c	31 Mar 2011 16:31:08 -0000
@@ -64,8 +64,6 @@ static int procfs_xfer_memory (CORE_ADDR
 			       struct mem_attrib *attrib,
 			       struct target_ops *);
 
-static void notice_signals (void);
-
 static void init_procfs_ops (void);
 
 static ptid_t do_attach (ptid_t ptid);
@@ -983,8 +981,6 @@ procfs_resume (struct target_ops *ops,
 
   run.flags |= _DEBUG_RUN_ARM;
 
-  sigemptyset (&run.trace);
-  notice_signals ();
   signal_to_pass = target_signal_to_host (signo);
 
   if (signal_to_pass)
@@ -1332,34 +1328,23 @@ procfs_store_registers (struct target_op
     }
 }
 
+/* Set list of signals to be handled in the target.  */
+
 static void
-notice_signals (void)
+procfs_pass_signals (int numsigs, unsigned char *pass_signals)
 {
   int signo;
 
+  sigfillset (&run.trace);
+
   for (signo = 1; signo < NSIG; signo++)
     {
-      if (signal_stop_state (target_signal_from_host (signo)) == 0
-	  && signal_print_state (target_signal_from_host (signo)) == 0
-	  && signal_pass_state (target_signal_from_host (signo)) == 1)
-	sigdelset (&run.trace, signo);
-      else
-	sigaddset (&run.trace, signo);
+      int target_signo = target_signal_from_host (signo);
+      if (target_signo < numsigs && pass_signals[target_signo])
+        sigdelset (&run.trace, signo);
     }
 }
 
-/* When the user changes the state of gdb's signal handling via the
-   "handle" command, this function gets called to see if any change
-   in the /proc interface is required.  It is also called internally
-   by other /proc interface functions to initialize the state of
-   the traced signal set.  */
-static void
-procfs_notice_signals (ptid_t ptid)
-{
-  sigemptyset (&run.trace);
-  notice_signals ();
-}
-
 static struct tidinfo *
 procfs_thread_info (pid_t pid, short tid)
 {
@@ -1424,7 +1409,7 @@ init_procfs_ops (void)
   procfs_ops.to_create_inferior = procfs_create_inferior;
   procfs_ops.to_mourn_inferior = procfs_mourn_inferior;
   procfs_ops.to_can_run = procfs_can_run;
-  procfs_ops.to_notice_signals = procfs_notice_signals;
+  procfs_ops.to_pass_signals = procfs_pass_signals;
   procfs_ops.to_thread_alive = procfs_thread_alive;
   procfs_ops.to_find_new_threads = procfs_find_new_threads;
   procfs_ops.to_pid_to_str = procfs_pid_to_str;
@@ -1456,8 +1441,8 @@ _initialize_procfs (void)
   sigaddset (&set, SIGUSR1);
   sigprocmask (SIG_BLOCK, &set, NULL);
 
-  /* Set up trace and fault sets, as gdb expects them.  */
-  sigemptyset (&run.trace);
+  /* Initially, make sure all signals are reported.  */
+  sigfillset (&run.trace);
 
   /* Stuff some information.  */
   nto_cpuinfo_flags = SYSPAGE_ENTRY (cpuinfo)->flags;
Index: gdb/procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/procfs.c,v
retrieving revision 1.143
diff -u -p -r1.143 procfs.c
--- gdb/procfs.c	9 Mar 2011 12:48:55 -0000	1.143
+++ gdb/procfs.c	31 Mar 2011 16:31:09 -0000
@@ -120,7 +120,7 @@ static void procfs_fetch_registers (stru
 				    struct regcache *, int);
 static void procfs_store_registers (struct target_ops *,
 				    struct regcache *, int);
-static void procfs_notice_signals (ptid_t);
+static void procfs_pass_signals (ptid_t, int, unsigned char *);
 static void procfs_kill_inferior (struct target_ops *ops);
 static void procfs_mourn_inferior (struct target_ops *ops);
 static void procfs_create_inferior (struct target_ops *, char *,
@@ -201,7 +201,7 @@ procfs_target (void)
   t->to_store_registers = procfs_store_registers;
   t->to_xfer_partial = procfs_xfer_partial;
   t->deprecated_xfer_memory = procfs_xfer_memory;
-  t->to_notice_signals = procfs_notice_signals;
+  t->to_pass_signals = procfs_pass_signals;
   t->to_files_info = procfs_files_info;
   t->to_stop = procfs_stop;
 
@@ -3147,7 +3147,6 @@ proc_iterate_over_threads (procinfo *pi,
 
 static ptid_t do_attach (ptid_t ptid);
 static void do_detach (int signo);
-static int register_gdb_signals (procinfo *, gdb_sigset_t *);
 static void proc_trace_syscalls_1 (procinfo *pi, int syscallnum,
 				   int entry_or_exit, int mode, int from_tty);
 
@@ -3186,9 +3185,9 @@ procfs_debug_inferior (procinfo *pi)
   if (!proc_set_traced_faults  (pi, &traced_faults))
     return __LINE__;
 
-  /* Register to trace selected signals in the child.  */
-  premptyset (&traced_signals);
-  if (!register_gdb_signals (pi, &traced_signals))
+  /* Initially, register to trace all signals in the child.  */
+  prfillset (&traced_signals);
+  if (!proc_set_traced_signals (pi, &traced_signals))
     return __LINE__;
 
 
@@ -4464,40 +4463,26 @@ procfs_resume (struct target_ops *ops,
     }
 }
 
-/* Traverse the list of signals that GDB knows about (see "handle"
-   command), and arrange for the target to be stopped or not,
-   according to these settings.  Returns non-zero for success, zero
-   for failure.  */
-
-static int
-register_gdb_signals (procinfo *pi, gdb_sigset_t *signals)
-{
-  int signo;
-
-  for (signo = 0; signo < NSIG; signo ++)
-    if (signal_stop_state  (target_signal_from_host (signo)) == 0 &&
-	signal_print_state (target_signal_from_host (signo)) == 0 &&
-	signal_pass_state  (target_signal_from_host (signo)) == 1)
-      gdb_prdelset (signals, signo);
-    else
-      gdb_praddset (signals, signo);
-
-  return proc_set_traced_signals (pi, signals);
-}
-
 /* Set up to trace signals in the child process.  */
 
 static void
-procfs_notice_signals (ptid_t ptid)
+procfs_pass_signals (int numsigs, unsigned char *pass_signals)
 {
   gdb_sigset_t signals;
-  procinfo *pi = find_procinfo_or_die (PIDGET (ptid), 0);
+  procinfo *pi = find_procinfo_or_die (PIDGET (inferior_ptid), 0);
+  int signo;
 
-  if (proc_get_traced_signals (pi, &signals) &&
-      register_gdb_signals    (pi, &signals))
-    return;
-  else
-    proc_error (pi, "notice_signals", __LINE__);
+  prfillset (&signals);
+
+  for (signo = 0; signo < NSIG; signo++)
+    {
+      int target_signo = target_signal_from_host (signo);
+      if (target_signo < numsigs && pass_signals[target_signo])
+	gdb_prdelset (&signals, signo);
+    }
+
+  if (!proc_set_traced_signals (pi, &signals))
+    proc_error (pi, "pass_signals", __LINE__);
 }
 
 /* Print status information about the child process.  */
@@ -4679,11 +4664,6 @@ procfs_init_inferior (struct target_ops 
   if (!proc_get_traced_sysexit  (pi, pi->saved_exitset))
     proc_error (pi, "init_inferior, get_traced_sysexit", __LINE__);
 
-  /* Register to trace selected signals in the child.  */
-  prfillset (&signals);
-  if (!register_gdb_signals (pi, &signals))
-    proc_error (pi, "init_inferior, register_signals", __LINE__);
-
   if ((fail = procfs_debug_inferior (pi)) != 0)
     proc_error (pi, "init_inferior (procfs_debug_inferior)", fail);
 
Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.441
diff -u -p -r1.441 remote.c
--- gdb/remote.c	18 Mar 2011 14:22:34 -0000	1.441
+++ gdb/remote.c	31 Mar 2011 16:31:10 -0000
@@ -1559,20 +1559,17 @@ static char *last_pass_packet;
    it can simply pass through to the inferior without reporting.  */
 
 static void
-remote_pass_signals (void)
+remote_pass_signals (int numsigs, unsigned char *pass_signals)
 {
   if (remote_protocol_packets[PACKET_QPassSignals].support != PACKET_DISABLE)
     {
       char *pass_packet, *p;
-      int numsigs = (int) TARGET_SIGNAL_LAST;
       int count = 0, i;
 
       gdb_assert (numsigs < 256);
       for (i = 0; i < numsigs; i++)
 	{
-	  if (signal_stop_state (i) == 0
-	      && signal_print_state (i) == 0
-	      && signal_pass_state (i) == 1)
+	  if (pass_signals[i])
 	    count++;
 	}
       pass_packet = xmalloc (count * 3 + strlen ("QPassSignals:") + 1);
@@ -1580,9 +1577,7 @@ remote_pass_signals (void)
       p = pass_packet + strlen (pass_packet);
       for (i = 0; i < numsigs; i++)
 	{
-	  if (signal_stop_state (i) == 0
-	      && signal_print_state (i) == 0
-	      && signal_pass_state (i) == 1)
+	  if (pass_signals[i])
 	    {
 	      if (i >= 16)
 		*p++ = tohex (i >> 4);
@@ -1612,14 +1607,6 @@ remote_pass_signals (void)
     }
 }
 
-static void
-remote_notice_signals (ptid_t ptid)
-{
-  /* Update the remote on signals to silently pass, if they've
-     changed.  */
-  remote_pass_signals ();
-}
-
 /* If PTID is MAGIC_NULL_PTID, don't set any thread.  If PTID is
    MINUS_ONE_PTID, set the thread to -1, so the stub returns the
    thread.  If GEN is set, set the general thread, if not, then set
@@ -3355,10 +3342,8 @@ remote_start_remote (struct ui_out *uiou
 	 the stop reply queue.  */
       gdb_assert (wait_status == NULL);
 
-      /* Update the remote on signals to silently pass, or more
-	 importantly, which to not ignore, in case a previous session
-	 had set some different set of signals to be ignored.  */
-      remote_pass_signals ();
+      /* Report all signals during attach/startup.  */
+      remote_pass_signals (0, NULL);
     }
 
   /* If we connected to a live target, do some additional setup.  */
@@ -4528,9 +4513,6 @@ remote_resume (struct target_ops *ops,
   last_sent_signal = siggnal;
   last_sent_step = step;
 
-  /* Update the inferior on signals to silently pass, if they've changed.  */
-  remote_pass_signals ();
-
   /* The vCont packet doesn't need to specify threads via Hc.  */
   /* No reverse support (yet) for vCont.  */
   if (execution_direction != EXEC_REVERSE)
@@ -10272,7 +10254,7 @@ Specify the serial device it is connecte
   remote_ops.to_kill = remote_kill;
   remote_ops.to_load = generic_load;
   remote_ops.to_mourn_inferior = remote_mourn;
-  remote_ops.to_notice_signals = remote_notice_signals;
+  remote_ops.to_pass_signals = remote_pass_signals;
   remote_ops.to_thread_alive = remote_thread_alive;
   remote_ops.to_find_new_threads = remote_threads_info;
   remote_ops.to_pid_to_str = remote_pid_to_str;
Index: gdb/target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.280
diff -u -p -r1.280 target.c
--- gdb/target.c	31 Mar 2011 14:32:48 -0000	1.280
+++ gdb/target.c	31 Mar 2011 16:31:11 -0000
@@ -149,8 +149,6 @@ static void debug_to_load (char *, int);
 
 static int debug_to_can_run (void);
 
-static void debug_to_notice_signals (ptid_t);
-
 static void debug_to_stop (ptid_t);
 
 /* Pointer to array of target architecture structures; the size of the
@@ -625,7 +623,7 @@ update_current_target (void)
       INHERIT (to_has_exited, t);
       /* Do not inherit to_mourn_inferior.  */
       INHERIT (to_can_run, t);
-      INHERIT (to_notice_signals, t);
+      /* Do not inherit to_pass_signals.  */
       /* Do not inherit to_thread_alive.  */
       /* Do not inherit to_find_new_threads.  */
       /* Do not inherit to_pid_to_str.  */
@@ -793,9 +791,6 @@ update_current_target (void)
 	    return_zero);
   de_fault (to_can_run,
 	    return_zero);
-  de_fault (to_notice_signals,
-	    (void (*) (ptid_t))
-	    target_ignore);
   de_fault (to_extra_thread_info,
 	    (char *(*) (struct thread_info *))
 	    return_zero);
@@ -2587,6 +2582,37 @@ target_resume (ptid_t ptid, int step, en
 
   noprocess ();
 }
+
+void
+target_pass_signals (int numsigs, unsigned char *pass_signals)
+{
+  struct target_ops *t;
+
+  for (t = current_target.beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_pass_signals != NULL)
+	{
+	  if (targetdebug)
+	    {
+	      int i;
+
+	      fprintf_unfiltered (gdb_stdlog, "target_pass_signals (%d, {",
+				  numsigs);
+
+	      for (i = 0; i < numsigs; i++)
+		if (pass_signals[i])
+		  fprintf_unfiltered (gdb_stdlog, " %s",
+				      target_signal_to_name (i));
+
+	      fprintf_unfiltered (gdb_stdlog, " })\n");
+	    }
+
+	  (*t->to_pass_signals) (numsigs, pass_signals);
+	  return;
+	}
+    }
+}
+
 /* Look through the list of possible targets for a target that can
    follow forks.  */
 
@@ -3914,15 +3940,6 @@ debug_to_can_run (void)
   return retval;
 }
 
-static void
-debug_to_notice_signals (ptid_t ptid)
-{
-  debug_target.to_notice_signals (ptid);
-
-  fprintf_unfiltered (gdb_stdlog, "target_notice_signals (%d)\n",
-                      PIDGET (ptid));
-}
-
 static struct gdbarch *
 debug_to_thread_architecture (struct target_ops *ops, ptid_t ptid)
 {
@@ -4010,7 +4027,6 @@ setup_target_debug (void)
   current_target.to_remove_exec_catchpoint = debug_to_remove_exec_catchpoint;
   current_target.to_has_exited = debug_to_has_exited;
   current_target.to_can_run = debug_to_can_run;
-  current_target.to_notice_signals = debug_to_notice_signals;
   current_target.to_stop = debug_to_stop;
   current_target.to_rcmd = debug_to_rcmd;
   current_target.to_pid_to_exec_file = debug_to_pid_to_exec_file;
Index: gdb/target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.203
diff -u -p -r1.203 target.h
--- gdb/target.h	31 Mar 2011 14:32:48 -0000	1.203
+++ gdb/target.h	31 Mar 2011 16:31:11 -0000
@@ -494,7 +494,11 @@ struct target_ops
     int (*to_has_exited) (int, int, int *);
     void (*to_mourn_inferior) (struct target_ops *);
     int (*to_can_run) (void);
-    void (*to_notice_signals) (ptid_t ptid);
+
+    /* Documentation of this routine is provided with the corresponding
+       target_* macro.  */
+    void (*to_pass_signals) (int, unsigned char *);
+
     int (*to_thread_alive) (struct target_ops *, ptid_t ptid);
     void (*to_find_new_threads) (struct target_ops *);
     char *(*to_pid_to_str) (struct target_ops *, ptid_t);
@@ -1120,10 +1124,19 @@ void target_mourn_inferior (void);
 #define target_can_run(t) \
      ((t)->to_can_run) ()
 
-/* post process changes to signal handling in the inferior.  */
+/* Set list of signals to be handled in the target.
+
+   PASS_SIGNALS is an array of size NSIG, indexed by target signal number
+   (enum target_signal).  For every signal whose entry in this array is
+   non-zero, the target is allowed -but not required- to skip reporting
+   arrival of the signal to the GDB core by returning from target_wait,
+   and to pass the signal directly to the inferior instead.
+
+   However, if the target is hardware single-stepping a thread that is
+   about to receive a signal, it needs to be reported in any case, even
+   if mentioned in a previous target_pass_signals call.   */
 
-#define target_notice_signals(ptid) \
-     (*current_target.to_notice_signals) (ptid)
+extern void target_pass_signals (int nsig, unsigned char *pass_signals);
 
 /* Check to see if a thread is still alive.  */
 

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Pedro Alves]

On Thursday 31 March 2011 20:11:49, Ulrich Weigand wrote:
> Pedro Alves wrote:
> 
> > Might be we just need to check if _any_ thread is stepping, when
> > deciding whether to tell the target to report back all signals?
> 
> I've thought about this a bit more.  The primary cause why we need
> to tell the target to report back all signals is because we have
> pulled out breakpoints to step over one -- this is why running
> arbitrary signal handlers would be dangerous.

...

> Thoughts?

This makes sense to me.  Thanks for thinking this through.

The patch looked okay to me.

-- 
Pedro Alves

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Pedro Alves wrote:
> On Thursday 31 March 2011 20:11:49, Ulrich Weigand wrote:
> > Thoughts?
> 
> This makes sense to me.  Thanks for thinking this through.
> 
> The patch looked okay to me.

Thanks for your review and comments!


I'd prefer if there were some way to test this on a profcs and a
nto-procfs target before applying the patch, though ...

Joel, I was wondering if you still have some procfs-based machines
(e.g. Solaris) where you could run a test?

Aleksandar, would it be possible for you to test the patch on a
NTO machine?

The patch in question is here:
http://sourceware.org/ml/gdb-patches/2011-03/msg01223.html

Thanks for your help!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Aleksandar Ristovski]

Hello all,


Thanks for thinking about nto :-) I haven't been able to be active
lately, but hope will come  back soon.


On Tue, 2011-04-05 at 15:22 -0400, Ulrich Weigand wrote:
> Pedro Alves wrote:
> > On Thursday 31 March 2011 20:11:49, Ulrich Weigand wrote:
> > > Thoughts?
> >
> > This makes sense to me.  Thanks for thinking this through.
> >
> > The patch looked okay to me.
> 
> Thanks for your review and comments!
> 
> 
> I'd prefer if there were some way to test this on a profcs and a
> nto-procfs target before applying the patch, though ...
> 
> Joel, I was wondering if you still have some procfs-based machines
> (e.g. Solaris) where you could run a test?
> 
> Aleksandar, would it be possible for you to test the patch on a
> NTO machine?

[AR]
yes, but you'd have to give me some time. I am over my head with work on
immediate deliveries.

How urgent do you need it?

Thanks,

Aleksandar

> 
> The patch in question is here:
> http://sourceware.org/ml/gdb-patches/2011-03/msg01223.html
> 
> Thanks for your help!
> 
> Bye,
> Ulrich
> 
> --
>   Dr. Ulrich Weigand
>   GNU Toolchain for Linux on System z and Cell BE
>   Ulrich.Weigand@de.ibm.com
> 
> 

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Aleksandar Ristovski wrote:
> > Aleksandar, would it be possible for you to test the patch on a
> > NTO machine?
> 
> [AR]
> yes, but you'd have to give me some time. I am over my head with work on
> immediate deliveries.

Sure, thanks for helping with the test!
 
> How urgent do you need it?

There's no fixed deadline -- I was hoping to get this resolved maybe
by end of the month or so, but whenever you get around to it is of
course fine with me ...

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Aleksandar Ristovski]

On Tue, 2011-04-05 at 18:56 -0400, Ulrich Weigand wrote:
> Aleksandar Ristovski wrote:
> > > Aleksandar, would it be possible for you to test the patch on a
> > > NTO machine?
> >
> > [AR]
> > yes, but you'd have to give me some time. I am over my head with
> work on
> > immediate deliveries.
> 
> Sure, thanks for helping with the test!
> 
> > How urgent do you need it?
> 
> There's no fixed deadline -- I was hoping to get this resolved maybe
> by end of the month or so, but whenever you get around to it is of
> course fine with me ...


I should be able to do it by the end of the month (probably close to the
end - sorry for the delay).


Thanks,

Aleksandar

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Joel Brobecker]

> Joel, I was wondering if you still have some procfs-based machines
> (e.g. Solaris) where you could run a test?

I can't test on Solaris because it causes any of our Solaris machines
to crash really badly. I can try mips-irix, although the results
on mips-irix are not as stellar as GNU/Linux (even if the debugger
itself is certainly quite good in general, I think).  I should be
able to do that sometime next week, probably mid-week, if you don't
find a more convenient solution. I think Pierre might have access
to an OpenSolaris box that he used to do some testing at some point?

-- 
Joel

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Aleksandar Ristovski wrote:

> I should be able to do it by the end of the month (probably close to the
> end - sorry for the delay).

No problem, thanks again for your help!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Joel Brobecker wrote:
> > Joel, I was wondering if you still have some procfs-based machines
> > (e.g. Solaris) where you could run a test?
> 
> I can't test on Solaris because it causes any of our Solaris machines
> to crash really badly. I can try mips-irix, although the results
> on mips-irix are not as stellar as GNU/Linux (even if the debugger
> itself is certainly quite good in general, I think).  I should be
> able to do that sometime next week, probably mid-week, if you don't
> find a more convenient solution. I think Pierre might have access
> to an OpenSolaris box that he used to do some testing at some point?

Sometime next week is certainly fine with me, I'll be out next week
anyway ...   Thanks for your help!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Aleksandar Ristovski wrote:

> I tested your patch on self-hosted x86 Neutrino and it did not show any 
> regressions.

Great!  Thanks for your support.

> BTW, can you incorporate this in your patch to remove compile issues:
> 
> @@ -72,9 +72,11 @@ static ptid_t do_attach (ptid_t ptid);
> 
>   static int procfs_can_use_hw_breakpoint (int, int, int);
> 
> -static int procfs_insert_hw_watchpoint (CORE_ADDR addr, int len, int type);
> +static int procfs_insert_hw_watchpoint (CORE_ADDR addr, int len, int type,
> +                                       struct expression *);
> 
> -static int procfs_remove_hw_watchpoint (CORE_ADDR addr, int len, int type);
> +static int procfs_remove_hw_watchpoint (CORE_ADDR addr, int len, int type,
> +                                       struct expression *);
> 
>   static int procfs_stopped_by_watchpoint (void);

Sure; I've checked this part in right away.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Joel Brobecker]

> Joel, I was wondering if you still have some procfs-based machines
> (e.g. Solaris) where you could run a test?

I finally found the time to test this patch on mips-irix, and the
testsuite revealed no regression.  I have a few FAIL that turned
into PASS, but these are thread testcases, so it could be by pure
accident. Sorry for taking so long for doing this...

Results on mips-irix are not as bad as I thought they might be.
I don't have a C++ compiler on this platform, so it accounts
for some of the failures.  But otherwise, here are the totals:

     # of expected passes            11461
     # of unexpected failures        357
     # of unexpected successes       3
     # of expected failures          57
     # of untested testcases         119
     # of unresolved testcases       5
     # of unsupported tests          56

-- 
Joel

Re: [rfc v2][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Ulrich Weigand]

Joel Brobecker wrote:
> > Joel, I was wondering if you still have some procfs-based machines
> > (e.g. Solaris) where you could run a test?
> 
> I finally found the time to test this patch on mips-irix, and the
> testsuite revealed no regression.  I have a few FAIL that turned
> into PASS, but these are thread testcases, so it could be by pure
> accident. Sorry for taking so long for doing this...

Great!  Thanks for you support with this!

I've now checked in the patch.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com