public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 3/7] infrun.c: Don't set ecs->random_signal for "catchpoint" events (eliminate ecs->random_signal).
  2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
@ 2013-10-31 21:32 ` Pedro Alves
  2013-10-31 21:32 ` [PATCH 4/7] infrun.c:handle_inferior_event: Rework random signal checks Pedro Alves
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-31 21:32 UTC (permalink / raw)
  To: gdb-patches

This goes a step forward in making only TARGET_WAITKIND_STOPPED talk
about signals.

There's no reason for the "catchpoint" TARGET_WAITKIND_XXXs to consult
bpstat about signals -- unlike breakpoints, all these events are
continuable, so we don't need to do a remove-break/step/reinsert-break
-like dance.  That means we don't actually need to run them through
process_event_stop_test (for the bpstat_what checks), and can just use
bpstat_causes_stop instead.  Note we were already using it in the
TARGET_WAITKIND_(V)FORKED cases.

Then, these "catchpoint" waitkinds don't need to set
ecs->random_signal for anything, because they check it immediately
afterwards (and the value they set is never used again).

gdb/
2013-10-31  Pedro Alves  <palves@redhat.com>

	* infrun.c (struct execution_control_state): Remove
	'random_signal' field.
	(handle_syscall_event): Use bpstat_causes_stop instead of
	bpstat_explains_signal.  Don't set ecs->random_signal.
	(handle_inferior_event): New 'random_signal' local.
	<TARGET_WAITKIND_FORKED, TARGET_WAITKIND_VFORKED,
	TARGET_WAITKIND_EXECD>: Use bpstat_causes_stop instead of
	bpstat_explains_signal.  Don't set ecs->random_signal.
	<TARGET_WAITKIND_STOPPED>: Adjust to use local instead of
	ecs->random_signal.
---
 gdb/infrun.c | 45 +++++++++++++--------------------------------
 1 file changed, 13 insertions(+), 32 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 8ab6b61..f37d881 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2419,7 +2419,6 @@ struct execution_control_state
   struct thread_info *event_thread;
 
   struct target_waitstatus ws;
-  int random_signal;
   int stop_func_filled_in;
   CORE_ADDR stop_func_start;
   CORE_ADDR stop_func_end;
@@ -3094,8 +3093,6 @@ handle_syscall_event (struct execution_control_state *ecs)
   if (catch_syscall_enabled () > 0
       && catching_syscall_number (syscall_number) > 0)
     {
-      enum bpstat_signal_value sval;
-
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: syscall number = '%d'\n",
                             syscall_number);
@@ -3104,11 +3101,7 @@ handle_syscall_event (struct execution_control_state *ecs)
 	= bpstat_stop_status (get_regcache_aspace (regcache),
 			      stop_pc, ecs->ptid, &ecs->ws);
 
-      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
-				     GDB_SIGNAL_0);
-      ecs->random_signal = sval == BPSTAT_SIGNAL_NO;
-
-      if (!ecs->random_signal)
+      if (bpstat_causes_stop (ecs->event_thread->control.stop_bpstat))
 	{
 	  /* Catchpoint hit.  */
 	  return 0;
@@ -3160,6 +3153,8 @@ handle_inferior_event (struct execution_control_state *ecs)
   int stopped_by_watchpoint;
   int stepped_after_stopped_by_watchpoint = 0;
   enum stop_kind stop_soon;
+  int random_signal;
+  enum bpstat_signal_value sval;
 
   if (ecs->ws.kind == TARGET_WAITKIND_IGNORE)
     {
@@ -3339,7 +3334,6 @@ handle_inferior_event (struct execution_control_state *ecs)
       if (stop_soon == NO_STOP_QUIETLY)
 	{
 	  struct regcache *regcache;
-	  enum bpstat_signal_value sval;
 
 	  regcache = get_thread_regcache (ecs->ptid);
 
@@ -3349,12 +3343,7 @@ handle_inferior_event (struct execution_control_state *ecs)
 	    = bpstat_stop_status (get_regcache_aspace (regcache),
 				  stop_pc, ecs->ptid, &ecs->ws);
 
-	  sval
-	    = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
-				      GDB_SIGNAL_0);
-	  ecs->random_signal = sval == BPSTAT_SIGNAL_NO;
-
-	  if (!ecs->random_signal)
+	  if (bpstat_causes_stop (ecs->event_thread->control.stop_bpstat))
 	    {
 	      /* A catchpoint triggered.  */
 	      process_event_stop_test (ecs);
@@ -3593,15 +3582,11 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
 			      stop_pc, ecs->ptid, &ecs->ws);
 
-      /* Note that we're interested in knowing the bpstat actually
-	 causes a stop, not just if it may explain the signal.
-	 Software watchpoints, for example, always appear in the
-	 bpstat.  */
-      ecs->random_signal
-	= !bpstat_causes_stop (ecs->event_thread->control.stop_bpstat);
-
-      /* If no catchpoint triggered for this, then keep going.  */
-      if (ecs->random_signal)
+      /* If no catchpoint triggered for this, then keep going.  Note
+	 that we're interested in knowing the bpstat actually causes a
+	 stop, not just if it may explain the signal.  Software
+	 watchpoints, for example, always appear in the bpstat.  */
+      if (!bpstat_causes_stop (ecs->event_thread->control.stop_bpstat))
 	{
 	  ptid_t parent;
 	  ptid_t child;
@@ -3687,10 +3672,6 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       ecs->event_thread->control.stop_bpstat
 	= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
 			      stop_pc, ecs->ptid, &ecs->ws);
-      ecs->random_signal
-	= (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
-				   GDB_SIGNAL_0)
-	   == BPSTAT_SIGNAL_NO);
 
       /* Note that this may be referenced from inside
 	 bpstat_stop_status above, through inferior_has_execd.  */
@@ -3698,7 +3679,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       ecs->ws.value.execd_pathname = NULL;
 
       /* If no catchpoint triggered for this, then keep going.  */
-      if (ecs->random_signal)
+      if (!bpstat_causes_stop (ecs->event_thread->control.stop_bpstat))
 	{
 	  ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
 	  keep_going (ecs);
@@ -4274,7 +4255,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
      SPARC.  */
 
   if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
-    ecs->random_signal
+    random_signal
       = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
 				   GDB_SIGNAL_TRAP)
 	   != BPSTAT_SIGNAL_NO)
@@ -4289,7 +4270,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 
       sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
 				     ecs->event_thread->suspend.stop_signal);
-      ecs->random_signal = (sval == BPSTAT_SIGNAL_NO);
+      random_signal = (sval == BPSTAT_SIGNAL_NO);
 
       if (sval == BPSTAT_SIGNAL_HIDE)
 	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
@@ -4298,7 +4279,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
   /* For the program's own signals, act according to
      the signal handling tables.  */
 
-  if (ecs->random_signal)
+  if (random_signal)
     {
       /* Signal not for debugging purposes.  */
       int printed = 0;
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/7] infrun.c:handle_inferior_event: Don't fall through in TARGET_WAITKIND_LOADED handling.
  2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
                   ` (4 preceding siblings ...)
  2013-10-31 21:32 ` [PATCH 5/7] Eliminate enum bpstat_signal_value, simplify random signal checks further Pedro Alves
@ 2013-10-31 21:32 ` Pedro Alves
  2013-10-31 22:24 ` [PATCH 7/7] infrun.c:handle_signal_stop: Move initial connection/attachment handling code earlier Pedro Alves
  2013-11-14 20:53 ` [PATCH 0/7] More run control cleanup Pedro Alves
  7 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-31 21:32 UTC (permalink / raw)
  To: gdb-patches

Of all the TARGET_WAITKIND_XXXs event kinds other than
TARGET_WAITKIND_STOPPED, TARGET_WAITKIND_LOADED is the only kind that
doesn't end in a return, instead falling through to all the
signal/breakpoint/stepping handling code.  But it only falls through
in the STOP_QUIETLY_NO_SIGSTOP and STOP_QUIETLY_REMOTE cases, which
means the

  /* This is originated from start_remote(), start_inferior() and
     shared libraries hook functions.  */
  if (stop_soon == STOP_QUIETLY || stop_soon == STOP_QUIETLY_REMOTE)
    {
      if (debug_infrun)
	fprintf_unfiltered (gdb_stdlog, "infrun: quietly stopped\n");
      stop_stepping (ecs);
      return;
    }

bit is eventually reached.  All tests before that is reached will
always fail.  It's simpler to inline the stop_soon checks close to the
TARGET_WAITKIND_LOADED code, which allows removing the fall through.

Tested on x86_64 Fedora 17, but that doesn't exercise this
TARGET_WAITKIND_LOADED.

Also ran gdb.base/solib-disc.exp on Cygwin/gdbserver, which exercises
reconnection while the inferior is stopped at an solib event, but then
again, gdbserver always replies a regular trap on initial connection,
instead of the last event the program had seen:

 Sending packet: $?#3f...Packet received: T0505:4ca72800;04:f8a62800;08:62fcc877;thread:d28;
 Sending packet: $Hc-1#09...Packet received: E01
 Sending packet: $qAttached#8f...Packet received: 0
 Packet qAttached (query-attached) is supported
 infrun: clear_proceed_status_thread (Thread 3368)
 Sending packet: $qOffsets#4b...Packet received:
 infrun: wait_for_inferior ()
 infrun: target_wait (-1, status) =
 infrun:   42000 [Thread 3368],
 infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
 infrun: infwait_normal_state
 infrun: TARGET_WAITKIND_STOPPED
 infrun: stop_pc = 0x77c8fc62
 infrun: quietly stopped
 infrun: stop_stepping

So the only way to exercise this would be to hack gdbserver.  I didn't
go that far though.  I'm reasonably confident this is correct.

gdb/
2013-10-31  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_inferior_event) <TARGET_WAITKIND_LOADED>:
	Handle STOP_QUIETLY_NO_SIGSTOP and STOP_QUIETLY_REMOTE here.
	Assert we never fall through out of the TARGET_WAITKIND_LOADED
	case.
---
 gdb/infrun.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index c9e2fe2..ba55686 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3319,6 +3319,8 @@ handle_inferior_event (struct execution_control_state *ecs)
     case TARGET_WAITKIND_LOADED:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_LOADED\n");
+      if (!ptid_equal (ecs->ptid, inferior_ptid))
+	context_switch (ecs->ptid);
       /* Ignore gracefully during startup of the inferior, as it might
          be the shell which has just loaded some objects, otherwise
          add the symbols for the newly loaded objects.  Also ignore at
@@ -3330,8 +3332,6 @@ handle_inferior_event (struct execution_control_state *ecs)
 	  struct regcache *regcache;
 	  enum bpstat_signal_value sval;
 
-	  if (!ptid_equal (ecs->ptid, inferior_ptid))
-	    context_switch (ecs->ptid);
 	  regcache = get_thread_regcache (ecs->ptid);
 
 	  handle_solib_event ();
@@ -3370,13 +3370,9 @@ handle_inferior_event (struct execution_control_state *ecs)
 
       /* If we are skipping through a shell, or through shared library
 	 loading that we aren't interested in, resume the program.  If
-	 we're running the program normally, also resume.  But stop if
-	 we're attaching or setting up a remote connection.  */
+	 we're running the program normally, also resume.  */
       if (stop_soon == STOP_QUIETLY || stop_soon == NO_STOP_QUIETLY)
 	{
-	  if (!ptid_equal (ecs->ptid, inferior_ptid))
-	    context_switch (ecs->ptid);
-
 	  /* Loading of shared libraries might have changed breakpoint
 	     addresses.  Make sure new breakpoints are inserted.  */
 	  if (stop_soon == NO_STOP_QUIETLY
@@ -3387,7 +3383,19 @@ handle_inferior_event (struct execution_control_state *ecs)
 	  return;
 	}
 
-      break;
+      /* But stop if we're attaching or setting up a remote
+	 connection.  */
+      if (stop_soon == STOP_QUIETLY_NO_SIGSTOP
+	  || stop_soon == STOP_QUIETLY_REMOTE)
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog, "infrun: quietly stopped\n");
+	  stop_stepping (ecs);
+	  return;
+	}
+
+      internal_error (__FILE__, __LINE__,
+		      _("unhandled stop_soon: %d"), (int) stop_soon);
 
     case TARGET_WAITKIND_SPURIOUS:
       if (debug_infrun)
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 6/7] infrun.c: Split handle_inferior_event further.
  2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
  2013-10-31 21:32 ` [PATCH 3/7] infrun.c: Don't set ecs->random_signal for "catchpoint" events (eliminate ecs->random_signal) Pedro Alves
  2013-10-31 21:32 ` [PATCH 4/7] infrun.c:handle_inferior_event: Rework random signal checks Pedro Alves
@ 2013-10-31 21:32 ` Pedro Alves
  2013-10-31 21:32 ` [PATCH 2/7] infrun.c:handle_inferior_event: Move comment Pedro Alves
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-31 21:32 UTC (permalink / raw)
  To: gdb-patches

After the previous patches, we only ever reach the code after the
initial 'switch (ecs->ws.kind)' switch for TARGET_WAITKIND_STOPPED.
We can now factor out all that to its own function.

Unfortunately, stepped_after_stopped_by_watchpoint needed to move to
the ecs.  I think that indicates a state machine bug -- no event other
than TARGET_WAITKIND_STOPPED indicates a single-step actually
finished.  TARGET_WAITKIND_SYSCALL_XXX, TARGET_WAITKIND_FORK, etc. are
all events that are triggered from the kernel, _within_ a syscall,
IOW, from userspace's perspective, halfway through an instruction
being executed.  This might actually matter for the syscall events, as
syscalls can change memory (and thus trigger watchpoints).

gdb/
2013-10-31  Pedro Alves  <palves@redhat.com>

	* infrun.c (struct execution_control_state)
	<stepped_after_stopped_by_watchpoint>: New field.
	(get_inferior_stop_soon): New function.
	(handle_inferior_event): 'stepped_after_stopped_by_watchpoint' was
	moved to struct execution_control_state -- adjust.  Use
	get_inferior_stop_soon.  Split TARGET_WAITKIND_STOPPED handling to
	new function.
	(handle_signal_stop): New function, factored out from
	handle_inferior_event.
---
 gdb/infrun.c | 59 ++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4d26e37..52bb064 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2424,6 +2424,11 @@ struct execution_control_state
   CORE_ADDR stop_func_end;
   const char *stop_func_name;
   int wait_some_more;
+
+  /* We were in infwait_step_watch_state or
+     infwait_nonstep_watch_state state, and the thread reported an
+     event.  */
+  int stepped_after_stopped_by_watchpoint;
 };
 
 static void handle_inferior_event (struct execution_control_state *ecs);
@@ -2432,6 +2437,7 @@ static void handle_step_into_function (struct gdbarch *gdbarch,
 				       struct execution_control_state *ecs);
 static void handle_step_into_function_backward (struct gdbarch *gdbarch,
 						struct execution_control_state *ecs);
+static void handle_signal_stop (struct execution_control_state *ecs);
 static void check_exception_resume (struct execution_control_state *,
 				    struct frame_info *);
 
@@ -3132,6 +3138,18 @@ fill_in_stop_func (struct gdbarch *gdbarch,
     }
 }
 
+
+/* Return the STOP_SOON field of the inferior pointed at by PTID.  */
+
+static enum stop_kind
+get_inferior_stop_soon (ptid_t ptid)
+{
+  struct inferior *inf = find_inferior_pid (ptid_get_pid (ptid));
+
+  gdb_assert (inf != NULL);
+  return inf->control.stop_soon;
+}
+
 /* Given an execution control state that has been freshly filled in by
    an event from the inferior, figure out what it means and take
    appropriate action.
@@ -3148,12 +3166,7 @@ fill_in_stop_func (struct gdbarch *gdbarch,
 static void
 handle_inferior_event (struct execution_control_state *ecs)
 {
-  struct frame_info *frame;
-  struct gdbarch *gdbarch;
-  int stopped_by_watchpoint;
-  int stepped_after_stopped_by_watchpoint = 0;
   enum stop_kind stop_soon;
-  int random_signal;
 
   if (ecs->ws.kind == TARGET_WAITKIND_IGNORE)
     {
@@ -3187,18 +3200,6 @@ handle_inferior_event (struct execution_control_state *ecs)
       return;
     }
 
-  if (ecs->ws.kind != TARGET_WAITKIND_EXITED
-      && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED
-      && ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED)
-    {
-      struct inferior *inf = find_inferior_pid (ptid_get_pid (ecs->ptid));
-
-      gdb_assert (inf);
-      stop_soon = inf->control.stop_soon;
-    }
-  else
-    stop_soon = NO_STOP_QUIETLY;
-
   /* Cache the last pid/waitstatus.  */
   target_last_wait_ptid = ecs->ptid;
   target_last_waitstatus = ecs->ws;
@@ -3295,7 +3296,7 @@ handle_inferior_event (struct execution_control_state *ecs)
         fprintf_unfiltered (gdb_stdlog,
 			    "infrun: infwait_step_watch_state\n");
 
-      stepped_after_stopped_by_watchpoint = 1;
+      ecs->stepped_after_stopped_by_watchpoint = 1;
       break;
 
     case infwait_nonstep_watch_state:
@@ -3307,7 +3308,7 @@ handle_inferior_event (struct execution_control_state *ecs)
       /* FIXME-maybe: is this cleaner than setting a flag?  Does it
          handle things like signals arriving and other things happening
          in combination correctly?  */
-      stepped_after_stopped_by_watchpoint = 1;
+      ecs->stepped_after_stopped_by_watchpoint = 1;
       break;
 
     default:
@@ -3330,6 +3331,8 @@ handle_inferior_event (struct execution_control_state *ecs)
          the beginning of an attach or remote session; we will query
          the full list of libraries once the connection is
          established.  */
+
+      stop_soon = get_inferior_stop_soon (ecs->ptid);
       if (stop_soon == NO_STOP_QUIETLY)
 	{
 	  struct regcache *regcache;
@@ -3715,7 +3718,8 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_STOPPED\n");
       ecs->event_thread->suspend.stop_signal = ecs->ws.value.sig;
-      break;
+      handle_signal_stop (ecs);
+      return;
 
     case TARGET_WAITKIND_NO_HISTORY:
       if (debug_infrun)
@@ -3735,6 +3739,18 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       stop_stepping (ecs);
       return;
     }
+}
+
+/* Come here when the program has stopped with a signal.  */
+
+static void
+handle_signal_stop (struct execution_control_state *ecs)
+{
+  struct frame_info *frame;
+  struct gdbarch *gdbarch;
+  int stopped_by_watchpoint;
+  enum stop_kind stop_soon;
+  int random_signal;
 
   if (ecs->ws.kind == TARGET_WAITKIND_STOPPED)
     {
@@ -4022,7 +4038,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       singlestep_breakpoints_inserted_p = 0;
     }
 
-  if (stepped_after_stopped_by_watchpoint)
+  if (ecs->stepped_after_stopped_by_watchpoint)
     stopped_by_watchpoint = 0;
   else
     stopped_by_watchpoint = watchpoints_triggered (&ecs->ws);
@@ -4167,6 +4183,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 
   /* This is originated from start_remote(), start_inferior() and
      shared libraries hook functions.  */
+  stop_soon = get_inferior_stop_soon (ecs->ptid);
   if (stop_soon == STOP_QUIETLY || stop_soon == STOP_QUIETLY_REMOTE)
     {
       if (debug_infrun)
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/7] Eliminate enum bpstat_signal_value, simplify random signal checks further.
  2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
                   ` (3 preceding siblings ...)
  2013-10-31 21:32 ` [PATCH 2/7] infrun.c:handle_inferior_event: Move comment Pedro Alves
@ 2013-10-31 21:32 ` Pedro Alves
  2013-10-31 21:32 ` [PATCH 1/7] infrun.c:handle_inferior_event: Don't fall through in TARGET_WAITKIND_LOADED handling Pedro Alves
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-31 21:32 UTC (permalink / raw)
  To: gdb-patches

After the previous patch, there's actually no breakpoint type that
returns BPSTAT_SIGNAL_HIDE, so we can go back to having
bpstat_explains_signal return a boolean.  The signal hiding actually
disappears.

gdb/
2013-10-31  Pedro Alves  <palves@redhat.com>

	* break-catch-sig.c (signal_catchpoint_explains_signal): Adjust to
	return a boolean.
	* breakpoint.c (bpstat_explains_signal): Adjust to return a
	boolean.
	(explains_signal_watchpoint, base_breakpoint_explains_signal):
	Adjust to return a boolean.
	* breakpoint.h (enum bpstat_signal_value): Delete.
	(struct breakpoint_ops) <explains_signal>: New returns a boolean.
	(bpstat_explains_signal): Likewise.
	* infrun.c (handle_inferior_event) <random signal checks>:
	bpstat_explains_signal now returns a boolean - adjust.  No longer
	consider hiding signals.
---
 gdb/break-catch-sig.c |  4 ++--
 gdb/breakpoint.c      | 34 +++++++++++++---------------------
 gdb/breakpoint.h      | 32 ++++++--------------------------
 gdb/infrun.c          | 13 ++++---------
 4 files changed, 25 insertions(+), 58 deletions(-)

diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index 02d8b4a..c82984a 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -350,10 +350,10 @@ signal_catchpoint_print_recreate (struct breakpoint *b, struct ui_file *fp)
 /* Implement the "explains_signal" breakpoint_ops method for signal
    catchpoints.  */
 
-static enum bpstat_signal_value
+static int
 signal_catchpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
 {
-  return BPSTAT_SIGNAL_PASS;
+  return 1;
 }
 
 /* Create a new signal catchpoint.  TEMPFLAG is true if this should be
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c506fc0..ab5f26c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4221,35 +4221,27 @@ bpstat_find_breakpoint (bpstat bsp, struct breakpoint *breakpoint)
 
 /* See breakpoint.h.  */
 
-enum bpstat_signal_value
+int
 bpstat_explains_signal (bpstat bsp, enum gdb_signal sig)
 {
-  enum bpstat_signal_value result = BPSTAT_SIGNAL_NO;
-
   for (; bsp != NULL; bsp = bsp->next)
     {
-      /* Ensure that, if we ever entered this loop, then we at least
-	 return BPSTAT_SIGNAL_HIDE.  */
-      enum bpstat_signal_value newval;
-
       if (bsp->breakpoint_at == NULL)
 	{
 	  /* A moribund location can never explain a signal other than
 	     GDB_SIGNAL_TRAP.  */
 	  if (sig == GDB_SIGNAL_TRAP)
-	    newval = BPSTAT_SIGNAL_PASS;
-	  else
-	    newval = BPSTAT_SIGNAL_NO;
+	    return 1;
 	}
       else
-	newval = bsp->breakpoint_at->ops->explains_signal (bsp->breakpoint_at,
-							   sig);
-
-      if (newval > result)
-	result = newval;
+	{
+	  if (bsp->breakpoint_at->ops->explains_signal (bsp->breakpoint_at,
+							sig))
+	    return 1;
+	}
     }
 
-  return result;
+  return 0;
 }
 
 /* Put in *NUM the breakpoint number of the first breakpoint we are
@@ -10763,15 +10755,15 @@ print_recreate_watchpoint (struct breakpoint *b, struct ui_file *fp)
 /* Implement the "explains_signal" breakpoint_ops method for
    watchpoints.  */
 
-static enum bpstat_signal_value
+static int
 explains_signal_watchpoint (struct breakpoint *b, enum gdb_signal sig)
 {
   /* A software watchpoint cannot cause a signal other than
      GDB_SIGNAL_TRAP.  */
   if (b->type == bp_watchpoint && sig != GDB_SIGNAL_TRAP)
-    return BPSTAT_SIGNAL_NO;
+    return 0;
 
-  return BPSTAT_SIGNAL_PASS;
+  return 1;
 }
 
 /* The breakpoint_ops structure to be used in hardware watchpoints.  */
@@ -12879,10 +12871,10 @@ base_breakpoint_decode_linespec (struct breakpoint *b, char **s,
 
 /* The default 'explains_signal' method.  */
 
-static enum bpstat_signal_value
+static int
 base_breakpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
 {
-  return BPSTAT_SIGNAL_PASS;
+  return 1;
 }
 
 /* The default "after_condition_true" method.  */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 4a25fb7..b2ff02c 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -470,22 +470,6 @@ struct bp_location
   struct symtab *symtab;
 };
 
-/* Return values for bpstat_explains_signal.  Note that the order of
-   the constants is important here; they are compared directly in
-   bpstat_explains_signal.  */
-
-enum bpstat_signal_value
-  {
-    /* bpstat does not explain this signal.  */
-    BPSTAT_SIGNAL_NO = 0,
-
-    /* bpstat explains this signal; signal should not be delivered.  */
-    BPSTAT_SIGNAL_HIDE,
-
-    /* bpstat explains this signal; signal should be delivered.  */
-    BPSTAT_SIGNAL_PASS
-  };
-
 /* This structure is a collection of function pointers that, if available,
    will be called instead of the performing the default action for this
    bptype.  */
@@ -600,12 +584,9 @@ struct breakpoint_ops
   void (*decode_linespec) (struct breakpoint *, char **,
 			   struct symtabs_and_lines *);
 
-  /* Return true if this breakpoint explains a signal, but the signal
-     should still be delivered to the inferior.  This is used to make
-     'catch signal' interact properly with 'handle'; see
+  /* Return true if this breakpoint explains a signal.  See
      bpstat_explains_signal.  */
-  enum bpstat_signal_value (*explains_signal) (struct breakpoint *,
-					       enum gdb_signal);
+  int (*explains_signal) (struct breakpoint *, enum gdb_signal);
 
   /* Called after evaluating the breakpoint's condition,
      and only if it evaluated true.  */
@@ -1002,11 +983,10 @@ struct bpstat_what bpstat_what (bpstat);
 /* Find the bpstat associated with a breakpoint.  NULL otherwise.  */
 bpstat bpstat_find_breakpoint (bpstat, struct breakpoint *);
 
-/* Nonzero if a signal that we got in wait() was due to circumstances
-   explained by the bpstat; and the signal should therefore not be
-   delivered.  */
-extern enum bpstat_signal_value bpstat_explains_signal (bpstat,
-							enum gdb_signal);
+/* Nonzero if a signal that we got in target_wait() was due to
+   circumstances explained by the bpstat; the signal is therefore not
+   random.  */
+extern int bpstat_explains_signal (bpstat, enum gdb_signal);
 
 /* Nonzero is this bpstat causes a stop.  */
 extern int bpstat_causes_stop (bpstat);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 8eb2ddd..4d26e37 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3154,7 +3154,6 @@ handle_inferior_event (struct execution_control_state *ecs)
   int stepped_after_stopped_by_watchpoint = 0;
   enum stop_kind stop_soon;
   int random_signal;
-  enum bpstat_signal_value sval;
 
   if (ecs->ws.kind == TARGET_WAITKIND_IGNORE)
     {
@@ -4226,9 +4225,8 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 
   if (debug_infrun
       && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
-      && (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+      && !bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
 				  GDB_SIGNAL_TRAP)
-	  == BPSTAT_SIGNAL_NO)
       && stopped_by_watchpoint)
     fprintf_unfiltered (gdb_stdlog,
 			"infrun: no user watchpoint explains "
@@ -4255,9 +4253,9 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
      SPARC.  */
 
   /* See if the breakpoints module can explain the signal.  */
-  sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
-				 ecs->event_thread->suspend.stop_signal);
-  random_signal = (sval == BPSTAT_SIGNAL_NO);
+  random_signal
+    = !bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+			       ecs->event_thread->suspend.stop_signal);
 
   /* If not, perhaps stepping/nexting can.  */
   if (random_signal)
@@ -4268,9 +4266,6 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
   if (random_signal)
     random_signal = !stopped_by_watchpoint;
 
-  if (sval == BPSTAT_SIGNAL_HIDE)
-    ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
-
   /* For the program's own signals, act according to
      the signal handling tables.  */
 
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/7] infrun.c:handle_inferior_event: Rework random signal checks.
  2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
  2013-10-31 21:32 ` [PATCH 3/7] infrun.c: Don't set ecs->random_signal for "catchpoint" events (eliminate ecs->random_signal) Pedro Alves
@ 2013-10-31 21:32 ` Pedro Alves
  2013-10-31 21:32 ` [PATCH 6/7] infrun.c: Split handle_inferior_event further Pedro Alves
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-31 21:32 UTC (permalink / raw)
  To: gdb-patches

Looking at the current random signal checks:

  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
    random_signal
      = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
				   GDB_SIGNAL_TRAP)
	   != BPSTAT_SIGNAL_NO)
	  || stopped_by_watchpoint
	  || ecs->event_thread->control.trap_expected
	  || (ecs->event_thread->control.step_range_end
	      && (ecs->event_thread->control.step_resume_breakpoint
		  == NULL)));
  else
    {
      enum bpstat_signal_value sval;

      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
				     ecs->event_thread->suspend.stop_signal);
      random_signal = (sval == BPSTAT_SIGNAL_NO);

      if (sval == BPSTAT_SIGNAL_HIDE)
	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
    }

We can observe:

  - the stepping checks bit:

          ...
	  || ecs->event_thread->control.trap_expected
	  || (ecs->event_thread->control.step_range_end
	      && (ecs->event_thread->control.step_resume_breakpoint
		  == NULL)));
          ...

    is just like currently_stepping:

     static int
     currently_stepping (struct thread_info *tp)
     {
       return ((tp->control.step_range_end
                && tp->control.step_resume_breakpoint == NULL)
               || tp->control.trap_expected
               || bpstat_should_step ());
     }

    except it misses the bpstat_should_step check (***).

    It's not really necessary to check bpstat_should_step in the
    random signal tests, because software watchpoints always end up in
    the bpstat list anyway, which means bpstat_explains_signal with
    GDB_SIGNAL_TRAP always returns at least BPSSTAT_SIGNAL_HIDE, but I
    think the code is clearer if we reuse currently_stepping.

    *** - bpstat_should_step checks to see if there's any software
    watchpoint in the breakpoint list, because we need to force the
    target to single-step all the way, to evaluate the watchpoint's
    value at each step.

  - we never hide GDB_SIGNAL_TRAP, even if the bpstat returns
    BPSTAT_SIGNAL_HIDE, which is actually the default for all
    breakpoints.  If we make the default be BPSTAT_SIGNAL_PASS, then
    we can merge the two bpstat_explains_signal paths.

gdb/
2013-10-31  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (bpstat_explains_signal) <Moribund locations>:
	Return BPSTAT_SIGNAL_PASS instead of BPSTAT_SIGNAL_HIDE.
	(explains_signal_watchpoint): Return BPSTAT_SIGNAL_PASS instead of
	BPSTAT_SIGNAL_HIDE.
	(base_breakpoint_explains_signal): Return BPSTAT_SIGNAL_PASS
	instead of BPSTAT_SIGNAL_HIDE.
	* infrun.c (handle_inferior_event): Rework random signal checks.
---
 gdb/breakpoint.c |  6 +++---
 gdb/infrun.c     | 35 +++++++++++++++--------------------
 2 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1782c99..c506fc0 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4237,7 +4237,7 @@ bpstat_explains_signal (bpstat bsp, enum gdb_signal sig)
 	  /* A moribund location can never explain a signal other than
 	     GDB_SIGNAL_TRAP.  */
 	  if (sig == GDB_SIGNAL_TRAP)
-	    newval = BPSTAT_SIGNAL_HIDE;
+	    newval = BPSTAT_SIGNAL_PASS;
 	  else
 	    newval = BPSTAT_SIGNAL_NO;
 	}
@@ -10771,7 +10771,7 @@ explains_signal_watchpoint (struct breakpoint *b, enum gdb_signal sig)
   if (b->type == bp_watchpoint && sig != GDB_SIGNAL_TRAP)
     return BPSTAT_SIGNAL_NO;
 
-  return BPSTAT_SIGNAL_HIDE;
+  return BPSTAT_SIGNAL_PASS;
 }
 
 /* The breakpoint_ops structure to be used in hardware watchpoints.  */
@@ -12882,7 +12882,7 @@ base_breakpoint_decode_linespec (struct breakpoint *b, char **s,
 static enum bpstat_signal_value
 base_breakpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
 {
-  return BPSTAT_SIGNAL_HIDE;
+  return BPSTAT_SIGNAL_PASS;
 }
 
 /* The default "after_condition_true" method.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index f37d881..8eb2ddd 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4234,7 +4234,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 			"infrun: no user watchpoint explains "
 			"watchpoint SIGTRAP, ignoring\n");
 
-  /* NOTE: cagney/2003-03-29: These two checks for a random signal
+  /* NOTE: cagney/2003-03-29: These checks for a random signal
      at one stage in the past included checks for an inferior
      function call's call dummy's return breakpoint.  The original
      comment, that went with the test, read:
@@ -4254,27 +4254,22 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
      be necessary for call dummies on a non-executable stack on
      SPARC.  */
 
-  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
-    random_signal
-      = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
-				   GDB_SIGNAL_TRAP)
-	   != BPSTAT_SIGNAL_NO)
-	  || stopped_by_watchpoint
-	  || ecs->event_thread->control.trap_expected
-	  || (ecs->event_thread->control.step_range_end
-	      && (ecs->event_thread->control.step_resume_breakpoint
-		  == NULL)));
-  else
-    {
-      enum bpstat_signal_value sval;
+  /* See if the breakpoints module can explain the signal.  */
+  sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+				 ecs->event_thread->suspend.stop_signal);
+  random_signal = (sval == BPSTAT_SIGNAL_NO);
+
+  /* If not, perhaps stepping/nexting can.  */
+  if (random_signal)
+    random_signal = !(ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
+		      && currently_stepping (ecs->event_thread));
 
-      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
-				     ecs->event_thread->suspend.stop_signal);
-      random_signal = (sval == BPSTAT_SIGNAL_NO);
+  /* No?  Perhaps we got a moribund watchpoint.  */
+  if (random_signal)
+    random_signal = !stopped_by_watchpoint;
 
-      if (sval == BPSTAT_SIGNAL_HIDE)
-	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
-    }
+  if (sval == BPSTAT_SIGNAL_HIDE)
+    ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
 
   /* For the program's own signals, act according to
      the signal handling tables.  */
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/7] More run control cleanup.
@ 2013-10-31 21:32 Pedro Alves
  2013-10-31 21:32 ` [PATCH 3/7] infrun.c: Don't set ecs->random_signal for "catchpoint" events (eliminate ecs->random_signal) Pedro Alves
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-31 21:32 UTC (permalink / raw)
  To: gdb-patches

Here's another batch of fixes / cleanups to handle_inferior_event &
co.  It ends up splitting handle_inferior_event further in another two
halves around 600 lines each, and cleaning up random signal handling.
I think the flow ends a good chunk clearer afterwards.

Tested on x86_64 Fedora 17, native and gdbserver.

Comments?

Pedro Alves (7):
  infrun.c:handle_inferior_event: Don't fall through in
    TARGET_WAITKIND_LOADED handling.
  infrun.c:handle_inferior_event: Move comment.
  infrun.c: Don't set ecs->random_signal for "catchpoint" events
    (eliminate ecs->random_signal).
  infrun.c:handle_inferior_event: Rework random signal checks.
  Eliminate enum bpstat_signal_value, simplify random signal checks
    further.
  infrun.c: Split handle_inferior_event further.
  infrun.c:handle_signal_stop: Move initial connection/attachment
    handling code earlier.

 gdb/break-catch-sig.c |   4 +-
 gdb/breakpoint.c      |  34 +++---
 gdb/breakpoint.h      |  32 ++----
 gdb/infrun.c          | 285 +++++++++++++++++++++++++-------------------------
 4 files changed, 165 insertions(+), 190 deletions(-)

-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/7] infrun.c:handle_inferior_event: Move comment.
  2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
                   ` (2 preceding siblings ...)
  2013-10-31 21:32 ` [PATCH 6/7] infrun.c: Split handle_inferior_event further Pedro Alves
@ 2013-10-31 21:32 ` Pedro Alves
  2013-10-31 21:32 ` [PATCH 5/7] Eliminate enum bpstat_signal_value, simplify random signal checks further Pedro Alves
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-31 21:32 UTC (permalink / raw)
  To: gdb-patches

This comment applies to the whole handle_inferior_event flow, top to
bottom.  Best move it to the function's intro.

gdb/
2013-10-31  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_inferior_event): Move comment from the
	function's body to the function's description, adjusted.
---
 gdb/infrun.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index ba55686..8ab6b61 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3139,9 +3139,18 @@ fill_in_stop_func (struct gdbarch *gdbarch,
     }
 }
 
-/* Given an execution control state that has been freshly filled in
-   by an event from the inferior, figure out what it means and take
-   appropriate action.  */
+/* Given an execution control state that has been freshly filled in by
+   an event from the inferior, figure out what it means and take
+   appropriate action.
+
+   The alternatives are:
+
+   1) stop_stepping and return; to really stop and return to the
+   debugger.
+
+   2) keep_going and return; to wait for the next event (set
+   ecs->event_thread->stepping_over_breakpoint to 1 to single step
+   once).  */
 
 static void
 handle_inferior_event (struct execution_control_state *ecs)
@@ -4166,14 +4175,6 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	}
     }
 
-  /* Look at the cause of the stop, and decide what to do.
-     The alternatives are:
-     1) stop_stepping and return; to really stop and return to the debugger,
-     2) keep_going and return to start up again
-     (set ecs->event_thread->stepping_over_breakpoint to 1 to single step once)
-     3) set ecs->random_signal to 1, and the decision between 1 and 2
-     will be made according to the signal handling tables.  */
-
   if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
       && stop_after_trap)
     {
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 7/7] infrun.c:handle_signal_stop: Move initial connection/attachment handling code earlier.
  2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
                   ` (5 preceding siblings ...)
  2013-10-31 21:32 ` [PATCH 1/7] infrun.c:handle_inferior_event: Don't fall through in TARGET_WAITKIND_LOADED handling Pedro Alves
@ 2013-10-31 22:24 ` Pedro Alves
  2013-11-14 20:53 ` [PATCH 0/7] More run control cleanup Pedro Alves
  7 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-31 22:24 UTC (permalink / raw)
  To: gdb-patches

Before all this stop_soon handling, we have code that can end in
keep_going.  Particularly, the thread_hop_needed code looked
suspicious considering breakpoint always-inserted mode, though on
closer inspection, it'd take connecting to multiple remote targets
that shared the same address space to trigger that.

Still, I think it's clearer if all this remote connection setup /
attach code is placed early, before any keep_going path could be
reached.

gdb/
2013-10-31  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_signal_stop): Move STOP_QUIETLY,
	STOP_QUIETLY_REMOTE and 'stop_after_trap' handling earlier.
---
 gdb/infrun.c | 108 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 51 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 52bb064..0b8dffe 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3799,6 +3799,63 @@ handle_signal_stop (struct execution_control_state *ecs)
       do_cleanups (old_chain);
     }
 
+  /* This is originated from start_remote(), start_inferior() and
+     shared libraries hook functions.  */
+  stop_soon = get_inferior_stop_soon (ecs->ptid);
+  if (stop_soon == STOP_QUIETLY || stop_soon == STOP_QUIETLY_REMOTE)
+    {
+      if (!ptid_equal (ecs->ptid, inferior_ptid))
+	context_switch (ecs->ptid);
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog, "infrun: quietly stopped\n");
+      stop_print_frame = 1;
+      stop_stepping (ecs);
+      return;
+    }
+
+  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
+      && stop_after_trap)
+    {
+      if (!ptid_equal (ecs->ptid, inferior_ptid))
+	context_switch (ecs->ptid);
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog, "infrun: stopped\n");
+      stop_print_frame = 0;
+      stop_stepping (ecs);
+      return;
+    }
+
+  /* This originates from attach_command().  We need to overwrite
+     the stop_signal here, because some kernels don't ignore a
+     SIGSTOP in a subsequent ptrace(PTRACE_CONT,SIGSTOP) call.
+     See more comments in inferior.h.  On the other hand, if we
+     get a non-SIGSTOP, report it to the user - assume the backend
+     will handle the SIGSTOP if it should show up later.
+
+     Also consider that the attach is complete when we see a
+     SIGTRAP.  Some systems (e.g. Windows), and stubs supporting
+     target extended-remote report it instead of a SIGSTOP
+     (e.g. gdbserver).  We already rely on SIGTRAP being our
+     signal, so this is no exception.
+
+     Also consider that the attach is complete when we see a
+     GDB_SIGNAL_0.  In non-stop mode, GDB will explicitly tell
+     the target to stop all threads of the inferior, in case the
+     low level attach operation doesn't stop them implicitly.  If
+     they weren't stopped implicitly, then the stub will report a
+     GDB_SIGNAL_0, meaning: stopped for no particular reason
+     other than GDB's request.  */
+  if (stop_soon == STOP_QUIETLY_NO_SIGSTOP
+      && (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_STOP
+	  || ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
+	  || ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_0))
+    {
+      stop_print_frame = 1;
+      stop_stepping (ecs);
+      ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
+      return;
+    }
+
   if (stepping_past_singlestep_breakpoint)
     {
       gdb_assert (singlestep_breakpoints_inserted_p);
@@ -4171,57 +4228,6 @@ handle_signal_stop (struct execution_control_state *ecs)
 	}
     }
 
-  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
-      && stop_after_trap)
-    {
-      if (debug_infrun)
-	fprintf_unfiltered (gdb_stdlog, "infrun: stopped\n");
-      stop_print_frame = 0;
-      stop_stepping (ecs);
-      return;
-    }
-
-  /* This is originated from start_remote(), start_inferior() and
-     shared libraries hook functions.  */
-  stop_soon = get_inferior_stop_soon (ecs->ptid);
-  if (stop_soon == STOP_QUIETLY || stop_soon == STOP_QUIETLY_REMOTE)
-    {
-      if (debug_infrun)
-	fprintf_unfiltered (gdb_stdlog, "infrun: quietly stopped\n");
-      stop_stepping (ecs);
-      return;
-    }
-
-  /* This originates from attach_command().  We need to overwrite
-     the stop_signal here, because some kernels don't ignore a
-     SIGSTOP in a subsequent ptrace(PTRACE_CONT,SIGSTOP) call.
-     See more comments in inferior.h.  On the other hand, if we
-     get a non-SIGSTOP, report it to the user - assume the backend
-     will handle the SIGSTOP if it should show up later.
-
-     Also consider that the attach is complete when we see a
-     SIGTRAP.  Some systems (e.g. Windows), and stubs supporting
-     target extended-remote report it instead of a SIGSTOP
-     (e.g. gdbserver).  We already rely on SIGTRAP being our
-     signal, so this is no exception.
-
-     Also consider that the attach is complete when we see a
-     GDB_SIGNAL_0.  In non-stop mode, GDB will explicitly tell
-     the target to stop all threads of the inferior, in case the
-     low level attach operation doesn't stop them implicitly.  If
-     they weren't stopped implicitly, then the stub will report a
-     GDB_SIGNAL_0, meaning: stopped for no particular reason
-     other than GDB's request.  */
-  if (stop_soon == STOP_QUIETLY_NO_SIGSTOP
-      && (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_STOP
-	  || ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
-	  || ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_0))
-    {
-      stop_stepping (ecs);
-      ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
-      return;
-    }
-
   /* See if there is a breakpoint/watchpoint/catchpoint/etc. that
      handles this event.  */
   ecs->event_thread->control.stop_bpstat
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/7] More run control cleanup.
  2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
                   ` (6 preceding siblings ...)
  2013-10-31 22:24 ` [PATCH 7/7] infrun.c:handle_signal_stop: Move initial connection/attachment handling code earlier Pedro Alves
@ 2013-11-14 20:53 ` Pedro Alves
  7 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-11-14 20:53 UTC (permalink / raw)
  To: gdb-patches

I pushed this.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-11-14 20:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-31 21:32 [PATCH 0/7] More run control cleanup Pedro Alves
2013-10-31 21:32 ` [PATCH 3/7] infrun.c: Don't set ecs->random_signal for "catchpoint" events (eliminate ecs->random_signal) Pedro Alves
2013-10-31 21:32 ` [PATCH 4/7] infrun.c:handle_inferior_event: Rework random signal checks Pedro Alves
2013-10-31 21:32 ` [PATCH 6/7] infrun.c: Split handle_inferior_event further Pedro Alves
2013-10-31 21:32 ` [PATCH 2/7] infrun.c:handle_inferior_event: Move comment Pedro Alves
2013-10-31 21:32 ` [PATCH 5/7] Eliminate enum bpstat_signal_value, simplify random signal checks further Pedro Alves
2013-10-31 21:32 ` [PATCH 1/7] infrun.c:handle_inferior_event: Don't fall through in TARGET_WAITKIND_LOADED handling Pedro Alves
2013-10-31 22:24 ` [PATCH 7/7] infrun.c:handle_signal_stop: Move initial connection/attachment handling code earlier Pedro Alves
2013-11-14 20:53 ` [PATCH 0/7] More run control cleanup Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).