This is the mail archive of the gdb-patches@sources.redhat.com 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]

RFC: Handle thread hopping for the singlestep breakpoint


In June or so of last year, I proposed implying scheduler-locking == step
for targets that used software single step.  There's no innate reason for
that restriction; the problem was that existing code for software single
step didn't take thread issues into account.

I spent some time trying to use a normal breakpoint for software single
step.  It's a real tangle.  For instance, the breakpoint would have to be
inserted and removed separately from other breakpoints, since we step over
breakpoints with "all" breakpoints removed.  I still think this would be a
worthwhile cleanup, but it will need to wait until some other breakpoint
cleanups have come to pass - particularly, elimination of the
remove_breakpoints() and insert_breakpoints() hammers in favor of removing
and inserting only the necessary breakpoints.

In the mean time, this patch implements a thread ID for the singlestep
breakpoint, and thread hopping for it in parallel to the existing thread
hopping for normal breakpoints.  When we insert software single step
breakpoints, we save the PTID.  When we hit them, we check the thread; if it
is the wrong one, we remove breakpoints, insert a new software single step
breakpoint at the following instruction, step just that thread, reinsert the
breakpoint, and resume.

This patch leaves a big block of infrun.c mis-indented, but I wanted to
leave that for a followup patch, since this code is delicate enough to read
already.

Tested i386-linux (no change) and mipsel-linux (removes several failures
from schedlock.exp, of the form "Program received signal SIGTRAP").

I'll not check this in for at least a week.  I'd appreciate comments.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-02-01  Daniel Jacobowitz  <drow@mvista.com>

	* Makefile.in (infrun.o): Add $(gdb_assert_h).
	* infrun.c: Include "gdb_assert.h".
	(singlestep_ptid, saved_singlestep_ptid)
	(stepping_past_singlestep_breakpoint): New variables.
	(resume): Set singlestep_ptid.  Check for singlestep thread
	hop.
	(init_wait_for_inferior): Clear stepping_past_singlestep_breakpoint.
	(handle_inferior_event): Handle singlestep thread hop.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.484
diff -u -p -r1.484 Makefile.in
--- Makefile.in	1 Feb 2004 05:50:53 -0000	1.484
+++ Makefile.in	2 Feb 2004 02:01:07 -0000
@@ -1958,7 +1958,7 @@ infrun.o: infrun.c $(defs_h) $(gdb_strin
 	$(inferior_h) $(breakpoint_h) $(gdb_wait_h) $(gdbcore_h) $(gdbcmd_h) \
 	$(cli_script_h) $(target_h) $(gdbthread_h) $(annotate_h) \
 	$(symfile_h) $(top_h) $(inf_loop_h) $(regcache_h) $(value_h) \
-	$(observer_h) $(language_h)
+	$(observer_h) $(language_h) $(gdb_assert_h)
 inftarg.o: inftarg.c $(defs_h) $(frame_h) $(inferior_h) $(target_h) \
 	$(gdbcore_h) $(command_h) $(gdb_stat_h) $(gdb_wait_h) $(inflow_h)
 infttrace.o: infttrace.c $(defs_h) $(frame_h) $(inferior_h) $(target_h) \
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.134
diff -u -p -r1.134 infrun.c
--- infrun.c	1 Feb 2004 18:05:09 -0000	1.134
+++ infrun.c	2 Feb 2004 02:01:08 -0000
@@ -44,6 +44,7 @@
 #include "value.h"
 #include "observer.h"
 #include "language.h"
+#include "gdb_assert.h"
 
 /* Prototypes for local functions */
 
@@ -474,6 +475,14 @@ follow_exec (int pid, char *execd_pathna
    because we cannot remove the breakpoints in the inferior process
    until after the `wait' in `wait_for_inferior'.  */
 static int singlestep_breakpoints_inserted_p = 0;
+
+/* The thread we inserted single-step breakpoints for.  */
+static ptid_t singlestep_ptid;
+
+/* 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;
 
 
 /* Things to clean up if we QUIT out of resume ().  */
@@ -560,6 +569,7 @@ resume (int step, enum target_signal sig
       /* and do not pull these breakpoints until after a `wait' in
          `wait_for_inferior' */
       singlestep_breakpoints_inserted_p = 1;
+      singlestep_ptid = inferior_ptid;
     }
 
   /* Handle any optimized stores to the inferior NOW...  */
@@ -597,7 +607,8 @@ resume (int step, enum target_signal sig
       resume_ptid = RESUME_ALL;	/* Default */
 
       if ((step || singlestep_breakpoints_inserted_p) &&
-	  !breakpoints_inserted && breakpoint_here_p (read_pc ()))
+	  (stepping_past_singlestep_breakpoint
+	   || (!breakpoints_inserted && breakpoint_here_p (read_pc ()))))
 	{
 	  /* Stepping past a breakpoint without inserting breakpoints.
 	     Make sure only the current thread gets to step, so that
@@ -896,6 +907,8 @@ init_wait_for_inferior (void)
   number_of_threads_in_syscalls = 0;
 
   clear_proceed_status ();
+
+  stepping_past_singlestep_breakpoint = 0;
 }
 
 static void
@@ -1739,12 +1752,46 @@ handle_inferior_event (struct execution_
 
   stop_pc = read_pc_pid (ecs->ptid);
 
+  if (stepping_past_singlestep_breakpoint)
+    {
+      gdb_assert (SOFTWARE_SINGLE_STEP_P () && 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 (stop_signal == TARGET_SIGNAL_TRAP)
+        {
+	  /* Pull the single step breakpoints out of the target.  */
+	  SOFTWARE_SINGLE_STEP (0, 0);
+	  singlestep_breakpoints_inserted_p = 0;
+
+	  ecs->random_signal = 0;
+
+	  ecs->ptid = saved_singlestep_ptid;
+	  context_switch (ecs);
+	  if (context_hook)
+	    context_hook (pid_to_thread_id (ecs->ptid));
+
+	  resume (1, TARGET_SIGNAL_0);
+	  prepare_to_wait (ecs);
+	  return;
+	}
+    }
+
+  stepping_past_singlestep_breakpoint = 0;
+
   /* 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 (stop_signal == TARGET_SIGNAL_TRAP)
     {
+      int thread_hop_needed = 0;
+
       /* 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.  */
@@ -1752,12 +1799,38 @@ handle_inferior_event (struct execution_
 	{
 	  ecs->random_signal = 0;
 	  if (!breakpoint_thread_match (stop_pc, ecs->ptid))
+	    thread_hop_needed = 1;
+	}
+      else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+	{
+	  ecs->random_signal = 0;
+	  /* 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))
+	    {
+	      thread_hop_needed = 1;
+	      stepping_past_singlestep_breakpoint = 1;
+	      saved_singlestep_ptid = singlestep_ptid;
+	    }
+	}
+
+      if (thread_hop_needed)
 	    {
 	      int remove_status;
 
 	      /* Saw a breakpoint, but it was hit by the wrong thread.
 	         Just continue. */
 
+	      if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+		{
+		  /* Pull the single step breakpoints out of the target. */
+		  SOFTWARE_SINGLE_STEP (0, 0);
+		  singlestep_breakpoints_inserted_p = 0;
+		}
+
 	      remove_status = remove_breakpoints ();
 	      /* Did we fail to remove breakpoints?  If so, try
 	         to set the PC past the bp.  (There's at least
@@ -1799,7 +1872,6 @@ handle_inferior_event (struct execution_
 		  registers_changed ();
 		  return;
 		}
-	    }
 	}
       else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
         {


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