This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [patch/rfc] Rewrite decr-pc logic, eliminate step_sp
- From: Andrew Cagney <cagney at gnu dot org>
- To: Daniel Jacobowitz <drow at false dot org>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Mon, 10 May 2004 15:13:57 -0400
- Subject: Re: [patch/rfc] Rewrite decr-pc logic, eliminate step_sp
- References: <409EFE4A.8050807@gnu.org> <20040510043222.GA30284@nevyn.them.org>
+ 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. */