diff options
| -rw-r--r-- | gdb/ChangeLog | 15 | ||||
| -rw-r--r-- | gdb/breakpoint.c | 103 | ||||
| -rw-r--r-- | gdb/breakpoint.h | 16 | ||||
| -rw-r--r-- | gdb/gdbthread.h | 2 | ||||
| -rw-r--r-- | gdb/infrun.c | 2 | ||||
| -rw-r--r-- | gdb/thread.c | 12 |
6 files changed, 131 insertions, 19 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 25b8de3..d54679d 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,18 @@ +2008-06-28 Vladimir Prus <vladimir@codesourcery.com> + + * breakpoint.c (moribund_locations): New. + (bpstat_stop_status): Process moribund locations. + (update_global_location_list): Add removed + locations to moribund_locations. + (breakpoint_retire_moribund): New. + * breakpoint.h (struct bp_location): New field + events_till_retirement. + (breakpoint_retire_moribund): Declare. + * thread.c (thread_count): New. + * infrun.c (handle_inferior_event): Call + breakpoint_retire_moribund. + * gdbthread.h (thread_count): Declare. + 2008-06-27 Joseph Myers <joseph@codesourcery.com> * dfp.c (decimal_convert): Call match_endianness before and after diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 26eff8d..73d962c 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -308,6 +308,11 @@ struct breakpoint *breakpoint_chain; struct bp_location *bp_location_chain; +/* The locations that no longer correspond to any breakpoint, + unlinked from bp_location_chain, but for which a hit + may still be reported by a target. */ +VEC(bp_location_p) *moribund_locations = NULL; + /* Number of last breakpoint made. */ int breakpoint_count; @@ -3003,10 +3008,12 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid) { struct breakpoint *b = NULL; const struct bp_location *bl; + struct bp_location *loc; /* Root of the chain of bpstat's */ struct bpstats root_bs[1]; /* Pointer to the last thing in the chain currently. */ bpstat bs = root_bs; + int ix; ALL_BP_LOCATIONS (bl) { @@ -3075,6 +3082,18 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid) bs->print_it = print_it_noop; } + for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix) + { + if (loc->address == bp_addr) + { + bs = bpstat_alloc (loc, bs); + /* For hits of moribund locations, we should just proceed. */ + bs->stop = 0; + bs->print = 0; + bs->print_it = print_it_noop; + } + } + bs->next = NULL; /* Terminate the chain */ bs = root_bs->next; /* Re-grab the head of the chain */ @@ -3089,6 +3108,7 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid) if (bs == NULL) for (bs = root_bs->next; bs != NULL; bs = bs->next) if (!bs->stop + && bs->breakpoint_at->owner && (bs->breakpoint_at->owner->type == bp_hardware_watchpoint || bs->breakpoint_at->owner->type == bp_read_watchpoint || bs->breakpoint_at->owner->type == bp_access_watchpoint)) @@ -3253,6 +3273,9 @@ bpstat_what (bpstat bs) /* I suspect this can happen if it was a momentary breakpoint which has since been deleted. */ continue; + if (bs->breakpoint_at->owner == NULL) + bs_class = bp_nostop; + else switch (bs->breakpoint_at->owner->type) { case bp_none: @@ -6956,7 +6979,9 @@ breakpoint_auto_delete (bpstat bs) struct breakpoint *b, *temp; for (; bs; bs = bs->next) - if (bs->breakpoint_at && bs->breakpoint_at->owner->disposition == disp_del + if (bs->breakpoint_at + && bs->breakpoint_at->owner + && bs->breakpoint_at->owner->disposition == disp_del && bs->stop) delete_breakpoint (bs->breakpoint_at->owner); @@ -7004,6 +7029,9 @@ update_global_location_list (void) /* Tells if 'loc' is found amoung the new locations. If not, we have to free it. */ int found_object = 0; + /* Tells if the location should remain inserted in the target. */ + int keep_in_target = 0; + int removed = 0; for (loc2 = bp_location_chain; loc2; loc2 = loc2->global_next) if (loc2 == loc) { @@ -7020,13 +7048,12 @@ update_global_location_list (void) if (loc->inserted) { /* If the location is inserted now, we might have to remove it. */ - int keep = 0; if (found_object && should_be_inserted (loc)) { /* The location is still present in the location list, and still should be inserted. Don't do anything. */ - keep = 1; + keep_in_target = 1; } else { @@ -7044,30 +7071,53 @@ update_global_location_list (void) { loc2->inserted = 1; loc2->target_info = loc->target_info; - keep = 1; + keep_in_target = 1; break; } } } - if (!keep) - if (remove_breakpoint (loc, mark_uninserted)) - { - /* This is just about all we can do. We could keep this - location on the global list, and try to remove it next - time, but there's no particular reason why we will - succeed next time. - - Note that at this point, loc->owner is still valid, - as delete_breakpoint frees the breakpoint only - after calling us. */ - printf_filtered (_("warning: Error removing breakpoint %d\n"), - loc->owner->number); - } + if (!keep_in_target) + { + if (remove_breakpoint (loc, mark_uninserted)) + { + /* This is just about all we can do. We could keep this + location on the global list, and try to remove it next + time, but there's no particular reason why we will + succeed next time. + + Note that at this point, loc->owner is still valid, + as delete_breakpoint frees the breakpoint only + after calling us. */ + printf_filtered (_("warning: Error removing breakpoint %d\n"), + loc->owner->number); + } + removed = 1; + } } if (!found_object) - free_bp_location (loc); + { + if (removed) + { + /* This location was removed from the targets. In non-stop mode, + a race condition is possible where we've removed a breakpoint, + but stop events for that breakpoint are already queued and will + arrive later. To suppress spurious SIGTRAPs reported to user, + we keep this breakpoint location for a bit, and will retire it + after we see 3 * thread_count events. + The theory here is that reporting of events should, + "on the average", be fair, so after that many event we'll see + events from all threads that have anything of interest, and no + longer need to keep this breakpoint. This is just a + heuristic, but if it's wrong, we'll report unexpected SIGTRAP, + which is usability issue, but not a correctness problem. */ + loc->events_till_retirement = 3 * (thread_count () + 1); + loc->owner = NULL; + } + + free_bp_location (loc); + } } ALL_BREAKPOINTS (b) @@ -7079,6 +7129,21 @@ update_global_location_list (void) insert_breakpoint_locations (); } +void +breakpoint_retire_moribund (void) +{ + struct bp_location *loc; + int ix; + + for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix) + if (--(loc->events_till_retirement) == 0) + { + free_bp_location (loc); + VEC_unordered_remove (bp_location_p, moribund_locations, ix); + --ix; + } +} + static void update_global_location_list_nothrow (void) { diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 80544e4..2c98d64 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -298,6 +298,17 @@ struct bp_location /* Similarly, for the breakpoint at an overlay's LMA, if necessary. */ struct bp_target_info overlay_target_info; + + /* In a non-stop mode, it's possible that we delete a breakpoint, + but as we do that, some still running thread hits that breakpoint. + For that reason, we need to keep locations belonging to deleted + breakpoints for a bit, so that don't report unexpected SIGTRAP. + We can't keep such locations forever, so we use a heuristic -- + after we process certain number of inferior events since + breakpoint was deleted, we retire all locations of that breakpoint. + This variable keeps a number of events still to go, when + it becomes 0 this location is retired. */ + int events_till_retirement; }; /* This structure is a collection of function pointers that, if available, @@ -865,4 +876,9 @@ void breakpoint_restore_shadows (gdb_byte *buf, ULONGEST memaddr, extern int breakpoints_always_inserted_mode (void); +/* Called each time new event from target is processed. + Retires previously deleted breakpoint locations that + in our opinion won't ever trigger. */ +extern void breakpoint_retire_moribund (void); + #endif /* !defined (BREAKPOINT_H) */ diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index 4440417..9ce2753 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -118,6 +118,8 @@ extern struct thread_info *find_thread_pid (ptid_t ptid); typedef int (*thread_callback_func) (struct thread_info *, void *); extern struct thread_info *iterate_over_threads (thread_callback_func, void *); +extern int thread_count (void); + /* infrun context switch: save the debugger state for the given thread. */ extern void save_infrun_state (ptid_t ptid, CORE_ADDR prev_pc, diff --git a/gdb/infrun.c b/gdb/infrun.c index 73b92be..2c4ebc8 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1705,6 +1705,8 @@ handle_inferior_event (struct execution_control_state *ecs) int stopped_by_watchpoint; int stepped_after_stopped_by_watchpoint = 0; + breakpoint_retire_moribund (); + /* Cache the last pid/waitstatus. */ target_last_wait_ptid = ecs->ptid; target_last_waitstatus = *ecs->wp; diff --git a/gdb/thread.c b/gdb/thread.c index 80d745d..5b1b563 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -230,6 +230,18 @@ iterate_over_threads (int (*callback) (struct thread_info *, void *), } int +thread_count (void) +{ + int result = 0; + struct thread_info *tp; + + for (tp = thread_list; tp; tp = tp->next) + ++result; + + return result; +} + +int valid_thread_id (int num) { struct thread_info *tp; |