This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 4/6] Fix for even more missed events; eliminate thread-hop code.


Even with deferred_step_ptid out of the way, GDB can still lose
watchpoints on software single-step targets.

Here in the thread-hop code:

	  if (!ptid_equal (singlestep_ptid, ecs->ptid)
	      && in_thread_list (singlestep_ptid))
	    {
	      /* If the PC of the thread we were trying to single-step
		 has changed, discard this event (which we were going
		 to ignore anyway), and pretend we saw that thread
		 trap.  This prevents us continuously moving the
		 single-step breakpoint forward, one instruction at a
		 time.  If the PC has changed, then the thread we were
		 trying to single-step has trapped or been signalled,
		 but the event has not been reported to GDB yet.

		 There might be some cases where this loses signal
		 information, if a signal has arrived at exactly the
		 same time that the PC changed, but this is the best
		 we can do with the information available.  Perhaps we
		 should arrange to report all events for all threads
		 when they stop, or to re-poll the remote looking for
		 this particular thread (i.e. temporarily enable
		 schedlock).  */

	     CORE_ADDR new_singlestep_pc
	       = regcache_read_pc (get_thread_regcache (singlestep_ptid));

	     if (new_singlestep_pc != singlestep_pc)
	       {
		 enum gdb_signal stop_signal;

		 if (debug_infrun)
		   fprintf_unfiltered (gdb_stdlog, "infrun: unexpected thread,"
				       " but expected thread advanced also\n");

		 /* The current context still belongs to
		    singlestep_ptid.  Don't swap here, since that's
		    the context we want to use.  Just fudge our
		    state and continue.  */
                 stop_signal = ecs->event_thread->suspend.stop_signal;
                 ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
                 ecs->ptid = singlestep_ptid;
                 ecs->event_thread = find_thread_ptid (ecs->ptid);
                 ecs->event_thread->suspend.stop_signal = stop_signal;
                 stop_pc = new_singlestep_pc;
               }
             else
	       {
		 if (debug_infrun)
		   fprintf_unfiltered (gdb_stdlog,
				       "infrun: unexpected thread\n");

		 thread_hop_needed = 1;
		 stepping_past_singlestep_breakpoint = 1;
		 saved_singlestep_ptid = singlestep_ptid;
	       }
	    }
	}

Note we either end up with thread_hop_needed, ignoring the watchpoint
SIGTRAP, or switch to the stepping thread, again ignoring that the
SIGTRAP could be for some other event.

So the fix is similar to the deferred_step_ptid fix -- defer the
thread hop to _after_ the SIGTRAP had a change of passing through the
regular bpstat handling.  If the wrong thread hits a breakpoint, we'll
just end up with BPSTAT_WHAT_SINGLE, and if nothing causes a stop,
keep_going starts a step-over.

But in order to do that, we need to make bpstat aware of single-step
breakpoints.  So this patch starts convering the single-step
breakpoints to real breakpoints.  It doesn't put them in the regular
breakpoint chain yet though.  More changes would be needed for that.

The stepping_past_singlestep_breakpoint mechanism is really not
necessary -- just setting the thread to step over a breakpoint with
thread->stepping_over_breakpoint=1 has the exact same effect.

Special care is still needed to handle the case of PC of the thread we
were trying to single-step having changed, like in the old code.  We
can't just keep_going and re-step it, as in that case we can over-step
the thread (if it was already done with the step, but hsn't reported
it yet, we'd ask it to step even further).  That's now handled in
switch_back_to_stepped_thread.  As bonus, we're now using a technique
that doesn't lose signals, unlike the old code -- insert a breakpoint
at PC, and resume, which either reports the breakpoint immediately, or
any pending signal.

Tested on x86_64 Fedora 17, against pristine mainline, and against a
branch that implements software single-step on x86.

gdb/
2014-02-21  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (single_step_breakpoints): Change type to struct
	breakpoint array.
	(single_step_gdbarch): Delete.
	(bpstat_stop_status): Handle single-step breakpoints.
	(bpstat_what, bptype_string, print_one_breakpoint_location)
	(init_bp_location): Handle bp_single_step.
	(init_momentary_breakpoint): New, factored out from ...
	(set_momentary_breakpoint): ... this.  Rewrite.
	(init_momentary_breakpoint_at_pc): New, factored out from ...
	(set_momentary_breakpoint_at_pc): ... this.  Rewrite.
	(insert_single_step_breakpoint, remove_single_step_breakpoints)
	(cancel_single_step_breakpoints, detach_single_step_breakpoints):
	Rewrite/adjust to insert/remove real breakpoints instead of using
	deprecated_insert_raw_breakpoint and
	deprecated_remove_raw_breakpoint.
	(single_step_breakpoint_inserted_here_p): Adjust.
	* breakpoint.h (enum bptype) <bp_single_step>: New breakpoint
	type.
	* infrun.c (saved_singlestep_ptid)
	(stepping_past_singlestep_breakpoint): Delete.
	(resume): Remove stepping_past_singlestep_breakpoint handling.
	(proceed): Store the prev_pc of the stepping thread too.
	(init_wait_for_inferior): Adjust.  Clear singlestep_ptid and
	singlestep_pc.
	(enum infwait_states): Delete infwait_thread_hop_state.
	(handle_inferior_event): Adjust.
	(handle_signal_stop): Delete stepping_past_singlestep_breakpoint
	handling and the thread-hop code.  Defer removing single-step
	breakpoints till after bpstat handling.
	(switch_back_to_stepped_thread): Handle the case of the stepped
	thread having advanced already.
---
 gdb/breakpoint.c | 191 +++++++++++++++++++++++-----------
 gdb/breakpoint.h |   1 +
 gdb/infrun.c     | 308 +++++++++++++++++++------------------------------------
 3 files changed, 238 insertions(+), 262 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b462bad..811823d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -587,6 +587,11 @@ static CORE_ADDR bp_location_shadow_len_after_address_max;
    by a target.  */
 VEC(bp_location_p) *moribund_locations = NULL;
 
+/* One (or perhaps two) breakpoints used for software single
+   stepping.  */
+
+static struct breakpoint *single_step_breakpoints[2];
+
 /* Number of last breakpoint made.  */
 
 static int breakpoint_count;
@@ -5375,6 +5380,7 @@ bpstat_stop_status (struct address_space *aspace,
 	}
     }
 
+  /* Check if a moribund breakpoint explains the stop.  */
   for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
     {
       if (breakpoint_location_address_match (loc, aspace, bp_addr))
@@ -5387,6 +5393,27 @@ bpstat_stop_status (struct address_space *aspace,
 	}
     }
 
+  /* Check if a software single-step breakpoint explains the stop.  */
+  if (ws->kind == TARGET_WAITKIND_STOPPED
+      && ws->value.sig == GDB_SIGNAL_TRAP)
+    {
+      int i;
+
+      for (i = 0; i < 2; i++)
+	{
+	  struct breakpoint *bp = single_step_breakpoints[i];
+
+	  if (bp != NULL
+	      && breakpoint_location_address_match (bp->loc, aspace, bp_addr))
+	    {
+	      bs = bpstat_alloc (bp->loc, &bs_link);
+	      bs->stop = 0;
+	      bs->print = 0;
+	      bs->print_it = print_it_noop;
+	    }
+	}
+    }
+
   /* A bit of special processing for shlib breakpoints.  We need to
      process solib loading here, so that the lists of loaded and
      unloaded libraries are correct before we handle "catch load" and
@@ -5530,6 +5557,7 @@ bpstat_what (bpstat bs_head)
 	  break;
 	case bp_breakpoint:
 	case bp_hardware_breakpoint:
+	case bp_single_step:
 	case bp_until:
 	case bp_finish:
 	case bp_shlib_event:
@@ -5888,6 +5916,7 @@ bptype_string (enum bptype type)
     {bp_none, "?deleted?"},
     {bp_breakpoint, "breakpoint"},
     {bp_hardware_breakpoint, "hw breakpoint"},
+    {bp_single_step, "sw single-step"},
     {bp_until, "until"},
     {bp_finish, "finish"},
     {bp_watchpoint, "watchpoint"},
@@ -6079,6 +6108,7 @@ print_one_breakpoint_location (struct breakpoint *b,
 
       case bp_breakpoint:
       case bp_hardware_breakpoint:
+      case bp_single_step:
       case bp_until:
       case bp_finish:
       case bp_longjmp:
@@ -6957,6 +6987,7 @@ init_bp_location (struct bp_location *loc, const struct bp_location_ops *ops,
   switch (owner->type)
     {
     case bp_breakpoint:
+    case bp_single_step:
     case bp_until:
     case bp_finish:
     case bp_longjmp:
@@ -8884,33 +8915,43 @@ enable_breakpoints_after_startup (void)
 }
 
 
-/* Set a breakpoint that will evaporate an end of command
-   at address specified by SAL.
-   Restrict it to frame FRAME if FRAME is nonzero.  */
+/* Initialize a momentary breakpoint of type TYPE at address specified
+   by SAL.  If FRAME_ID is valid, the breakpoint is restricted to that
+   frame.  The breakpoint is not inserted in the breakpoint chain.  */
 
-struct breakpoint *
-set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
-			  struct frame_id frame_id, enum bptype type)
+static struct breakpoint *
+init_momentary_breakpoint (struct breakpoint *b, struct gdbarch *gdbarch,
+			   struct symtab_and_line sal, struct frame_id frame_id,
+			   enum bptype type)
 {
-  struct breakpoint *b;
-
   /* If FRAME_ID is valid, it should be a real frame, not an inlined or
      tail-called one.  */
   gdb_assert (!frame_id_artificial_p (frame_id));
 
-  b = set_raw_breakpoint (gdbarch, sal, type, &momentary_breakpoint_ops);
+  init_raw_breakpoint (b, gdbarch, sal, type, &momentary_breakpoint_ops);
   b->enable_state = bp_enabled;
   b->disposition = disp_donttouch;
   b->frame_id = frame_id;
 
-  /* If we're debugging a multi-threaded program, then we want
-     momentary breakpoints to be active in only a single thread of
-     control.  */
-  if (in_thread_list (inferior_ptid))
-    b->thread = pid_to_thread_id (inferior_ptid);
+  b->thread = pid_to_thread_id (inferior_ptid);
+  gdb_assert (b->thread != 0);
 
-  update_global_location_list_nothrow (1);
+  return b;
+}
 
+/* Set a momentary breakpoint of type TYPE at address specified by
+   SAL.  If FRAME_ID is valid, the breakpoint is restricted to that
+   frame.  */
+
+struct breakpoint *
+set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
+			  struct frame_id frame_id, enum bptype type)
+{
+  struct breakpoint *b = XNEW (struct breakpoint);
+
+  init_momentary_breakpoint (b, gdbarch, sal, frame_id, type);
+  add_to_breakpoint_chain (b);
+  update_global_location_list_nothrow (1);
   return b;
 }
 
@@ -8962,9 +9003,12 @@ clone_momentary_breakpoint (struct breakpoint *orig)
   return momentary_breakpoint_from_master (orig, orig->type, orig->ops);
 }
 
-struct breakpoint *
-set_momentary_breakpoint_at_pc (struct gdbarch *gdbarch, CORE_ADDR pc,
-				enum bptype type)
+/* Initialize a momentary breakpoint of type TYPE at address PC.  */
+
+static void
+init_momentary_breakpoint_at_pc (struct breakpoint *b,
+				 struct gdbarch *gdbarch, CORE_ADDR pc,
+				 enum bptype type)
 {
   struct symtab_and_line sal;
 
@@ -8973,7 +9017,21 @@ set_momentary_breakpoint_at_pc (struct gdbarch *gdbarch, CORE_ADDR pc,
   sal.section = find_pc_overlay (pc);
   sal.explicit_pc = 1;
 
-  return set_momentary_breakpoint (gdbarch, sal, null_frame_id, type);
+  init_momentary_breakpoint (b, gdbarch, sal, null_frame_id, type);
+}
+
+/* Set a momentary breakpoint of type TYPE at address PC.  */
+
+struct breakpoint *
+set_momentary_breakpoint_at_pc (struct gdbarch *gdbarch, CORE_ADDR pc,
+				enum bptype type)
+{
+  struct breakpoint *b = XNEW (struct breakpoint);
+
+  init_momentary_breakpoint_at_pc (b, gdbarch, pc, type);
+  add_to_breakpoint_chain (b);
+  update_global_location_list_nothrow (1);
+  return b;
 }
 
 
@@ -15027,12 +15085,6 @@ deprecated_remove_raw_breakpoint (struct gdbarch *gdbarch, void *bp)
   return ret;
 }
 
-/* One (or perhaps two) breakpoints used for software single
-   stepping.  */
-
-static void *single_step_breakpoints[2];
-static struct gdbarch *single_step_gdbarch[2];
-
 /* Create and insert a breakpoint for software single step.  */
 
 void
@@ -15040,18 +15092,21 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch,
 			       struct address_space *aspace, 
 			       CORE_ADDR next_pc)
 {
-  void **bpt_p;
+  struct breakpoint **bp_p;
+  struct breakpoint *bp;
+  enum errors bp_err = GDB_NO_ERROR;
+  volatile struct gdb_exception ex;
+  int val;
+  struct bp_location *bl;
 
   if (single_step_breakpoints[0] == NULL)
     {
-      bpt_p = &single_step_breakpoints[0];
-      single_step_gdbarch[0] = gdbarch;
+      bp_p = &single_step_breakpoints[0];
     }
   else
     {
       gdb_assert (single_step_breakpoints[1] == NULL);
-      bpt_p = &single_step_breakpoints[1];
-      single_step_gdbarch[1] = gdbarch;
+      bp_p = &single_step_breakpoints[1];
     }
 
   /* NOTE drow/2006-04-11: A future improvement to this function would
@@ -15060,11 +15115,26 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch,
      We could adjust the addresses each time they were needed.  Doing
      this requires corresponding changes elsewhere where single step
      breakpoints are handled, however.  So, for now, we use this.  */
+  bp = XNEW (struct breakpoint);
+  init_momentary_breakpoint_at_pc (bp, gdbarch, next_pc, bp_single_step);
+  bl = bp->loc;
 
-  *bpt_p = deprecated_insert_raw_breakpoint (gdbarch, aspace, next_pc);
-  if (*bpt_p == NULL)
-    error (_("Could not insert single-step breakpoint at %s"),
+  bl->target_info.placed_address = bl->address;
+  bl->target_info.placed_address_space = bl->pspace->aspace;
+  bl->target_info.length = bl->length;
+
+  TRY_CATCH (ex, RETURN_MASK_ALL)
+    {
+      val = bp->ops->insert_location (bl);
+    }
+  if (ex.reason < 0 || val != 0)
+    {
+      delete_breakpoint (bp);
+      error (_("Could not insert single-step breakpoint at %s"),
 	     paddress (gdbarch, next_pc));
+    }
+  else
+    *bp_p = bp;
 }
 
 /* Check if the breakpoints used for software single stepping
@@ -15082,21 +15152,20 @@ single_step_breakpoints_inserted (void)
 void
 remove_single_step_breakpoints (void)
 {
-  gdb_assert (single_step_breakpoints[0] != NULL);
+  int i;
 
-  /* See insert_single_step_breakpoint for more about this deprecated
-     call.  */
-  deprecated_remove_raw_breakpoint (single_step_gdbarch[0],
-				    single_step_breakpoints[0]);
-  single_step_gdbarch[0] = NULL;
-  single_step_breakpoints[0] = NULL;
+  gdb_assert (single_step_breakpoints[0] != NULL);
 
-  if (single_step_breakpoints[1] != NULL)
+  for (i = 0; i < 2; i++)
     {
-      deprecated_remove_raw_breakpoint (single_step_gdbarch[1],
-					single_step_breakpoints[1]);
-      single_step_gdbarch[1] = NULL;
-      single_step_breakpoints[1] = NULL;
+      struct breakpoint *bp = single_step_breakpoints[i];
+
+      if (bp != NULL)
+	{
+	  bp->ops->remove_location (bp->loc);
+	  delete_breakpoint (bp);
+	  single_step_breakpoints[i] = NULL;
+	}
     }
 }
 
@@ -15111,12 +15180,15 @@ cancel_single_step_breakpoints (void)
   int i;
 
   for (i = 0; i < 2; i++)
-    if (single_step_breakpoints[i])
-      {
-	xfree (single_step_breakpoints[i]);
-	single_step_breakpoints[i] = NULL;
-	single_step_gdbarch[i] = NULL;
-      }
+    {
+      struct breakpoint *bp = single_step_breakpoints[i];
+
+      if (bp != NULL)
+	{
+	  delete_breakpoint (bp);
+	  single_step_breakpoints[i] = NULL;
+	}
+    }
 }
 
 /* Detach software single-step breakpoints from INFERIOR_PTID without
@@ -15128,9 +15200,12 @@ detach_single_step_breakpoints (void)
   int i;
 
   for (i = 0; i < 2; i++)
-    if (single_step_breakpoints[i])
-      target_remove_breakpoint (single_step_gdbarch[i],
-				single_step_breakpoints[i]);
+    {
+      struct breakpoint *bp = single_step_breakpoints[i];
+
+      if (bp != NULL)
+	bp->ops->remove_location (bp->loc);
+    }
 }
 
 /* Check whether a software single-step breakpoint is inserted at
@@ -15144,11 +15219,9 @@ single_step_breakpoint_inserted_here_p (struct address_space *aspace,
 
   for (i = 0; i < 2; i++)
     {
-      struct bp_target_info *bp_tgt = single_step_breakpoints[i];
-      if (bp_tgt
-	  && breakpoint_address_match (bp_tgt->placed_address_space,
-				       bp_tgt->placed_address,
-				       aspace, pc))
+      struct breakpoint *bp = single_step_breakpoints[i];
+
+      if (bp != NULL && breakpoint_location_address_match (bp->loc, aspace, pc))
 	return 1;
     }
 
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 42b1492..109afbe 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -58,6 +58,7 @@ enum bptype
     bp_none = 0,		/* Eventpoint has been deleted */
     bp_breakpoint,		/* Normal breakpoint */
     bp_hardware_breakpoint,	/* Hardware assisted breakpoint */
+    bp_single_step,		/* Software single-step */
     bp_until,			/* used by until command */
     bp_finish,			/* used by finish command */
     bp_watchpoint,		/* Watchpoint */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 8bfce34..1ea8d04 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -972,11 +972,6 @@ static ptid_t singlestep_ptid;
 /* PC when we started this single-step.  */
 static CORE_ADDR singlestep_pc;
 
-/* If another thread hit the singlestep breakpoint, we save the original
-   thread here so that we can resume single-stepping it later.  */
-static ptid_t saved_singlestep_ptid;
-static int stepping_past_singlestep_breakpoint;
-
 /* If stepping over a breakpoint (not displaced stepping), this is the
    address (and address space) where that breakpoint is inserted.
    When not stepping over a breakpoint, STEP_OVER_ASPACE is NULL.
@@ -1905,24 +1900,8 @@ a command like `return' or `jump' to continue execution."));
       resume_ptid = user_visible_resume_ptid (step);
 
       /* Maybe resume a single thread after all.  */
-      if (singlestep_breakpoints_inserted_p
-	  && stepping_past_singlestep_breakpoint)
-	{
-	  /* The situation here is as follows.  In thread T1 we wanted to
-	     single-step.  Lacking hardware single-stepping we've
-	     set breakpoint at the PC of the next instruction -- call it
-	     P.  After resuming, we've hit that breakpoint in thread T2.
-	     Now we've removed original breakpoint, inserted breakpoint
-	     at P+1, and try to step to advance T2 past breakpoint.
-	     We need to step only T2, as if T1 is allowed to freely run,
-	     it can run past P, and if other threads are allowed to run,
-	     they can hit breakpoint at P+1, and nested hits of single-step
-	     breakpoints is not something we'd want -- that's complicated
-	     to support, and has no value.  */
-	  resume_ptid = inferior_ptid;
-	}
-      else if ((step || singlestep_breakpoints_inserted_p)
-	       && tp->control.trap_expected)
+      if ((step || singlestep_breakpoints_inserted_p)
+	  && tp->control.trap_expected)
 	{
 	  /* We're allowing a thread to run past a breakpoint it has
 	     hit, by single-stepping the thread with the breakpoint
@@ -2187,6 +2166,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
   gdbarch = get_regcache_arch (regcache);
   aspace = get_regcache_aspace (regcache);
   pc = regcache_read_pc (regcache);
+  tp = inferior_thread ();
 
   if (step > 0)
     step_start_function = find_pc_function (pc);
@@ -2243,13 +2223,19 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
 	 thread that reported the most recent event.  If a step-over
 	 is required it returns TRUE and sets the current thread to
 	 the old thread.  */
+
+      /* Store the prev_pc for the stepping thread too, needed by
+	 switch_back_to_stepping thread.  */
+      tp->prev_pc = regcache_read_pc (get_current_regcache ());
+
       if (prepare_to_proceed (step))
-	force_step = 1;
+	{
+	  force_step = 1;
+	  /* The current thread changed.  */
+	  tp = inferior_thread ();
+	}
     }
 
-  /* prepare_to_proceed may change the current thread.  */
-  tp = inferior_thread ();
-
   if (force_step)
     tp->control.trap_expected = 1;
 
@@ -2402,8 +2388,6 @@ init_wait_for_inferior (void)
 
   clear_proceed_status ();
 
-  stepping_past_singlestep_breakpoint = 0;
-
   target_last_wait_ptid = minus_one_ptid;
 
   previous_inferior_ptid = inferior_ptid;
@@ -2411,6 +2395,9 @@ init_wait_for_inferior (void)
 
   /* Discard any skipped inlined frames.  */
   clear_inline_frame_state (minus_one_ptid);
+
+  singlestep_ptid = null_ptid;
+  singlestep_pc = 0;
 }
 
 
@@ -2421,7 +2408,6 @@ init_wait_for_inferior (void)
 enum infwait_states
 {
   infwait_normal_state,
-  infwait_thread_hop_state,
   infwait_step_watch_state,
   infwait_nonstep_watch_state
 };
@@ -3332,11 +3318,6 @@ handle_inferior_event (struct execution_control_state *ecs)
 
   switch (infwait_state)
     {
-    case infwait_thread_hop_state:
-      if (debug_infrun)
-        fprintf_unfiltered (gdb_stdlog, "infrun: infwait_thread_hop_state\n");
-      break;
-
     case infwait_normal_state:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: infwait_normal_state\n");
@@ -3907,166 +3888,6 @@ handle_signal_stop (struct execution_control_state *ecs)
       return;
     }
 
-  if (stepping_past_singlestep_breakpoint)
-    {
-      gdb_assert (singlestep_breakpoints_inserted_p);
-      gdb_assert (ptid_equal (singlestep_ptid, ecs->ptid));
-      gdb_assert (!ptid_equal (singlestep_ptid, saved_singlestep_ptid));
-
-      stepping_past_singlestep_breakpoint = 0;
-
-      /* We've either finished single-stepping past the single-step
-         breakpoint, or stopped for some other reason.  It would be nice if
-         we could tell, but we can't reliably.  */
-      if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
-	{
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog,
-				"infrun: stepping_past_"
-				"singlestep_breakpoint\n");
-	  /* Pull the single step breakpoints out of the target.  */
-	  if (!ptid_equal (ecs->ptid, inferior_ptid))
-	    context_switch (ecs->ptid);
-	  remove_single_step_breakpoints ();
-	  singlestep_breakpoints_inserted_p = 0;
-
-	  ecs->event_thread->control.trap_expected = 0;
-
-	  context_switch (saved_singlestep_ptid);
-	  if (deprecated_context_hook)
-	    deprecated_context_hook (pid_to_thread_id (saved_singlestep_ptid));
-
-	  resume (1, GDB_SIGNAL_0);
-	  prepare_to_wait (ecs);
-	  return;
-	}
-    }
-
-  /* See if a thread hit a thread-specific breakpoint that was meant for
-     another thread.  If so, then step that thread past the breakpoint,
-     and continue it.  */
-
-  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
-    {
-      int thread_hop_needed = 0;
-      struct address_space *aspace = 
-	get_regcache_aspace (get_thread_regcache (ecs->ptid));
-
-      /* Check if a regular breakpoint has been hit before checking
-         for a potential single step breakpoint.  Otherwise, GDB will
-         not see this breakpoint hit when stepping onto breakpoints.  */
-      if (regular_breakpoint_inserted_here_p (aspace, stop_pc))
-	{
-	  if (!breakpoint_thread_match (aspace, stop_pc, ecs->ptid))
-	    thread_hop_needed = 1;
-	}
-      else if (singlestep_breakpoints_inserted_p)
-	{
-	  /* We have not context switched yet, so this should be true
-	     no matter which thread hit the singlestep breakpoint.  */
-	  gdb_assert (ptid_equal (inferior_ptid, singlestep_ptid));
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog, "infrun: software single step "
-				"trap for %s\n",
-				target_pid_to_str (ecs->ptid));
-
-	  /* The call to in_thread_list is necessary because PTIDs sometimes
-	     change when we go from single-threaded to multi-threaded.  If
-	     the singlestep_ptid is still in the list, assume that it is
-	     really different from ecs->ptid.  */
-	  if (!ptid_equal (singlestep_ptid, ecs->ptid)
-	      && in_thread_list (singlestep_ptid))
-	    {
-	      /* If the PC of the thread we were trying to single-step
-		 has changed, discard this event (which we were going
-		 to ignore anyway), and pretend we saw that thread
-		 trap.  This prevents us continuously moving the
-		 single-step breakpoint forward, one instruction at a
-		 time.  If the PC has changed, then the thread we were
-		 trying to single-step has trapped or been signalled,
-		 but the event has not been reported to GDB yet.
-
-		 There might be some cases where this loses signal
-		 information, if a signal has arrived at exactly the
-		 same time that the PC changed, but this is the best
-		 we can do with the information available.  Perhaps we
-		 should arrange to report all events for all threads
-		 when they stop, or to re-poll the remote looking for
-		 this particular thread (i.e. temporarily enable
-		 schedlock).  */
-
-	     CORE_ADDR new_singlestep_pc
-	       = regcache_read_pc (get_thread_regcache (singlestep_ptid));
-
-	     if (new_singlestep_pc != singlestep_pc)
-	       {
-		 enum gdb_signal stop_signal;
-
-		 if (debug_infrun)
-		   fprintf_unfiltered (gdb_stdlog, "infrun: unexpected thread,"
-				       " but expected thread advanced also\n");
-
-		 /* The current context still belongs to
-		    singlestep_ptid.  Don't swap here, since that's
-		    the context we want to use.  Just fudge our
-		    state and continue.  */
-                 stop_signal = ecs->event_thread->suspend.stop_signal;
-                 ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
-                 ecs->ptid = singlestep_ptid;
-                 ecs->event_thread = find_thread_ptid (ecs->ptid);
-                 ecs->event_thread->suspend.stop_signal = stop_signal;
-                 stop_pc = new_singlestep_pc;
-               }
-             else
-	       {
-		 if (debug_infrun)
-		   fprintf_unfiltered (gdb_stdlog,
-				       "infrun: unexpected thread\n");
-
-		 thread_hop_needed = 1;
-		 stepping_past_singlestep_breakpoint = 1;
-		 saved_singlestep_ptid = singlestep_ptid;
-	       }
-	    }
-	}
-
-      if (thread_hop_needed)
-	{
-	  struct regcache *thread_regcache;
-	  int remove_status = 0;
-
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog, "infrun: thread_hop_needed\n");
-
-	  /* Switch context before touching inferior memory, the
-	     previous thread may have exited.  */
-	  if (!ptid_equal (inferior_ptid, ecs->ptid))
-	    context_switch (ecs->ptid);
-
-	  /* Saw a breakpoint, but it was hit by the wrong thread.
-	     Just continue.  */
-
-	  if (singlestep_breakpoints_inserted_p)
-	    {
-	      /* Pull the single step breakpoints out of the target.  */
-	      remove_single_step_breakpoints ();
-	      singlestep_breakpoints_inserted_p = 0;
-	    }
-
-	  if (!non_stop)
-	    {
-	      /* Only need to require the next event from this thread
-		 in all-stop mode.  */
-	      waiton_ptid = ecs->ptid;
-	      infwait_state = infwait_thread_hop_state;
-	    }
-
-	  ecs->event_thread->stepping_over_breakpoint = 1;
-	  keep_going (ecs);
-	  return;
-	}
-    }
-
   /* See if something interesting happened to the non-current thread.  If
      so, then switch to that thread.  */
   if (!ptid_equal (ecs->ptid, inferior_ptid))
@@ -4084,13 +3905,6 @@ handle_signal_stop (struct execution_control_state *ecs)
   frame = get_current_frame ();
   gdbarch = get_frame_arch (frame);
 
-  if (singlestep_breakpoints_inserted_p)
-    {
-      /* Pull the single step breakpoints out of the target.  */
-      remove_single_step_breakpoints ();
-      singlestep_breakpoints_inserted_p = 0;
-    }
-
   if (ecs->stepped_after_stopped_by_watchpoint)
     stopped_by_watchpoint = 0;
   else
@@ -4124,6 +3938,13 @@ handle_signal_stop (struct execution_control_state *ecs)
 	 disable all watchpoints and breakpoints.  */
       int hw_step = 1;
 
+      if (singlestep_breakpoints_inserted_p)
+	{
+	  /* Pull the single step breakpoints out of the target.  */
+	  remove_single_step_breakpoints ();
+	  singlestep_breakpoints_inserted_p = 0;
+	}
+
       if (!target_have_steppable_watchpoint)
 	{
 	  remove_breakpoints ();
@@ -4206,6 +4027,13 @@ handle_signal_stop (struct execution_control_state *ecs)
       if (ecs->event_thread->control.step_range_end == 0
 	  && step_through_delay)
 	{
+	  if (singlestep_breakpoints_inserted_p)
+	    {
+	      /* Pull the single step breakpoints out of the target.  */
+	      remove_single_step_breakpoints ();
+	      singlestep_breakpoints_inserted_p = 0;
+	    }
+
 	  /* The user issued a continue when stopped at a breakpoint.
 	     Set up for another trap and get out of here.  */
          ecs->event_thread->stepping_over_breakpoint = 1;
@@ -4301,6 +4129,13 @@ handle_signal_stop (struct execution_control_state *ecs)
 
       stopped_by_random_signal = 1;
 
+      if (singlestep_breakpoints_inserted_p)
+	{
+	  /* Pull the single step breakpoints out of the target.  */
+	  remove_single_step_breakpoints ();
+	  singlestep_breakpoints_inserted_p = 0;
+	}
+
       if (signal_print[ecs->event_thread->suspend.stop_signal])
 	{
 	  printed = 1;
@@ -4441,6 +4276,18 @@ process_event_stop_test (struct execution_control_state *ecs)
       step_over_address = 0;
     }
 
+  /* If the wrong thread hits a single-step breakpoint, we want bpstat
+     to return at least BPSTAT_WHAT_SINGLE.  If the right thread hit
+     one though, the step is done, so remove them now avoiding a
+     needless step-over.  */
+  if (singlestep_breakpoints_inserted_p
+      && ptid_equal (ecs->ptid, singlestep_ptid))
+    {
+      /* Pull the single step breakpoints out of the target.  */
+      remove_single_step_breakpoints ();
+      singlestep_breakpoints_inserted_p = 0;
+    }
+
   what = bpstat_what (ecs->event_thread->control.stop_bpstat);
 
   if (what.call_dummy)
@@ -4448,6 +4295,13 @@ process_event_stop_test (struct execution_control_state *ecs)
       stop_stack_dummy = what.call_dummy;
     }
 
+  /* No longer need these.  See above.  */
+  if (singlestep_breakpoints_inserted_p)
+    {
+      remove_single_step_breakpoints ();
+      singlestep_breakpoints_inserted_p = 0;
+    }
+
   /* If we hit an internal event that triggers symbol changes, the
      current frame will be invalidated within bpstat_what (e.g., if we
      hit an internal solib event).  Re-fetch it.  */
@@ -5257,6 +5111,9 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 				 ecs->event_thread);
       if (tp)
 	{
+	  struct frame_info *frame;
+	  struct gdbarch *gdbarch;
+
 	  /* However, if the current thread is blocked on some internal
 	     breakpoint, and we simply need to step over that breakpoint
 	     to get it going again, do that first.  */
@@ -5314,7 +5171,52 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 	  ecs->event_thread = tp;
 	  ecs->ptid = tp->ptid;
 	  context_switch (ecs->ptid);
-	  keep_going (ecs);
+
+	  stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
+	  frame = get_current_frame ();
+	  gdbarch = get_frame_arch (frame);
+
+	  /* If the PC of the thread we were trying to single-step has
+	     changed, then the thread we were trying to single-step
+	     has trapped or been signalled, but the event has not been
+	     reported to GDB yet.  Re-poll the remote looking for this
+	     particular thread (i.e. temporarily enable schedlock):
+
+	       - set a break at the current PC
+	       - resuming that particular thread, only (by setting
+		 trap expected)
+
+	     This prevents us continuously moving the single-step
+	     breakpoint forward, one instruction at a time, thus
+	     overstepping.  */
+
+	  if (gdbarch_software_single_step_p (gdbarch)
+	      && stop_pc != tp->prev_pc)
+	    {
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "infrun: expected thread advanced also\n");
+
+	      insert_single_step_breakpoint (get_frame_arch (frame),
+					     get_frame_address_space (frame),
+					     stop_pc);
+	      singlestep_breakpoints_inserted_p = 1;
+	      ecs->event_thread->control.trap_expected = 1;
+	      singlestep_ptid = inferior_ptid;
+	      singlestep_pc = stop_pc;
+
+	      resume (0, GDB_SIGNAL_0);
+	      prepare_to_wait (ecs);
+	    }
+	  else
+	    {
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "infrun: expected thread still "
+				    "hasn't advanced\n");
+	      keep_going (ecs);
+	    }
+
 	  return 1;
 	}
     }
-- 
1.7.11.7


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]