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

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

[Andrew Cagney]

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.  */
From cagney@gnu.org Mon May 10 04:18:00 2004
From: Andrew Cagney <cagney@gnu.org>
To: gdb-patches@sources.redhat.com
Subject: [commit] Clean targets of step_range_*
Date: Mon, 10 May 2004 04:18:00 -0000
Message-id: <409F0266.4010201@gnu.org>
X-SW-Source: 2004-05/msg00281.html
Content-length: 6607

Hello,

This patch eliminates references to the globals step_range_start and 
step_range_end found in remote.c and remote-vx.c (remote.c's code was 
disabled, who knows if remote-vx.c even works).  Just one small step 
towards eliminating them entirely.

committed,
Andrew
2004-05-09  Andrew Cagney  <cagney@redhat.com>

	* remote-vx.c (net_step): Delete step-range code.
	* remote.c (remote_resume, init_all_packet_configs)
	(set_remote_protocol_E_packet_cmd)
	(show_remote_protocol_E_packet_cmd)
	(remote_protocol_E, show_remote_cmd, _initialize_remote)
	(remote_protocol_e, set_remote_protocol_e_packet_cmd)
	(show_remote_protocol_e_packet_cmd): Ditto.

Index: remote-vx.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-vx.c,v
retrieving revision 1.30
diff -p -u -r1.30 remote-vx.c
--- remote-vx.c	21 Sep 2003 01:26:45 -0000	1.30
+++ remote-vx.c	10 May 2004 04:02:49 -0000
@@ -747,17 +747,8 @@ net_step (void)
   SOURCE_STEP source_step;
 
   source_step.taskId = PIDGET (inferior_ptid);
-
-  if (step_range_end)
-    {
-      source_step.startAddr = step_range_start;
-      source_step.endAddr = step_range_end;
-    }
-  else
-    {
-      source_step.startAddr = 0;
-      source_step.endAddr = 0;
-    }
+  source_step.startAddr = 0;
+  source_step.endAddr = 0;
 
   status = net_clnt_call (VX_SOURCE_STEP, xdr_SOURCE_STEP, &source_step,
 			  xdr_int, &step_status);
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.134
diff -p -u -r1.134 remote.c
--- remote.c	26 Apr 2004 09:02:41 -0000	1.134
+++ remote.c	10 May 2004 04:02:50 -0000
@@ -774,42 +774,6 @@ show_remote_protocol_qSymbol_packet_cmd 
   show_packet_config_cmd (&remote_protocol_qSymbol);
 }
 
-/* Should we try the 'e' (step over range) request? */
-static struct packet_config remote_protocol_e;
-
-static void
-set_remote_protocol_e_packet_cmd (char *args, int from_tty,
-				  struct cmd_list_element *c)
-{
-  update_packet_config (&remote_protocol_e);
-}
-
-static void
-show_remote_protocol_e_packet_cmd (char *args, int from_tty,
-				   struct cmd_list_element *c)
-{
-  show_packet_config_cmd (&remote_protocol_e);
-}
-
-
-/* Should we try the 'E' (step over range / w signal #) request? */
-static struct packet_config remote_protocol_E;
-
-static void
-set_remote_protocol_E_packet_cmd (char *args, int from_tty,
-				  struct cmd_list_element *c)
-{
-  update_packet_config (&remote_protocol_E);
-}
-
-static void
-show_remote_protocol_E_packet_cmd (char *args, int from_tty,
-				   struct cmd_list_element *c)
-{
-  show_packet_config_cmd (&remote_protocol_E);
-}
-
-
 /* Should we try the 'P' (set register) request?  */
 
 static struct packet_config remote_protocol_P;
@@ -2077,8 +2041,6 @@ static void
 init_all_packet_configs (void)
 {
   int i;
-  update_packet_config (&remote_protocol_e);
-  update_packet_config (&remote_protocol_E);
   update_packet_config (&remote_protocol_P);
   update_packet_config (&remote_protocol_qSymbol);
   update_packet_config (&remote_protocol_vcont);
@@ -2565,60 +2527,6 @@ remote_resume (ptid_t ptid, int step, en
   else
     set_thread (pid, 0);	/* run this thread */
 
-  /* The s/S/c/C packets do not return status.  So if the target does
-     not support the S or C packets, the debug agent returns an empty
-     string which is detected in remote_wait().  This protocol defect
-     is fixed in the e/E packets. */
-
-  if (step && step_range_end)
-    {
-      /* If the target does not support the 'E' packet, we try the 'S'
-	 packet.  Ideally we would fall back to the 'e' packet if that
-	 too is not supported.  But that would require another copy of
-	 the code to issue the 'e' packet (and fall back to 's' if not
-	 supported) in remote_wait().  */
-
-      if (siggnal != TARGET_SIGNAL_0)
-	{
-	  if (remote_protocol_E.support != PACKET_DISABLE)
-	    {
-	      p = buf;
-	      *p++ = 'E';
-	      *p++ = tohex (((int) siggnal >> 4) & 0xf);
-	      *p++ = tohex (((int) siggnal) & 0xf);
-	      *p++ = ',';
-	      p += hexnumstr (p, (ULONGEST) step_range_start);
-	      *p++ = ',';
-	      p += hexnumstr (p, (ULONGEST) step_range_end);
-	      *p++ = 0;
-
-	      putpkt (buf);
-	      getpkt (buf, (rs->remote_packet_size), 0);
-
-	      if (packet_ok (buf, &remote_protocol_E) == PACKET_OK)
-		return;
-	    }
-	}
-      else
-	{
-	  if (remote_protocol_e.support != PACKET_DISABLE)
-	    {
-	      p = buf;
-	      *p++ = 'e';
-	      p += hexnumstr (p, (ULONGEST) step_range_start);
-	      *p++ = ',';
-	      p += hexnumstr (p, (ULONGEST) step_range_end);
-	      *p++ = 0;
-
-	      putpkt (buf);
-	      getpkt (buf, (rs->remote_packet_size), 0);
-
-	      if (packet_ok (buf, &remote_protocol_e) == PACKET_OK)
-		return;
-	    }
-	}
-    }
-
   if (siggnal != TARGET_SIGNAL_0)
     {
       buf[0] = step ? 'S' : 'C';
@@ -5422,8 +5330,6 @@ show_remote_cmd (char *args, int from_tt
   /* FIXME: cagney/2002-06-15: This function should iterate over
      remote_show_cmdlist for a list of sub commands to show.  */
   show_remote_protocol_Z_packet_cmd (args, from_tty, NULL);
-  show_remote_protocol_e_packet_cmd (args, from_tty, NULL);
-  show_remote_protocol_E_packet_cmd (args, from_tty, NULL);
   show_remote_protocol_P_packet_cmd (args, from_tty, NULL);
   show_remote_protocol_qSymbol_packet_cmd (args, from_tty, NULL);
   show_remote_protocol_vcont_packet_cmd (args, from_tty, NULL);
@@ -5607,28 +5513,6 @@ in a memory packet.\n",
 			 show_remote_protocol_qSymbol_packet_cmd,
 			 &remote_set_cmdlist, &remote_show_cmdlist,
 			 0);
-
-  add_packet_config_cmd (&remote_protocol_e,
-			 "e", "step-over-range",
-			 set_remote_protocol_e_packet_cmd,
-			 show_remote_protocol_e_packet_cmd,
-			 &remote_set_cmdlist, &remote_show_cmdlist,
-			 0);
-  /* Disable by default.  The ``e'' packet has nasty interactions with
-     the threading code - it relies on global state.  */
-  remote_protocol_e.detect = AUTO_BOOLEAN_FALSE;
-  update_packet_config (&remote_protocol_e);
-
-  add_packet_config_cmd (&remote_protocol_E,
-			 "E", "step-over-range-w-signal",
-			 set_remote_protocol_E_packet_cmd,
-			 show_remote_protocol_E_packet_cmd,
-			 &remote_set_cmdlist, &remote_show_cmdlist,
-			 0);
-  /* Disable by default.  The ``e'' packet has nasty interactions with
-     the threading code - it relies on global state.  */
-  remote_protocol_E.detect = AUTO_BOOLEAN_FALSE;
-  update_packet_config (&remote_protocol_E);
 
   add_packet_config_cmd (&remote_protocol_P,
 			 "P", "set-register",
From drow@false.org Mon May 10 04:32:00 2004
From: Daniel Jacobowitz <drow@false.org>
To: Andrew Cagney <cagney@gnu.org>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [patch/rfc] Rewrite decr-pc logic, eliminate step_sp
Date: Mon, 10 May 2004 04:32:00 -0000
Message-id: <20040510043222.GA30284@nevyn.them.org>
References: <409EFE4A.8050807@gnu.org>
X-SW-Source: 2004-05/msg00282.html
Content-length: 1809

On Mon, May 10, 2004 at 12:00:10AM -0400, Andrew Cagney wrote:
> 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

> +  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);
> +	}

I'm pretty sure that won't work.  prev_pc is where we were stopped
before we decided to single step.  breakpoint_pc is where, if we have
hit a breakpoint, the breakpoint would be.  They won't be equal;
breakpoint_pc will be the following instruction, or the target of a
branch if *prev_pc was a taken branch.  The old code assumes we hit a
breakpoint if we stopped with SIGTRAP with singlestep_breakpoints_inserted_p
- any reason not to keep that behavior?

I think Alpha OSF/1 and Alpha NetBSD are the only current
software-single-step and decr-pc targets, which makes this case a
little hard to test - at least OSF/1 had dreadful test results already,
I'm not sure about NetBSD.  Might want to verify that it isn't
catastrophic, at least.

The rest of it looks right to me, though I had to stare at it for
the last twenty minutes or so.

-- 
Daniel Jacobowitz

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

[Andrew Cagney]

+  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);
+	}


I'm pretty sure that won't work.  prev_pc is where we were stopped
before we decided to single step.  breakpoint_pc is where, if we have
hit a breakpoint, the breakpoint would be.  They won't be equal;
breakpoint_pc will be the following instruction, or the target of a
branch if *prev_pc was a taken branch.  The old code assumes we hit a
breakpoint if we stopped with SIGTRAP with singlestep_breakpoints_inserted_p
- any reason not to keep that behavior?
It's plain wrong.  I'm pretty sure that, when doing the thread-hop, 
singlestep_breakpoints_inserted_p holds, but current_stepping() doesn't.

I think Alpha OSF/1 and Alpha NetBSD are the only current
software-single-step and decr-pc targets, which makes this case a
little hard to test - at least OSF/1 had dreadful test results already,
I'm not sure about NetBSD.  Might want to verify that it isn't
catastrophic, at least.
The rest of it looks right to me, though I had to stare at it for
the last twenty minutes or so.
I gave up staring at the old code, it made no sense.

Attached is a revision.

Andrew

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 19:10:10 -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.153
diff -p -u -r1.153 infrun.c
--- infrun.c	10 May 2004 18:36:06 -0000	1.153
+++ infrun.c	10 May 2004 19:10:10 -0000
@@ -953,7 +953,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;
@@ -1106,7 +1105,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;
@@ -1260,7 +1258,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.  */
@@ -1294,40 +1292,53 @@ 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 (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.  */
+	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.  */
+      if (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
@@ -2410,11 +2421,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
@@ -2835,10 +2841,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.  */
From thorpej@wasabisystems.com Mon May 10 19:33:00 2004
From: Jason Thorpe <thorpej@wasabisystems.com>
To: Jim Blandy <jimb@redhat.com>
Cc: Kevin Buettner <kevinb@redhat.com>, gdb-patches@sources.redhat.com
Subject: Re: RFA: handle missing fpregs
Date: Mon, 10 May 2004 19:33:00 -0000
Message-id: <D567851C-A2B8-11D8-94E3-000A957650EC@wasabisystems.com>
References: <vt21xlx19f8.fsf@zenia.home> <20040507131804.7c9325d6@saguaro> <vt2pt9gx81k.fsf@zenia.home> <20040507155610.43b806ff@saguaro> <vt2pt9cw1ji.fsf@zenia.home>
X-SW-Source: 2004-05/msg00298.html
Content-length: 1285

On May 10, 2004, at 12:00 PM, Jim Blandy wrote:

Actually, I just realized that the NetBSD stuff needs Jason Thorpe's
approval.  Since the change needs to go in all at once or not at all,
I've backed out whole change, pending Jason's review.
Sorry for the delay -- I was traveling when you sent the mail.

I've just looked over the patches.  These bits are problematic:

+   /* FIXME: jimb/2004-05-05: Some PPC variants don't have
+      floating-point registers.  For such variants,
+      tdep->ppc_fp0_regnum and tdep->ppc_fpscr_regnum will be -1.  I
+      don't think NetBSD runs on any of those chips, but we can at
+      least make sure that if someone tries it, they'll get a proper
+      notification.  */
NetBSD does, in fact, run on the IBM405 and other FPU-less PowerPC 
variants.  We have an FPU emulation module in the kernel that provides 
compatibility with PowerPC variants that have FPUs, but we can also 
build the system for soft-float only.

        -- Jason R. Thorpe <thorpej@wasabisystems.com>

Attachment:
PGP.sig
Description: This is a digitally signed message part

-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (Darwin)

iD8DBQFAn9jqOpVKkaBm8XkRAshyAJ9pPS+q1S33VCneqXrsVzrsPvzQFQCfZBkC
2ALsekX2WyxL1LZ5sdzogHI=
=/jLO
-----END PGP SIGNATURE-----
From drow@false.org Mon May 10 20:08:00 2004
From: Daniel Jacobowitz <drow@false.org>
To: "Maciej W. Rozycki" <macro@ds2.pg.gda.pl>
Cc: gdb-patches@sources.redhat.com
Subject: Re: 6.1: Quote macro names being defined by AC_DEFUN
Date: Mon, 10 May 2004 20:08:00 -0000
Message-id: <20040510200802.GA15632@nevyn.them.org>
References: <Pine.LNX.4.55.0404192244001.18710@jurand.ds.pg.gda.pl>
X-SW-Source: 2004-05/msg00299.html
Content-length: 620

On Mon, Apr 19, 2004 at 10:49:24PM +0200, Maciej W. Rozycki wrote:
> Hello,
> 
>  As of automake 1.8, aclocal requires all macro names being defined by
> AC_DEFUN to be quoted.  Here's an obvious fix.
> 
> 2004-04-19  Maciej W. Rozycki  <macro@ds2.pg.gda.pl>
> 
> 	* acinclude.m4: Quote macro names being defined by AC_DEFUN 
> 	througout.
> 
>  Please apply.

I've checked this in, with this changelog entry:

2004-05-10  Maciej W. Rozycki  <macro@ds2.pg.gda.pl>

        * acinclude.m4: Quote macro names being defined by AC_DEFUN
        throughout.
        * aclocal.m4: Regenerate.

Thanks.

-- 
Daniel Jacobowitz

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

[Andrew Cagney]

+  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);
+    }


I'm pretty sure that won't work.  prev_pc is where we were stopped
before we decided to single step.  breakpoint_pc is where, if we have
hit a breakpoint, the breakpoint would be.  They won't be equal;
breakpoint_pc will be the following instruction, or the target of a
branch if *prev_pc was a taken branch.  The old code assumes we hit a
breakpoint if we stopped with SIGTRAP with singlestep_breakpoints_inserted_p
- any reason not to keep that behavior?


It's plain wrong.  I'm pretty sure that, when doing the thread-hop, singlestep_breakpoints_inserted_p holds, but current_stepping() doesn't.

I think Alpha OSF/1 and Alpha NetBSD are the only current
software-single-step and decr-pc targets, which makes this case a
little hard to test - at least OSF/1 had dreadful test results already,
I'm not sure about NetBSD.  Might want to verify that it isn't
catastrophic, at least.
The rest of it looks right to me, though I had to stare at it for
the last twenty minutes or so.


I gave up staring at the old code, it made no sense.

Attached is a revision.
I've checked this in.  In addition to PPC (h/w single step) and i386 
(decr pc after break) I gave it a sniff test on alpha GNU/Linux (hacked 
to use s/w single step).

Andrew

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.