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]

[rfc] Fix infrun.c fall-out from the software single-step logic change


Hello,

this fixes one outstanding issue after the ppc software single-step patch.

adjust_pc_after_break was using the assumption that SOFTWARE_SINGLE_STEP_P
was true if and only if we're using software single-step.  With the new
logic this is no longer true; instead the arch can decide on a case-by-case
basis whether to use software or hardware single-stepping.

This patch re-works the adjust_pc_after_break logic to remove that 
assumption.  It also simplifies the logic a bit, due to the fact that
now the software single-step breakpoints also show up via
software_breakpoint_inserted_here_p.

I've also removed a couple of checks for SOFTWARE_SINGLE_STEP_P in
handle_inferior_event -- simply checking for singlestep_breakpoints_inserted_p
should be OK now.

Tested on powerpc64-linux and spu-elf.

Any comments?  I don't actually have a test case where the old code
shows a bug, but it is clearly wrong ...

Bye,
Ulrich


ChangeLog:

	* infrun.c (adjust_pc_after_break): Do not assume software single-step
	is always active if SOFTWARE_SINGLE_STEP_P is true.
	(handle_inferior_event): Do not check for SOFTWARE_SINGLE_STEP_P.

diff -urNp gdb-orig/gdb/infrun.c gdb-head/gdb/infrun.c
--- gdb-orig/gdb/infrun.c	2007-06-15 19:20:34.030743111 +0200
+++ gdb-head/gdb/infrun.c	2007-06-15 21:08:14.534043469 +0200
@@ -1183,53 +1183,34 @@ adjust_pc_after_break (struct execution_
   breakpoint_pc = read_pc_pid (ecs->ptid) - gdbarch_decr_pc_after_break
 					    (current_gdbarch);
 
-  if (SOFTWARE_SINGLE_STEP_P ())
-    {
-      /* When using software single-step, a SIGTRAP can only indicate
-         an inserted breakpoint.  This actually makes things
-         easier.  */
-      if (singlestep_breakpoints_inserted_p)
-	/* When software single stepping, the instruction at [prev_pc]
-	   is never a breakpoint, but the instruction following
-	   [prev_pc] (in program execution order) always is.  Assume
-	   that following instruction was reached and hence a software
-	   breakpoint was hit.  */
-	write_pc_pid (breakpoint_pc, ecs->ptid);
-      else if (software_breakpoint_inserted_here_p (breakpoint_pc))
-	/* The inferior was free running (i.e., no single-step
-	   breakpoints inserted) and it hit a software breakpoint.  */
+  /* Check whether there actually is a software breakpoint inserted
+     at that location.  */
+  if (software_breakpoint_inserted_here_p (breakpoint_pc))
+    {
+      /* When using hardware single-step, a SIGTRAP is reported for both
+	 a completed single-step and a software breakpoint.  Need to
+	 differentiate between the two, as the latter needs adjusting
+	 but the former does not.
+
+	 The SIGTRAP can be due to a completed hardware single-step only if 
+	  - we didn't insert software single-step breakpoints
+	  - the thread to be examined is still the current thread
+	  - this thread is currently being stepped
+
+	 If any of these events did not occur, we must have stopped
+	 due to hitting a the software breakpoint, and have to back
+	 up the breakpoint address.
+
+	 As a special case, we could have hardware single-stepped a
+	 a software breakpoint.  In this case (prev_pc == breakpoint_pc),
+	 we also need to back up to the breakpoint address.  */
+
+      if (singlestep_breakpoints_inserted_p
+	  || !ptid_equal (ecs->ptid, inferior_ptid)
+	  || !currently_stepping (ecs)
+	  || prev_pc == breakpoint_pc)
 	write_pc_pid (breakpoint_pc, ecs->ptid);
     }
-  else
-    {
-      /* When using hardware single-step, a SIGTRAP is reported for
-         both a completed single-step and a software breakpoint.  Need
-         to differentiate between the two as the latter needs
-         adjusting but the former does not.
-
-         When the thread to be examined does not match the current thread
-         context we can't use currently_stepping, so assume no
-         single-stepping in this case.  */
-      if (ptid_equal (ecs->ptid, inferior_ptid) && currently_stepping (ecs))
-	{
-	  if (prev_pc == breakpoint_pc
-	      && software_breakpoint_inserted_here_p (breakpoint_pc))
-	    /* Hardware single-stepped a software breakpoint (as
-	       occures when the inferior is resumed with PC pointing
-	       at not-yet-hit software breakpoint).  Since the
-	       breakpoint really is executed, the inferior needs to be
-	       backed up to the breakpoint address.  */
-	    write_pc_pid (breakpoint_pc, ecs->ptid);
-	}
-      else
-	{
-	  if (software_breakpoint_inserted_here_p (breakpoint_pc))
-	    /* The inferior was free running (i.e., no hardware
-	       single-step and no possibility of a false SIGTRAP) and
-	       hit a software breakpoint.  */
-	    write_pc_pid (breakpoint_pc, ecs->ptid);
-	}
-    }
 }
 
 /* Given an execution control state that has been freshly filled in
@@ -1372,7 +1353,7 @@ handle_inferior_event (struct execution_
 					   (LONGEST) ecs->ws.value.integer));
       gdb_flush (gdb_stdout);
       target_mourn_inferior ();
-      singlestep_breakpoints_inserted_p = 0;	/* SOFTWARE_SINGLE_STEP_P() */
+      singlestep_breakpoints_inserted_p = 0;
       stop_print_frame = 0;
       stop_stepping (ecs);
       return;
@@ -1392,7 +1373,7 @@ handle_inferior_event (struct execution_
       target_mourn_inferior ();
 
       print_stop_reason (SIGNAL_EXITED, stop_signal);
-      singlestep_breakpoints_inserted_p = 0;	/* SOFTWARE_SINGLE_STEP_P() */
+      singlestep_breakpoints_inserted_p = 0;
       stop_stepping (ecs);
       return;
 
@@ -1548,8 +1529,7 @@ handle_inferior_event (struct execution_
 
   if (stepping_past_singlestep_breakpoint)
     {
-      gdb_assert (SOFTWARE_SINGLE_STEP_P ()
-		  && singlestep_breakpoints_inserted_p);
+      gdb_assert (singlestep_breakpoints_inserted_p);
       gdb_assert (ptid_equal (singlestep_ptid, ecs->ptid));
       gdb_assert (!ptid_equal (singlestep_ptid, saved_singlestep_ptid));
 
@@ -1598,7 +1578,7 @@ handle_inferior_event (struct execution_
 	  if (!breakpoint_thread_match (stop_pc, ecs->ptid))
 	    thread_hop_needed = 1;
 	}
-      else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+      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.  */
@@ -1669,7 +1649,7 @@ handle_inferior_event (struct execution_
 	  /* Saw a breakpoint, but it was hit by the wrong thread.
 	     Just continue. */
 
-	  if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+	  if (singlestep_breakpoints_inserted_p)
 	    {
 	      /* Pull the single step breakpoints out of the target. */
 	      remove_single_step_breakpoints ();
@@ -1718,7 +1698,7 @@ handle_inferior_event (struct execution_
 	      return;
 	    }
 	}
-      else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+      else if (singlestep_breakpoints_inserted_p)
 	{
 	  sw_single_step_trap_p = 1;
 	  ecs->random_signal = 0;
@@ -1740,7 +1720,7 @@ handle_inferior_event (struct execution_
 	deprecated_context_hook (pid_to_thread_id (ecs->ptid));
     }
 
-  if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+  if (singlestep_breakpoints_inserted_p)
     {
       /* Pull the single step breakpoints out of the target. */
       remove_single_step_breakpoints ();
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


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