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]

[patch/rfc] Rewrite decr-pc logic, eliminate step_sp


Hello,

The attached patch rewrites the logic (er, heuristic) used to decide when to apply decr_pc_after_break.

The heuristic even included:
step_range_end && INNER_THAN (read_sp (), (step_sp - 16))
yes, the 16 is for real! From memory it has something to do with SPARC signal trampolines (I see the comment has been lost).


The new logic, while based on the old code, isn't identical. I've tested it in i386 without regressions (which doesn't cover the s/w single step case).

comments?
Andrew
Index: ChangeLog
2004-05-09  Andrew Cagney  <cagney@redhat.com>

	* infrun.c (adjust_pc_after_break): Rewrite decr logic,
	eliminate reference to step_sp.
	(struct execution_control_state, init_execution_control_state)
	(handle_inferior_event, keep_going): Delete update_step_sp and
	step_sp.
	* infcmd.c (step_sp): Note that variable is unused.

Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.109
diff -p -u -r1.109 infcmd.c
--- infcmd.c	8 May 2004 23:02:10 -0000	1.109
+++ infcmd.c	10 May 2004 03:45:59 -0000
@@ -187,7 +187,8 @@ CORE_ADDR step_range_end;	/* Exclusive *
 struct frame_id step_frame_id;
 
 /* Our notion of the current stack pointer.  */
-
+/* NOTE: cagney/2004-05-09: This variable is not used and should be
+   garbage collected.  */
 CORE_ADDR step_sp;
 
 enum step_over_calls_kind step_over_calls;
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.152
diff -p -u -r1.152 infrun.c
--- infrun.c	8 May 2004 22:26:09 -0000	1.152
+++ infrun.c	10 May 2004 03:46:06 -0000
@@ -967,7 +967,6 @@ struct execution_control_state
   int handling_longjmp;		/* FIXME */
   ptid_t ptid;
   ptid_t saved_inferior_ptid;
-  int update_step_sp;
   int stepping_through_solib_after_catch;
   bpstat stepping_through_solib_catchpoints;
   int enable_hw_watchpoints_after_wait;
@@ -1123,7 +1122,6 @@ init_execution_control_state (struct exe
   ecs->random_signal = 0;
   ecs->remove_breakpoints_on_following_step = 0;
   ecs->handling_longjmp = 0;	/* FIXME */
-  ecs->update_step_sp = 0;
   ecs->stepping_through_solib_after_catch = 0;
   ecs->stepping_through_solib_catchpoints = NULL;
   ecs->enable_hw_watchpoints_after_wait = 0;
@@ -1277,7 +1275,7 @@ handle_step_into_function (struct execut
 static void
 adjust_pc_after_break (struct execution_control_state *ecs)
 {
-  CORE_ADDR stop_pc;
+  CORE_ADDR breakpoint_pc;
 
   /* If this target does not decrement the PC after breakpoints, then
      we have nothing to do.  */
@@ -1311,40 +1309,37 @@ adjust_pc_after_break (struct execution_
   if (ecs->ws.value.sig != TARGET_SIGNAL_TRAP)
     return;
 
-  /* Find the location where (if we've hit a breakpoint) the breakpoint would
-     be.  */
-  stop_pc = read_pc_pid (ecs->ptid) - DECR_PC_AFTER_BREAK;
-
-  /* If we're software-single-stepping, then assume this is a breakpoint.
-     NOTE drow/2004-01-17: This doesn't check that the PC matches, or that
-     we're even in the right thread.  The software-single-step code needs
-     some modernization.
-
-     If we're not software-single-stepping, then we first check that there
-     is an enabled software breakpoint at this address.  If there is, and
-     we weren't using hardware-single-step, then we've hit the breakpoint.
-
-     If we were using hardware-single-step, we check prev_pc; if we just
-     stepped over an inserted software breakpoint, then we should decrement
-     the PC and eventually report hitting the breakpoint.  The prev_pc check
-     prevents us from decrementing the PC if we just stepped over a jump
-     instruction and landed on the instruction after a breakpoint.
-
-     The last bit checks that we didn't hit a breakpoint in a signal handler
-     without an intervening stop in sigtramp, which is detected by a new
-     stack pointer value below any usual function calling stack adjustments.
-
-     NOTE drow/2004-01-17: I'm not sure that this is necessary.  The check
-     predates checking for software single step at the same time.  Also,
-     if we've moved into a signal handler we should have seen the
-     signal.  */
-
-  if ((SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
-      || (software_breakpoint_inserted_here_p (stop_pc)
-	  && !(currently_stepping (ecs)
-	       && prev_pc != stop_pc
-	       && !(step_range_end && INNER_THAN (read_sp (), (step_sp - 16))))))
-    write_pc_pid (stop_pc, ecs->ptid);
+  /* Find the location where (if we've hit a breakpoint) the
+     breakpoint would be.  */
+  breakpoint_pc = read_pc_pid (ecs->ptid) - DECR_PC_AFTER_BREAK;
+
+  if (currently_stepping (ecs))
+    {
+      if (SOFTWARE_SINGLE_STEP_P ())
+	{
+	  if (singlestep_breakpoints_inserted_p
+	      && prev_pc == breakpoint_pc)
+	    /* If we're software-single-stepping, assume we hit one of
+	       the inserted software breakpoints.  */
+	    write_pc_pid (breakpoint_pc, ecs->ptid);
+	}
+      else 
+	{
+	  if (prev_pc == breakpoint_pc
+	      && software_breakpoint_inserted_here_p (breakpoint_pc))
+	    /* Hardware single-stepped a breakpoint, back up to the
+	       breakpoint address.  */
+	    write_pc_pid (breakpoint_pc, ecs->ptid);
+	}
+    }
+  else
+    {
+      if (software_breakpoint_inserted_here_p (breakpoint_pc))
+	/* If we're not single-stepping, and there is an enabled
+	   software breakpoint at this address, then we've hit that
+	   breakpoint.  */
+	write_pc_pid (breakpoint_pc, ecs->ptid);
+    }
 }
 
 /* Given an execution control state that has been freshly filled in
@@ -2472,11 +2467,6 @@ process_event_stop_test:
       return;
     }
 
-  /* We can't update step_sp every time through the loop, because
-     reading the stack pointer would slow down stepping too much.
-     But we can update it every time we leave the step range.  */
-  ecs->update_step_sp = 1;
-
   /* Did we just step into a singal trampoline (either by stepping out
      of a handler, or by taking a signal)?  */
   if (get_frame_type (get_current_frame ()) == SIGTRAMP_FRAME
@@ -2902,10 +2892,6 @@ keep_going (struct execution_control_sta
 {
   /* Save the pc before execution, to compare with pc after stop.  */
   prev_pc = read_pc ();		/* Might have been DECR_AFTER_BREAK */
-
-  if (ecs->update_step_sp)
-    step_sp = read_sp ();
-  ecs->update_step_sp = 0;
 
   /* If we did not do break;, it means we should keep running the
      inferior and not return to debugger.  */

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