[RFA] Fix watchpoints when stepping over a breakpoint

Daniel Jacobowitz
2002-04-02T23:43:33Z

For some of the history of this problem, see PR gdb/81 and:
http://sourceware.cygnus.com/ml/gdb-patches/2000-q1/msg00665.html

How Peter's or Jim's patches worked for them, I'm not entirely sure; I
suspect there's been a change to shared library debugging since then, and/or
a severe deficiency in the testsuite somewhere. When I tried it I got stuck
on the bp_shlib_event breakpoint. We would hit it, remove, single-step
(trap expected), check where we were... and find ourselves at the
shared library breakpoint again because of DECR_PC_AFTER_BREAK!

Instead, I now collect only non-breakpoint events. This appears to work
fine. It causes no changes in the testsuite; a trivial followup patch to
update the formatting in annota2.exp fixes the test in question. Is this OK
to commit? Anyone see a problem with my method?

--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer

2002-04-02 Daniel Jacobowitz <drow@mvista.com>

Fix PR gdb/81
* breakpoint.c (bpstat_stop_status): Add ignore_breakpoints
argument. If ignore_breakpoints is nonzero, skip all
breakpoint-like events.
* breakpoint.h (bpstat_stop_status): Update prototype.
* infrun.c (handle_inferior_event): Update calls to
bpstat_stop_status. Call bpstat_stop_status even if
trap_expected.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.56
diff -u -p -r1.56 infrun.c
--- infrun.c 2002/03/18 02:26:31 1.56
+++ infrun.c 2002/04/02 23:17:48
@@ -1417,6 +1417,7 @@ handle_inferior_event (struct execution_
{
CORE_ADDR tmp;
int stepped_after_stopped_by_watchpoint;
+ int ignore_breakpoints;

/* Cache the last pid/waitstatus. */
target_last_wait_ptid = ecs->ptid;
@@ -1606,7 +1607,8 @@ handle_inferior_event (struct execution_
stop_bpstat = bpstat_stop_status (&stop_pc,
currently_stepping (ecs) &&
prev_pc !=
- stop_pc - DECR_PC_AFTER_BREAK);
+ stop_pc - DECR_PC_AFTER_BREAK,
+ 0);
ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
inferior_ptid = ecs->saved_inferior_ptid;
goto process_event_stop_test;
@@ -1665,7 +1667,8 @@ handle_inferior_event (struct execution_
stop_bpstat = bpstat_stop_status (&stop_pc,
currently_stepping (ecs) &&
prev_pc !=
- stop_pc - DECR_PC_AFTER_BREAK);
+ stop_pc - DECR_PC_AFTER_BREAK,
+ 0);
ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
goto process_event_stop_test;

@@ -1740,7 +1743,8 @@ handle_inferior_event (struct execution_
stop_bpstat = bpstat_stop_status (&stop_pc,
currently_stepping (ecs) &&
prev_pc !=
- stop_pc - DECR_PC_AFTER_BREAK);
+ stop_pc - DECR_PC_AFTER_BREAK,
+ 0);
ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
inferior_ptid = ecs->saved_inferior_ptid;
goto process_event_stop_test;
@@ -2097,44 +2101,45 @@ handle_inferior_event (struct execution_
return;
}

- /* Don't even think about breakpoints
- if just proceeded over a breakpoint.
+ /* See if there is a breakpoint at the current PC.
+ Older versions of GDB did not call bpstat_stop_status after
+ proceeding over a breakpoint, causing missed watchpoints when
+ proceeding over a breakpoint on an instruction which triggers
+ a watchpoint. */

- However, if we are trying to proceed over a breakpoint
- and end up in sigtramp, then through_sigtramp_breakpoint
- will be set and we should check whether we've hit the
- step breakpoint. */
if (stop_signal == TARGET_SIGNAL_TRAP && trap_expected
&& through_sigtramp_breakpoint == NULL)
- bpstat_clear (&stop_bpstat);
+ {
+ ignore_breakpoints = 1;
+ }
else
{
- /* See if there is a breakpoint at the current PC. */
-
- /* The second argument of bpstat_stop_status is meant to help
- distinguish between a breakpoint trap and a singlestep trap.
- This is only important on targets where DECR_PC_AFTER_BREAK
- is non-zero. The prev_pc test is meant to distinguish between
- singlestepping a trap instruction, and singlestepping thru a
- jump to the instruction following a trap instruction. */
-
- stop_bpstat = bpstat_stop_status
- (&stop_pc,
- /* Pass TRUE if our reason for stopping is something other
- than hitting a breakpoint. We do this by checking that
- 1) stepping is going on and 2) 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. */
- (currently_stepping (ecs)
- && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
- && !(step_range_end
- && INNER_THAN (read_sp (), (step_sp - 16))))
- );
- /* Following in case break condition called a
- function. */
+ ignore_breakpoints = 0;
+ /* Following in case break condition called a function. */
stop_print_frame = 1;
}
+
+ /* The second argument of bpstat_stop_status is meant to help
+ distinguish between a breakpoint trap and a singlestep trap.
+ This is only important on targets where DECR_PC_AFTER_BREAK
+ is non-zero. The prev_pc test is meant to distinguish between
+ singlestepping a trap instruction, and singlestepping thru a
+ jump to the instruction following a trap instruction. */
+
+ stop_bpstat = bpstat_stop_status
+ (&stop_pc,
+ /* Pass TRUE if our reason for stopping is something other
+ than hitting a breakpoint. We do this by checking that
+ 1) stepping is going on and 2) 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. */
+ (ignore_breakpoints
+ || (currently_stepping (ecs)
+ && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
+ && !(step_range_end
+ && INNER_THAN (read_sp (), (step_sp - 16))))),
+ ignore_breakpoints);

if (stop_signal == TARGET_SIGNAL_TRAP)
ecs->random_signal
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.68
diff -u -p -r1.68 breakpoint.c
--- breakpoint.c 2002/03/28 01:35:55 1.68
+++ breakpoint.c 2002/04/02 23:17:49
@@ -2345,11 +2345,14 @@ which its expression is valid.\n");
watchpoint at which we have stopped. (We may have stopped for
several reasons concurrently.)

+ If IGNORE_BREAKPOINTS, we return only non-breakpoint results
+ (e.g. any watchpoints we may have hit).
+
Each element of the chain has valid next, breakpoint_at,
commands, FIXME??? fields. */

bpstat
-bpstat_stop_status (CORE_ADDR *pc, int not_a_breakpoint)
+bpstat_stop_status (CORE_ADDR *pc, int not_a_breakpoint, int ignore_breakpoints)
{
register struct breakpoint *b, *temp;
CORE_ADDR bp_addr;
@@ -2390,6 +2393,8 @@ bpstat_stop_status (CORE_ADDR *pc, int n
&& b->type != bp_catch_catch
&& b->type != bp_catch_throw) /* a non-watchpoint bp */
{
+ if (ignore_breakpoints)
+ continue;
if (b->address != bp_addr) /* address doesn't match */
continue;
if (overlay_debugging /* unmapped overlay section */
Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.11
diff -u -p -r1.11 breakpoint.h
--- breakpoint.h 2002/02/06 18:31:07 1.11
+++ breakpoint.h 2002/04/02 23:17:49
@@ -322,7 +322,7 @@ extern void bpstat_clear (bpstat *);
is part of the bpstat is copied as well. */
extern bpstat bpstat_copy (bpstat);

-extern bpstat bpstat_stop_status (CORE_ADDR *, int);
+extern bpstat bpstat_stop_status (CORE_ADDR *, int, int);
\f
/* This bpstat_what stuff tells wait_for_inferior what to do with a
breakpoint (a challenging task). */