This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc, rfa/doc] Multi-threaded watchpoint improvements
- From: Daniel Jacobowitz <drow at false dot org>
- To: gdb-patches at sourceware dot org
- Date: Sun, 30 Sep 2007 20:20:15 -0400
- Subject: Re: [rfc, rfa/doc] Multi-threaded watchpoint improvements
- References: <20070916183949.GA23966@caradoc.them.org>
On Sun, Sep 16, 2007 at 02:39:49PM -0400, Daniel Jacobowitz wrote:
> I have (finally, sorry for the delay) finished revamping the
> multi-threaded watchpoint patches Jeff submitted ages ago and Luis
> resubmitted recently. This message contains all the changes except
> for various native GNU/Linux target files. It includes manual and
> gdbint changes describing what I've figured out to date.
Here's the version I've checked in. There was a merge conflict in
breakpoint.c, at the end of bpstat_stop_status. I think the code I'm
removing is wrong - b->type refers back to the breakpoint b inside the
loop, not necessarily the one in the bpstat.
--
Daniel Jacobowitz
CodeSourcery
2007-09-30 Daniel Jacobowitz <dan@codesourcery.com>
Jeff Johnston <jjohnstn@redhat.com>
* breakpoint.c (watchpoints_triggered): New.
(bpstat_stop_status): Remove STOPPED_BY_WATCHPOINT argument.
Check watchpoint_triggered instead. Combine handling for software
and hardware watchpoints. Do not use target_stopped_data_address
here. Always check a watchpoint if its scope breakpoint triggers.
Do not stop for thread or overlay events. Improve check for
triggered watchpoints without a value change.
(watch_command_1): Insert the scope breakpoint first. Link the
scope breakpoint to the watchpoint.
* breakpoint.h (enum watchpoint_triggered): New.
(struct breakpoint): Add watchpoint_triggered.
(bpstat_stop_status): Update prototype.
(watchpoints_triggered): Declare.
* infrun.c (enum infwait_status): Add infwait_step_watch_state.
(stepped_after_stopped_by_watchpoint): Delete.
(handle_inferior_event): Make stepped_after_stopped_by_watchpoint
local. Handle infwait_step_watch_state. Update calls to
bpstat_stop_status. Use watchpoints_triggered to check
watchpoints.
* remote.c (stepped_after_stopped_by_watchpoint): Remove extern.
(remote_stopped_data_address): Do not check it.
2007-09-30 Daniel Jacobowitz <dan@codesourcery.com>
* gdb.texinfo (Setting Watchpoints): Adjust warning text about
multi-threaded watchpoints.
* gdbint.texinfo (Watchpoints): Describe how watchpoints are
checked. Describe sticky notification. Expand description
of steppable and continuable watchpoints.
(Watchpoints and Threads): New subsection.
2007-09-30 Daniel Jacobowitz <dan@codesourcery.com>
* gdb.threads/watchthreads.c (thread_function): Sleep between
iterations.
* gdb.threads/watchthreads.exp: Allow two watchpoints to trigger
at once for S/390. Generate matching fails and passes.
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.270
diff -u -p -r1.270 breakpoint.c
--- breakpoint.c 26 Sep 2007 18:44:55 -0000 1.270
+++ breakpoint.c 1 Oct 2007 00:11:08 -0000
@@ -2537,6 +2537,83 @@ bpstat_alloc (struct bp_location *bl, bp
return bs;
}
+/* The target has stopped with waitstatus WS. Check if any hardware
+ watchpoints have triggered, according to the target. */
+
+int
+watchpoints_triggered (struct target_waitstatus *ws)
+{
+ int stopped_by_watchpoint = STOPPED_BY_WATCHPOINT (*ws);
+ CORE_ADDR addr;
+ struct breakpoint *b;
+
+ if (!stopped_by_watchpoint)
+ {
+ /* We were not stopped by a watchpoint. Mark all watchpoints
+ as not triggered. */
+ ALL_BREAKPOINTS (b)
+ if (b->type == bp_hardware_watchpoint
+ || b->type == bp_read_watchpoint
+ || b->type == bp_access_watchpoint)
+ b->watchpoint_triggered = watch_triggered_no;
+
+ return 0;
+ }
+
+ if (!target_stopped_data_address (¤t_target, &addr))
+ {
+ /* We were stopped by a watchpoint, but we don't know where.
+ Mark all watchpoints as unknown. */
+ ALL_BREAKPOINTS (b)
+ if (b->type == bp_hardware_watchpoint
+ || b->type == bp_read_watchpoint
+ || b->type == bp_access_watchpoint)
+ b->watchpoint_triggered = watch_triggered_unknown;
+
+ return stopped_by_watchpoint;
+ }
+
+ /* The target could report the data address. Mark watchpoints
+ affected by this data address as triggered, and all others as not
+ triggered. */
+
+ ALL_BREAKPOINTS (b)
+ if (b->type == bp_hardware_watchpoint
+ || b->type == bp_read_watchpoint
+ || b->type == bp_access_watchpoint)
+ {
+ struct value *v;
+
+ b->watchpoint_triggered = watch_triggered_no;
+ for (v = b->val_chain; v; v = value_next (v))
+ {
+ if (VALUE_LVAL (v) == lval_memory && ! value_lazy (v))
+ {
+ struct type *vtype = check_typedef (value_type (v));
+
+ if (v == b->val_chain
+ || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
+ && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
+ {
+ CORE_ADDR vaddr;
+
+ vaddr = VALUE_ADDRESS (v) + value_offset (v);
+ /* Exact match not required. Within range is
+ sufficient. */
+ if (addr >= vaddr
+ && addr < vaddr + TYPE_LENGTH (value_type (v)))
+ {
+ b->watchpoint_triggered = watch_triggered_yes;
+ break;
+ }
+ }
+ }
+ }
+ }
+
+ return 1;
+}
+
/* Possible return values for watchpoint_check (this can't be an enum
because of check_errors). */
/* The watchpoint has been deleted. */
@@ -2655,11 +2732,9 @@ which its expression is valid.\n");
}
/* Get a bpstat associated with having just stopped at address
- BP_ADDR in thread PTID. STOPPED_BY_WATCHPOINT is 1 if the
- target thinks we stopped due to a hardware watchpoint, 0 if we
- know we did not trigger a hardware watchpoint, and -1 if we do not know. */
+ BP_ADDR in thread PTID.
-/* Determine whether we stopped at a breakpoint, etc, or whether we
+ Determine whether we stopped at a breakpoint, etc, or whether we
don't understand this stop. Result is a chain of bpstat's such that:
if we don't understand the stop, the result is a null pointer.
@@ -2674,7 +2749,7 @@ which its expression is valid.\n");
commands, FIXME??? fields. */
bpstat
-bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
+bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
{
struct breakpoint *b = NULL;
struct bp_location *bl;
@@ -2712,16 +2787,17 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
continue;
}
- /* Continuable hardware watchpoints are treated as non-existent if the
- reason we stopped wasn't a hardware watchpoint (we didn't stop on
- some data address). Otherwise gdb won't stop on a break instruction
- in the code (not from a breakpoint) when a hardware watchpoint has
- been defined. */
+ /* Continuable hardware watchpoints are treated as non-existent if the
+ reason we stopped wasn't a hardware watchpoint (we didn't stop on
+ some data address). Otherwise gdb won't stop on a break instruction
+ in the code (not from a breakpoint) when a hardware watchpoint has
+ been defined. Also skip watchpoints which we know did not trigger
+ (did not match the data address). */
if ((b->type == bp_hardware_watchpoint
|| b->type == bp_read_watchpoint
|| b->type == bp_access_watchpoint)
- && !stopped_by_watchpoint)
+ && b->watchpoint_triggered == watch_triggered_no)
continue;
if (b->type == bp_hardware_breakpoint)
@@ -2787,82 +2863,33 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
bs->stop = 1;
bs->print = 1;
- if (b->type == bp_watchpoint ||
- b->type == bp_hardware_watchpoint)
- {
- char *message = xstrprintf ("Error evaluating expression for watchpoint %d\n",
- b->number);
- struct cleanup *cleanups = make_cleanup (xfree, message);
- int e = catch_errors (watchpoint_check, bs, message,
- RETURN_MASK_ALL);
- do_cleanups (cleanups);
- switch (e)
- {
- case WP_DELETED:
- /* We've already printed what needs to be printed. */
- /* Actually this is superfluous, because by the time we
- call print_it_typical() the wp will be already deleted,
- and the function will return immediately. */
- bs->print_it = print_it_done;
- /* Stop. */
- break;
- case WP_VALUE_CHANGED:
- /* Stop. */
- ++(b->hit_count);
- break;
- case WP_VALUE_NOT_CHANGED:
- /* Don't stop. */
- bs->print_it = print_it_noop;
- bs->stop = 0;
- continue;
- default:
- /* Can't happen. */
- /* FALLTHROUGH */
- case 0:
- /* Error from catch_errors. */
- printf_filtered (_("Watchpoint %d deleted.\n"), b->number);
- if (b->related_breakpoint)
- b->related_breakpoint->disposition = disp_del_at_next_stop;
- b->disposition = disp_del_at_next_stop;
- /* We've already printed what needs to be printed. */
- bs->print_it = print_it_done;
-
- /* Stop. */
- break;
- }
- }
- else if (b->type == bp_read_watchpoint ||
- b->type == bp_access_watchpoint)
+ if (b->type == bp_watchpoint
+ || b->type == bp_read_watchpoint
+ || b->type == bp_access_watchpoint
+ || b->type == bp_hardware_watchpoint)
{
CORE_ADDR addr;
struct value *v;
- int found = 0;
+ int must_check_value = 0;
- if (!target_stopped_data_address (¤t_target, &addr))
- continue;
- for (v = b->val_chain; v; v = value_next (v))
- {
- if (VALUE_LVAL (v) == lval_memory
- && ! value_lazy (v))
- {
- struct type *vtype = check_typedef (value_type (v));
-
- if (v == b->val_chain
- || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
- && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
- {
- CORE_ADDR vaddr;
+ if (b->type == bp_watchpoint)
+ /* For a software watchpoint, we must always check the
+ watched value. */
+ must_check_value = 1;
+ else if (b->watchpoint_triggered == watch_triggered_yes)
+ /* We have a hardware watchpoint (read, write, or access)
+ and the target earlier reported an address watched by
+ this watchpoint. */
+ must_check_value = 1;
+ else if (b->watchpoint_triggered == watch_triggered_unknown
+ && b->type == bp_hardware_watchpoint)
+ /* We were stopped by a hardware watchpoint, but the target could
+ not report the data address. We must check the watchpoint's
+ value. Access and read watchpoints are out of luck; without
+ a data address, we can't figure it out. */
+ must_check_value = 1;
- vaddr = VALUE_ADDRESS (v) + value_offset (v);
- /* Exact match not required. Within range is
- sufficient. */
- if (addr >= vaddr &&
- addr < vaddr + TYPE_LENGTH (value_type (v)))
- found = 1;
- }
- }
- }
- if (found)
+ if (must_check_value)
{
char *message = xstrprintf ("Error evaluating expression for watchpoint %d\n",
b->number);
@@ -2890,6 +2917,15 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
++(b->hit_count);
break;
case WP_VALUE_NOT_CHANGED:
+ if (b->type == bp_hardware_watchpoint
+ || b->type == bp_watchpoint)
+ {
+ /* Don't stop: write watchpoints shouldn't fire if
+ the value hasn't changed. */
+ bs->print_it = print_it_noop;
+ bs->stop = 0;
+ continue;
+ }
/* Stop. */
++(b->hit_count);
break;
@@ -2906,12 +2942,12 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
break;
}
}
- else /* found == 0 */
+ else /* must_check_value == 0 */
{
- /* This is a case where some watchpoint(s) triggered,
- but not at the address of this watchpoint (FOUND
- was left zero). So don't print anything for this
- watchpoint. */
+ /* This is a case where some watchpoint(s) triggered, but
+ not at the address of this watchpoint, or else no
+ watchpoint triggered after all. So don't print
+ anything for this watchpoint. */
bs->print_it = print_it_noop;
bs->stop = 0;
continue;
@@ -2933,6 +2969,13 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
{
int value_is_zero = 0;
+ /* If this is a scope breakpoint, mark the associated
+ watchpoint as triggered so that we will handle the
+ out-of-scope event. We'll get to the watchpoint next
+ iteration. */
+ if (b->type == bp_watchpoint_scope)
+ b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
+
if (bl->cond)
{
/* Need to select the frame, with all that implies
@@ -2963,6 +3006,9 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
annotate_ignore_count_change ();
bs->stop = 0;
}
+ else if (b->type == bp_thread_event || b->type == bp_overlay_event)
+ /* We do not stop for these. */
+ bs->stop = 0;
else
{
/* We will stop here */
@@ -2989,17 +3035,27 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
bs->next = NULL; /* Terminate the chain */
bs = root_bs->next; /* Re-grab the head of the chain */
- /* The value of a hardware watchpoint hasn't changed, but the
- intermediate memory locations we are watching may have. */
- if (bs && !bs->stop &&
- (b->type == bp_hardware_watchpoint ||
- b->type == bp_read_watchpoint ||
- b->type == bp_access_watchpoint))
- {
- remove_breakpoints ();
- insert_breakpoints ();
- }
- return bs;
+ /* If we aren't stopping, the value of some hardware watchpoint may
+ not have changed, but the intermediate memory locations we are
+ watching may have. Don't bother if we're stopping; this will get
+ done later. */
+ for (bs = root_bs->next; bs != NULL; bs = bs->next)
+ if (bs->stop)
+ break;
+
+ if (bs == NULL)
+ for (bs = root_bs->next; bs != NULL; bs = bs->next)
+ if (!bs->stop
+ && (bs->breakpoint_at->owner->type == bp_hardware_watchpoint
+ || bs->breakpoint_at->owner->type == bp_read_watchpoint
+ || bs->breakpoint_at->owner->type == bp_access_watchpoint))
+ {
+ remove_breakpoints ();
+ insert_breakpoints ();
+ break;
+ }
+
+ return root_bs->next;
}
/* Tell what to do about this bpstat. */
@@ -5965,7 +6021,7 @@ stopat_command (char *arg, int from_tty)
static void
watch_command_1 (char *arg, int accessflag, int from_tty)
{
- struct breakpoint *b;
+ struct breakpoint *b, *scope_breakpoint = NULL;
struct symtab_and_line sal;
struct expression *exp;
struct block *exp_valid_block;
@@ -6043,6 +6099,37 @@ watch_command_1 (char *arg, int accessfl
if (!mem_cnt || target_resources_ok <= 0)
bp_type = bp_watchpoint;
+ frame = block_innermost_frame (exp_valid_block);
+ if (frame)
+ prev_frame = get_prev_frame (frame);
+ else
+ prev_frame = NULL;
+
+ /* If the expression is "local", then set up a "watchpoint scope"
+ breakpoint at the point where we've left the scope of the watchpoint
+ expression. Create the scope breakpoint before the watchpoint, so
+ that we will encounter it first in bpstat_stop_status. */
+ if (innermost_block && prev_frame)
+ {
+ scope_breakpoint = create_internal_breakpoint (get_frame_pc (prev_frame),
+ bp_watchpoint_scope);
+
+ scope_breakpoint->enable_state = bp_enabled;
+
+ /* Automatically delete the breakpoint when it hits. */
+ scope_breakpoint->disposition = disp_del;
+
+ /* Only break in the proper frame (help with recursion). */
+ scope_breakpoint->frame_id = get_frame_id (prev_frame);
+
+ /* Set the address at which we will stop. */
+ scope_breakpoint->loc->requested_address
+ = get_frame_pc (prev_frame);
+ scope_breakpoint->loc->address
+ = adjust_breakpoint_address (scope_breakpoint->loc->requested_address,
+ scope_breakpoint->type);
+ }
+
/* Now set up the breakpoint. */
b = set_raw_breakpoint (sal, bp_type);
set_breakpoint_count (breakpoint_count + 1);
@@ -6058,48 +6145,19 @@ watch_command_1 (char *arg, int accessfl
else
b->cond_string = 0;
- frame = block_innermost_frame (exp_valid_block);
if (frame)
- {
- prev_frame = get_prev_frame (frame);
- b->watchpoint_frame = get_frame_id (frame);
- }
+ b->watchpoint_frame = get_frame_id (frame);
else
- {
- memset (&b->watchpoint_frame, 0, sizeof (b->watchpoint_frame));
- }
+ memset (&b->watchpoint_frame, 0, sizeof (b->watchpoint_frame));
- /* If the expression is "local", then set up a "watchpoint scope"
- breakpoint at the point where we've left the scope of the watchpoint
- expression. */
- if (innermost_block)
+ if (scope_breakpoint != NULL)
{
- if (prev_frame)
- {
- struct breakpoint *scope_breakpoint;
- scope_breakpoint = create_internal_breakpoint (get_frame_pc (prev_frame),
- bp_watchpoint_scope);
-
- scope_breakpoint->enable_state = bp_enabled;
-
- /* Automatically delete the breakpoint when it hits. */
- scope_breakpoint->disposition = disp_del;
-
- /* Only break in the proper frame (help with recursion). */
- scope_breakpoint->frame_id = get_frame_id (prev_frame);
-
- /* Set the address at which we will stop. */
- scope_breakpoint->loc->requested_address
- = get_frame_pc (prev_frame);
- scope_breakpoint->loc->address
- = adjust_breakpoint_address (scope_breakpoint->loc->requested_address,
- scope_breakpoint->type);
-
- /* The scope breakpoint is related to the watchpoint. We
- will need to act on them together. */
- b->related_breakpoint = scope_breakpoint;
- }
+ /* The scope breakpoint is related to the watchpoint. We will
+ need to act on them together. */
+ b->related_breakpoint = scope_breakpoint;
+ scope_breakpoint->related_breakpoint = b;
}
+
value_free_to_mark (mark);
mention (b);
}
Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.51
diff -u -p -r1.51 breakpoint.h
--- breakpoint.h 23 Sep 2007 07:56:22 -0000 1.51
+++ breakpoint.h 1 Oct 2007 00:11:08 -0000
@@ -318,6 +318,19 @@ struct breakpoint_ops
void (*print_mention) (struct breakpoint *);
};
+enum watchpoint_triggered
+{
+ /* This watchpoint definitely did not trigger. */
+ watch_triggered_no = 0,
+
+ /* Some hardware watchpoint triggered, and it might have been this
+ one, but we do not know which it was. */
+ watch_triggered_unknown,
+
+ /* This hardware watchpoint definitely did trigger. */
+ watch_triggered_yes
+};
+
/* Note that the ->silent field is not currently used by any commands
(though the code is in there if it was to be, and set_raw_breakpoint
does set it to 0). I implemented it because I thought it would be
@@ -395,6 +408,10 @@ struct breakpoint
should be evaluated on the outermost frame. */
struct frame_id watchpoint_frame;
+ /* For hardware watchpoints, the triggered status according to the
+ hardware. */
+ enum watchpoint_triggered watchpoint_triggered;
+
/* Thread number for thread-specific breakpoint, or -1 if don't care */
int thread;
@@ -459,8 +476,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 pc, ptid_t ptid,
- int stopped_by_watchpoint);
+extern bpstat bpstat_stop_status (CORE_ADDR pc, ptid_t ptid);
/* This bpstat_what stuff tells wait_for_inferior what to do with a
breakpoint (a challenging task). */
@@ -853,4 +869,8 @@ extern void remove_single_step_breakpoin
extern void *deprecated_insert_raw_breakpoint (CORE_ADDR);
extern int deprecated_remove_raw_breakpoint (void *);
+/* Check if any hardware watchpoints have triggered, according to the
+ target. */
+int watchpoints_triggered (struct target_waitstatus *);
+
#endif /* !defined (BREAKPOINT_H) */
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.248
diff -u -p -r1.248 infrun.c
--- infrun.c 22 Sep 2007 19:33:31 -0000 1.248
+++ infrun.c 1 Oct 2007 00:11:09 -0000
@@ -881,6 +881,7 @@ enum infwait_states
{
infwait_normal_state,
infwait_thread_hop_state,
+ infwait_step_watch_state,
infwait_nonstep_watch_state
};
@@ -1220,17 +1221,12 @@ adjust_pc_after_break (struct execution_
by an event from the inferior, figure out what it means and take
appropriate action. */
-int stepped_after_stopped_by_watchpoint;
-
void
handle_inferior_event (struct execution_control_state *ecs)
{
- /* NOTE: bje/2005-05-02: If you're looking at this code and thinking
- that the variable stepped_after_stopped_by_watchpoint isn't used,
- then you're wrong! See remote.c:remote_stopped_data_address. */
-
int sw_single_step_trap_p = 0;
- int stopped_by_watchpoint = -1; /* Mark as unknown. */
+ int stopped_by_watchpoint;
+ int stepped_after_stopped_by_watchpoint = 0;
/* Cache the last pid/waitstatus. */
target_last_wait_ptid = ecs->ptid;
@@ -1250,7 +1246,14 @@ handle_inferior_event (struct execution_
case infwait_normal_state:
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: infwait_normal_state\n");
- stepped_after_stopped_by_watchpoint = 0;
+ break;
+
+ case infwait_step_watch_state:
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: infwait_step_watch_state\n");
+
+ stepped_after_stopped_by_watchpoint = 1;
break;
case infwait_nonstep_watch_state:
@@ -1435,7 +1438,7 @@ handle_inferior_event (struct execution_
stop_pc = read_pc ();
- stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid, 0);
+ stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
@@ -1483,7 +1486,7 @@ handle_inferior_event (struct execution_
ecs->saved_inferior_ptid = inferior_ptid;
inferior_ptid = ecs->ptid;
- stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid, 0);
+ stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
inferior_ptid = ecs->saved_inferior_ptid;
@@ -1796,24 +1799,20 @@ handle_inferior_event (struct execution_
singlestep_breakpoints_inserted_p = 0;
}
- /* It may not be necessary to disable the watchpoint to stop over
- it. For example, the PA can (with some kernel cooperation)
- single step over a watchpoint without disabling the watchpoint. */
- if (HAVE_STEPPABLE_WATCHPOINT && STOPPED_BY_WATCHPOINT (ecs->ws))
+ if (stepped_after_stopped_by_watchpoint)
+ stopped_by_watchpoint = 0;
+ else
+ stopped_by_watchpoint = watchpoints_triggered (&ecs->ws);
+
+ /* If necessary, step over this watchpoint. We'll be back to display
+ it in a moment. */
+ if (stopped_by_watchpoint
+ && (HAVE_STEPPABLE_WATCHPOINT
+ || gdbarch_have_nonsteppable_watchpoint (current_gdbarch)))
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: STOPPED_BY_WATCHPOINT\n");
- resume (1, 0);
- prepare_to_wait (ecs);
- return;
- }
- /* It is far more common to need to disable a watchpoint to step
- the inferior over it. FIXME. What else might a debug
- register or page protection watchpoint scheme need here? */
- if (gdbarch_have_nonsteppable_watchpoint (current_gdbarch)
- && STOPPED_BY_WATCHPOINT (ecs->ws))
- {
/* At this point, we are stopped at an instruction which has
attempted to write to a piece of memory under control of
a watchpoint. The instruction hasn't actually executed
@@ -1823,31 +1822,31 @@ handle_inferior_event (struct execution_
In order to make watchpoints work `right', we really need
to complete the memory write, and then evaluate the
- watchpoint expression. The following code does that by
- removing the watchpoint (actually, all watchpoints and
- breakpoints), single-stepping the target, re-inserting
- watchpoints, and then falling through to let normal
- single-step processing handle proceed. Since this
- includes evaluating watchpoints, things will come to a
- stop in the correct manner. */
+ watchpoint expression. We do this by single-stepping the
+ target.
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: STOPPED_BY_WATCHPOINT\n");
- remove_breakpoints ();
+ It may not be necessary to disable the watchpoint to stop over
+ it. For example, the PA can (with some kernel cooperation)
+ single step over a watchpoint without disabling the watchpoint.
+
+ It is far more common to need to disable a watchpoint to step
+ the inferior over it. If we have non-steppable watchpoints,
+ we must disable the current watchpoint; it's simplest to
+ disable all watchpoints and breakpoints. */
+
+ if (!HAVE_STEPPABLE_WATCHPOINT)
+ remove_breakpoints ();
registers_changed ();
target_resume (ecs->ptid, 1, TARGET_SIGNAL_0); /* Single step */
-
ecs->waiton_ptid = ecs->ptid;
- ecs->wp = &(ecs->ws);
- ecs->infwait_state = infwait_nonstep_watch_state;
+ if (HAVE_STEPPABLE_WATCHPOINT)
+ ecs->infwait_state = infwait_step_watch_state;
+ else
+ ecs->infwait_state = infwait_nonstep_watch_state;
prepare_to_wait (ecs);
return;
}
- /* It may be possible to simply continue after a watchpoint. */
- if (HAVE_CONTINUABLE_WATCHPOINT)
- stopped_by_watchpoint = STOPPED_BY_WATCHPOINT (ecs->ws);
-
ecs->stop_func_start = 0;
ecs->stop_func_end = 0;
ecs->stop_func_name = 0;
@@ -1969,8 +1968,7 @@ handle_inferior_event (struct execution_
else
{
/* See if there is a breakpoint at the current PC. */
- stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid,
- stopped_by_watchpoint);
+ stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
/* Following in case break condition called a
function. */
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.268
diff -u -p -r1.268 remote.c
--- remote.c 26 Sep 2007 18:32:54 -0000 1.268
+++ remote.c 1 Oct 2007 00:11:09 -0000
@@ -5406,14 +5406,11 @@ remote_stopped_by_watchpoint (void)
return remote_stopped_by_watchpoint_p;
}
-extern int stepped_after_stopped_by_watchpoint;
-
static int
remote_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
{
int rc = 0;
- if (remote_stopped_by_watchpoint ()
- || stepped_after_stopped_by_watchpoint)
+ if (remote_stopped_by_watchpoint ())
{
*addr_p = remote_watch_data_address;
rc = 1;
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.434
diff -u -p -r1.434 gdb.texinfo
--- doc/gdb.texinfo 28 Sep 2007 11:09:55 -0000 1.434
+++ doc/gdb.texinfo 1 Oct 2007 00:11:12 -0000
@@ -3346,20 +3346,13 @@ rerun the program, you will need to set
way of doing that would be to set a code breakpoint at the entry to the
@code{main} function and when it breaks, set all the watchpoints.
-@quotation
@cindex watchpoints and threads
@cindex threads and watchpoints
-@emph{Warning:} In multi-thread programs, watchpoints have only limited
-usefulness. With the current watchpoint implementation, @value{GDBN}
-can only watch the value of an expression @emph{in a single thread}. If
-you are confident that the expression can only change due to the current
-thread's activity (and if you are also confident that no other thread
-can become current), then you can use watchpoints as usual. However,
-@value{GDBN} may not notice when a non-current thread's activity changes
-the expression.
+In multi-threaded programs, watchpoints will detect changes to the
+watched expression from every thread.
-@c FIXME: this is almost identical to the previous paragraph.
-@emph{HP-UX Warning:} In multi-thread programs, software watchpoints
+@quotation
+@emph{Warning:} In multi-threaded programs, software watchpoints
have only limited usefulness. If @value{GDBN} creates a software
watchpoint, it can only watch the value of an expression @emph{in a
single thread}. If you are confident that the expression can only
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.266
diff -u -p -r1.266 gdbint.texinfo
--- doc/gdbint.texinfo 5 Jul 2007 12:22:32 -0000 1.266
+++ doc/gdbint.texinfo 1 Oct 2007 00:11:12 -0000
@@ -660,15 +660,26 @@ section is mostly irrelevant for softwar
When the inferior stops, @value{GDBN} tries to establish, among other
possible reasons, whether it stopped due to a watchpoint being hit.
-For a data-write watchpoint, it does so by evaluating, for each
-watchpoint, the expression whose value is being watched, and testing
-whether the watched value has changed. For data-read and data-access
-watchpoints, @value{GDBN} needs the target to supply a primitive that
-returns the address of the data that was accessed or read (see the
-description of @code{target_stopped_data_address} below): if this
-primitive returns a valid address, @value{GDBN} infers that a
-watchpoint triggered if it watches an expression whose evaluation uses
-that address.
+It first uses @code{STOPPED_BY_WATCHPOINT} to see if any watchpoint
+was hit. If not, all watchpoint checking is skipped.
+
+Then @value{GDBN} calls @code{target_stopped_data_address} exactly
+once. This method returns the address of the watchpoint which
+triggered, if the target can determine it. If the triggered address
+is available, @value{GDBN} compares the address returned by this
+method with each watched memory address in each active watchpoint.
+For data-read and data-access watchpoints, @value{GDBN} announces
+every watchpoint that watches the triggered address as being hit.
+For this reason, data-read and data-access watchpoints
+@emph{require} that the triggered address be available; if not, read
+and access watchpoints will never be considered hit. For data-write
+watchpoints, if the triggered address is available, @value{GDBN}
+considers only those watchpoints which match that address;
+otherwise, @value{GDBN} considers all data-write watchpoints. For
+each data-write watchpoint that @value{GDBN} considers, it evaluates
+the expression whose value is being watched, and tests whether the
+watched value has changed. Watchpoints whose watched values have
+changed are announced as hit.
@value{GDBN} uses several macros and primitives to support hardware
watchpoints:
@@ -721,26 +732,40 @@ These two macros should return 0 for suc
@item target_stopped_data_address (@var{addr_p})
If the inferior has some watchpoint that triggered, place the address
associated with the watchpoint at the location pointed to by
-@var{addr_p} and return non-zero. Otherwise, return zero. Note that
-this primitive is used by @value{GDBN} only on targets that support
-data-read or data-access type watchpoints, so targets that have
-support only for data-write watchpoints need not implement these
-primitives.
+@var{addr_p} and return non-zero. Otherwise, return zero. This
+is required for data-read and data-access watchpoints. It is
+not required for data-write watchpoints, but @value{GDBN} uses
+it to improve handling of those also.
+
+@value{GDBN} will only call this method once per watchpoint stop,
+immediately after calling @code{STOPPED_BY_WATCHPOINT}. If the
+target's watchpoint indication is sticky, i.e., stays set after
+resuming, this method should clear it. For instance, the x86 debug
+control register has sticky triggered flags.
@findex HAVE_STEPPABLE_WATCHPOINT
@item HAVE_STEPPABLE_WATCHPOINT
If defined to a non-zero value, it is not necessary to disable a
-watchpoint to step over it.
+watchpoint to step over it. Like @code{gdbarch_have_nonsteppable_watchpoint},
+this is usually set when watchpoints trigger at the instruction
+which will perform an interesting read or write. It should be
+set if there is a temporary disable bit which allows the processor
+to step over the interesting instruction without raising the
+watchpoint exception again.
@findex gdbarch_have_nonsteppable_watchpoint
@item int gdbarch_have_nonsteppable_watchpoint (@var{gdbarch})
If it returns a non-zero value, @value{GDBN} should disable a
-watchpoint to step the inferior over it.
+watchpoint to step the inferior over it. This is usually set when
+watchpoints trigger at the instruction which will perform an
+interesting read or write.
@findex HAVE_CONTINUABLE_WATCHPOINT
@item HAVE_CONTINUABLE_WATCHPOINT
If defined to a non-zero value, it is possible to continue the
-inferior after a watchpoint has been hit.
+inferior after a watchpoint has been hit. This is usually set
+when watchpoints trigger at the instruction following an interesting
+read or write.
@findex CANNOT_STEP_HW_WATCHPOINTS
@item CANNOT_STEP_HW_WATCHPOINTS
@@ -763,6 +788,32 @@ determine for sure whether the inferior
it could return non-zero ``just in case''.
@end table
+@subsection Watchpoints and Threads
+@cindex watchpoints, with threads
+
+@value{GDBN} only supports process-wide watchpoints, which trigger
+in all threads. @value{GDBN} uses the thread ID to make watchpoints
+act as if they were thread-specific, but it cannot set hardware
+watchpoints that only trigger in a specific thread. Therefore, even
+if the target supports threads, per-thread debug registers, and
+watchpoints which only affect a single thread, it should set the
+per-thread debug registers for all threads to the same value. On
+@sc{gnu}/Linux native targets, this is accomplished by using
+@code{ALL_LWPS} in @code{target_insert_watchpoint} and
+@code{target_remove_watchpoint} and by using
+@code{linux_set_new_thread} to register a handler for newly created
+threads.
+
+@value{GDBN}'s @sc{gnu}/Linux support only reports a single event
+at a time, although multiple events can trigger simultaneously for
+multi-threaded programs. When multiple events occur, @file{linux-nat.c}
+queues subsequent events and returns them the next time the program
+is resumed. This means that @code{STOPPED_BY_WATCHPOINT} and
+@code{target_stopped_data_address} only need to consult the current
+thread's state---the thread indicated by @code{inferior_ptid}. If
+two threads have hit watchpoints simultaneously, those routines
+will be called a second time for the second thread.
+
@subsection x86 Watchpoints
@cindex x86 debug registers
@cindex watchpoints, on x86
Index: testsuite/gdb.threads/watchthreads.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/watchthreads.c,v
retrieving revision 1.3
diff -u -p -r1.3 watchthreads.c
--- testsuite/gdb.threads/watchthreads.c 23 Aug 2007 18:08:50 -0000 1.3
+++ testsuite/gdb.threads/watchthreads.c 1 Oct 2007 00:11:12 -0000
@@ -56,7 +56,7 @@ void *thread_function(void *arg) {
/* Don't run forever. Run just short of it :) */
while (*myp > 0)
{
- (*myp) ++; /* Loop increment. */
+ (*myp) ++; usleep (1); /* Loop increment. */
}
pthread_exit(NULL);
Index: testsuite/gdb.threads/watchthreads.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/watchthreads.exp,v
retrieving revision 1.4
diff -u -p -r1.4 watchthreads.exp
--- testsuite/gdb.threads/watchthreads.exp 23 Aug 2007 18:14:19 -0000 1.4
+++ testsuite/gdb.threads/watchthreads.exp 1 Oct 2007 00:11:12 -0000
@@ -30,6 +30,10 @@ if [target_info exists gdb,no_hardware_w
return 0;
}
+proc target_no_stopped_data { } {
+ return [istarget s390*-*-*]
+}
+
set testfile "watchthreads"
set srcfile ${testfile}.c
set binfile ${objdir}/${subdir}/${testfile}
@@ -61,20 +65,58 @@ gdb_test "watch args\[1\]" "Hardware wat
set init_line [expr [gdb_get_line_number "Init value"]+1]
set inc_line [gdb_get_line_number "Loop increment"]
+set main_loc "main \\\(\\\) at .*watchthreads.c:$init_line"
+set thread0_loc "thread_function \\\(arg=0x0\\\) at .*watchthreads.c:$inc_line"
+set thread1_loc "thread_function \\\(arg=0x1\\\) at .*watchthreads.c:$inc_line"
# Loop and continue to allow both watchpoints to be triggered.
for {set i 0} {$i < 30} {incr i} {
+ set test_flag_0 0
+ set test_flag_1 0
set test_flag 0
gdb_test_multiple "continue" "threaded watch loop" {
- -re "Hardware watchpoint 2: args\\\[0\\\].*Old value = 0.*New value = 1.*main \\\(\\\) at .*watchthreads.c:$init_line.*$gdb_prompt $"
- { set args_0 1; set test_flag 1 }
- -re "Hardware watchpoint 3: args\\\[1\\\].*Old value = 0.*New value = 1.*main \\\(\\\) at .*watchthreads.c:$init_line.*$gdb_prompt $"
- { set args_1 1; set test_flag 1 }
- -re "Hardware watchpoint 2: args\\\[0\\\].*Old value = $args_0.*New value = [expr $args_0+1].*in thread_function \\\(arg=0x0\\\) at .*watchthreads.c:$inc_line.*$gdb_prompt $"
- { set args_0 [expr $args_0+1]; set test_flag 1 }
- -re "Hardware watchpoint 3: args\\\[1\\\].*Old value = $args_1.*New value = [expr $args_1+1].*in thread_function \\\(arg=0x1\\\) at .*watchthreads.c:$inc_line.*$gdb_prompt $"
- { set args_1 [expr $args_1+1]; set test_flag 1 }
+ -re "(.*Hardware watchpoint.*)$gdb_prompt $" {
+ # At least one hardware watchpoint was hit. Check if both were.
+ set string $expect_out(1,string)
+
+ if [regexp "Hardware watchpoint 2: args\\\[0\\\]\[^\r\]*\r\[^\r\]*\r\[^\r\]*Old value = $args_0\[^\r\]*\r\[^\r\]*New value = [expr $args_0+1]\r" $string] {
+ incr args_0
+ incr test_flag_0
+ }
+ if [regexp "Hardware watchpoint 3: args\\\[1\\\]\[^\r\]*\r\[^\r\]*\r\[^\r\]*Old value = $args_1\[^\r\]*\r\[^\r\]*New value = [expr $args_1+1]\r" $string] {
+ incr args_1
+ incr test_flag_1
+ }
+
+ set expected_loc "bogus location"
+ if { $test_flag_0 == 1 && $test_flag_1 == 0 && $args_0 == 1 } {
+ set expected_loc $main_loc
+ } elseif { $test_flag_0 == 0 && $test_flag_1 == 1 && $args_1 == 1 } {
+ set expected_loc $main_loc
+ } elseif { $test_flag_0 == 1 && $test_flag_1 == 0 } {
+ set expected_loc $thread0_loc
+ } elseif { $test_flag_0 == 0 && $test_flag_1 == 1 } {
+ set expected_loc $thread1_loc
+ } elseif { $test_flag_0 + $test_flag_1 == 2 } {
+ # On S/390, or any other system which can not report the
+ # stopped data address, it is OK to report two watchpoints
+ # at once in this test. Make sure the reported location
+ # corresponds to at least one of the watchpoints (and not,
+ # e.g., __nptl_create_event). On other systems, we should
+ # report the two watchpoints serially.
+ if { [target_no_stopped_data] } {
+ set expected_loc "($main_loc|$thread0_loc|$thread1_loc)"
+ }
+ }
+
+ if [ regexp "$expected_loc" $string ] {
+ set test_flag 1
+ } else {
+ fail "threaded watch loop"
+ }
+ }
}
+
# If we fail above, don't bother continuing loop
if { $test_flag == 0 } {
set i 30;
@@ -120,7 +162,14 @@ if { $args_1 > 1 } {
# Verify that all watchpoint hits are accounted for.
set message "combination of threaded watchpoints = 30"
-if { [expr $args_0+$args_1] == 30 } {
+if { [target_no_stopped_data] } {
+ # See above. If we allow two watchpoints to be hit at once, we
+ # may have more than 30 hits total.
+ set result [expr $args_0 + $args_1 >= 30]
+} else {
+ set result [expr $args_0 + $args_1 == 30]
+}
+if { $result } {
pass $message
} else {
fail $message