diff options
| -rw-r--r-- | gdb/ChangeLog | 25 | ||||
| -rw-r--r-- | gdb/breakpoint.c | 334 | ||||
| -rw-r--r-- | gdb/breakpoint.h | 24 | ||||
| -rw-r--r-- | gdb/doc/ChangeLog | 9 | ||||
| -rw-r--r-- | gdb/doc/gdb.texinfo | 17 | ||||
| -rw-r--r-- | gdb/doc/gdbint.texinfo | 85 | ||||
| -rw-r--r-- | gdb/infrun.c | 86 | ||||
| -rw-r--r-- | gdb/remote.c | 5 | ||||
| -rw-r--r-- | gdb/testsuite/ChangeLog | 7 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.threads/watchthreads.c | 2 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.threads/watchthreads.exp | 67 |
11 files changed, 434 insertions, 227 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 8ae983d..1ecf2e4 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,28 @@ +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-29 Daniel Jacobowitz <dan@codesourcery.com> * configure.ac: Add $LIBINTL when testing libbfd. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0d1cb9e..e4fdb33 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2537,6 +2537,83 @@ bpstat_alloc (struct bp_location *bl, bpstat cbs /* Current "bs" value */ ) 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, ptid_t ptid, int stopped_by_watchpoint) 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, ptid_t ptid, int stopped_by_watchpoint) 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; - - 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; - - 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) + int must_check_value = 0; + + 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; + + 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, ptid_t ptid, int stopped_by_watchpoint) ++(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, ptid_t ptid, int stopped_by_watchpoint) 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, ptid_t ptid, int stopped_by_watchpoint) { 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, ptid_t ptid, int stopped_by_watchpoint) 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, ptid_t ptid, int stopped_by_watchpoint) 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 accessflag, int from_tty) 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 accessflag, int from_tty) 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); } diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 6dfb27c..1e68a2a 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -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_breakpoints (void); 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) */ diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index 4c8e7cd..e7f03fd 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,12 @@ +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-28 Vladimir Prus <vladimir@codesourcery.com> * gdb.texinfo (Setting Breakpoints): Revise diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 3c4d99d..fac3f67 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -3346,20 +3346,13 @@ rerun the program, you will need to set all such watchpoints again. One 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. - -@c FIXME: this is almost identical to the previous paragraph. -@emph{HP-UX Warning:} In multi-thread programs, software watchpoints +In multi-threaded programs, watchpoints will detect changes to the +watched expression from every thread. + +@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 diff --git a/gdb/doc/gdbint.texinfo b/gdb/doc/gdbint.texinfo index e6d9e87..facea70 100644 --- a/gdb/doc/gdbint.texinfo +++ b/gdb/doc/gdbint.texinfo @@ -660,15 +660,26 @@ section is mostly irrelevant for software watchpoints. 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 success, non-zero for failure. @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 stopped due to a watchpoint, 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 diff --git a/gdb/infrun.c b/gdb/infrun.c index f1a96a3..f03a5f2 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -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_control_state *ecs) 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_control_state *ecs) 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_control_state *ecs) 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_control_state *ecs) 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_control_state *ecs) 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_control_state *ecs) 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. */ - - if (debug_infrun) - fprintf_unfiltered (gdb_stdlog, "infrun: STOPPED_BY_WATCHPOINT\n"); - remove_breakpoints (); + watchpoint expression. We do this by single-stepping the + target. + + 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_control_state *ecs) 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. */ diff --git a/gdb/remote.c b/gdb/remote.c index 309ee67..23fa18c 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -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; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index e5212be..f4882ef 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +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. + 2007-09-27 Vladimir Prus <vladimir@codesourcery.com> * gdb.mi/var-cmd.c (do_children_tests): Initialize diff --git a/gdb/testsuite/gdb.threads/watchthreads.c b/gdb/testsuite/gdb.threads/watchthreads.c index c607429..ef391f0 100644 --- a/gdb/testsuite/gdb.threads/watchthreads.c +++ b/gdb/testsuite/gdb.threads/watchthreads.c @@ -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); diff --git a/gdb/testsuite/gdb.threads/watchthreads.exp b/gdb/testsuite/gdb.threads/watchthreads.exp index cdd3edb..11fc2af 100644 --- a/gdb/testsuite/gdb.threads/watchthreads.exp +++ b/gdb/testsuite/gdb.threads/watchthreads.exp @@ -30,6 +30,10 @@ if [target_info exists gdb,no_hardware_watchpoints] { 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 watchpoint 3: args\\\[1\\\]" 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 |