2008-10-15 Pedro Alves * breakpoint.c (breakpoint_init_inferior): Clean up the moribund locations list. (moribund_breakpoint_here_p): Record the moribund location in the moribund_locations vector. * breakpoint.h (moribund_breakpoint_here_p): Declare. (displaced_step_fixup): Check if the breakpoint the thread was trying to step over has been removed since having been placed in the displaced stepping queue. (adjust_pc_after_break): In non-stop mode, check for a moribund breakpoint at the stop pc. (handle_inferior_event): Don't retire moribund breakpoints on TARGET_WAITKIND_IGNORE. --- gdb/breakpoint.c | 38 ++++++++++++++++++++----- gdb/breakpoint.h | 2 + gdb/infrun.c | 82 ++++++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 92 insertions(+), 30 deletions(-) Index: src/gdb/breakpoint.c =================================================================== --- src.orig/gdb/breakpoint.c 2008-10-15 18:39:06.000000000 +0100 +++ src/gdb/breakpoint.c 2008-10-15 18:48:46.000000000 +0100 @@ -1744,6 +1744,7 @@ breakpoint_init_inferior (enum inf_conte { struct breakpoint *b, *temp; struct bp_location *bpt; + int ix; ALL_BP_LOCATIONS (bpt) if (bpt->owner->enable_state != bp_permanent) @@ -1786,6 +1787,11 @@ breakpoint_init_inferior (enum inf_conte break; } } + + /* Get rid of the moribund locations. */ + for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, bpt); ++ix) + free_bp_location (bpt); + VEC_free (bp_location_p, moribund_locations); } /* breakpoint_here_p (PC) returns non-zero if an enabled breakpoint @@ -1828,6 +1834,20 @@ breakpoint_here_p (CORE_ADDR pc) return any_breakpoint_here ? ordinary_breakpoint_here : 0; } +/* Return true if there's a moribund breakpoint at PC. */ + +int +moribund_breakpoint_here_p (CORE_ADDR pc) +{ + struct bp_location *loc; + int ix; + + for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix) + if (loc->address == pc) + return 1; + + return 0; +} /* Returns non-zero if there's a breakpoint inserted at PC, which is inserted using regular breakpoint_chain/bp_location_chain mechanism. @@ -7107,8 +7127,8 @@ update_global_location_list (int should_ } if (!found_object) - { - if (removed) + { + if (removed && non_stop) { /* This location was removed from the targets. In non-stop mode, a race condition is possible where we've removed a breakpoint, @@ -7116,20 +7136,22 @@ update_global_location_list (int should_ 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, + 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 + 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. */ + which is usability issue, but not a correctness problem. */ loc->events_till_retirement = 3 * (thread_count () + 1); loc->owner = NULL; - } - free_bp_location (loc); + VEC_safe_push (bp_location_p, moribund_locations, loc); + } + else + free_bp_location (loc); } } - + ALL_BREAKPOINTS (b) { check_duplicates (b); Index: src/gdb/breakpoint.h =================================================================== --- src.orig/gdb/breakpoint.h 2008-10-15 18:39:06.000000000 +0100 +++ src/gdb/breakpoint.h 2008-10-15 18:48:46.000000000 +0100 @@ -681,6 +681,8 @@ enum breakpoint_here extern enum breakpoint_here breakpoint_here_p (CORE_ADDR); +extern int moribund_breakpoint_here_p (CORE_ADDR); + extern int breakpoint_inserted_here_p (CORE_ADDR); extern int regular_breakpoint_inserted_here_p (CORE_ADDR); Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c 2008-10-15 18:39:06.000000000 +0100 +++ src/gdb/infrun.c 2008-10-15 18:52:09.000000000 +0100 @@ -789,40 +789,71 @@ displaced_step_fixup (ptid_t event_ptid, do_cleanups (old_cleanups); + displaced_step_ptid = null_ptid; + /* Are there any pending displaced stepping requests? If so, run one now. */ - if (displaced_step_request_queue) + while (displaced_step_request_queue) { struct displaced_step_request *head; ptid_t ptid; + CORE_ADDR actual_pc; head = displaced_step_request_queue; ptid = head->ptid; displaced_step_request_queue = head->next; xfree (head); - if (debug_displaced) - fprintf_unfiltered (gdb_stdlog, - "displaced: stepping queued %s now\n", - target_pid_to_str (ptid)); - - displaced_step_ptid = null_ptid; - displaced_step_prepare (ptid); context_switch (ptid); - if (debug_displaced) + actual_pc = read_pc (); + + if (breakpoint_here_p (actual_pc)) { - struct regcache *resume_regcache = get_thread_regcache (ptid); - CORE_ADDR actual_pc = regcache_read_pc (resume_regcache); - gdb_byte buf[4]; + if (debug_displaced) + fprintf_unfiltered (gdb_stdlog, + "displaced: stepping queued %s now\n", + target_pid_to_str (ptid)); + + displaced_step_prepare (ptid); + + if (debug_displaced) + { + gdb_byte buf[4]; + + fprintf_unfiltered (gdb_stdlog, "displaced: run 0x%s: ", + paddr_nz (actual_pc)); + read_memory (actual_pc, buf, sizeof (buf)); + displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf)); + } - fprintf_unfiltered (gdb_stdlog, "displaced: run 0x%s: ", - paddr_nz (actual_pc)); - read_memory (actual_pc, buf, sizeof (buf)); - displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf)); + target_resume (ptid, 1, TARGET_SIGNAL_0); + + /* Done, we're stepping a thread. */ + break; } + else + { + int step; + struct thread_info *tp = inferior_thread (); + + /* The breakpoint we were sitting under has since been + removed. */ + tp->trap_expected = 0; + + /* Go back to what we were trying to do. */ + step = currently_stepping (tp); + + if (debug_displaced) + fprintf_unfiltered (gdb_stdlog, "breakpoint is gone %s: step(%d)\n", + target_pid_to_str (tp->ptid), step); - target_resume (ptid, 1, TARGET_SIGNAL_0); + target_resume (ptid, step, TARGET_SIGNAL_0); + tp->stop_signal = TARGET_SIGNAL_0; + + /* This request was discarded. See if there's any other + thread waiting for its turn. */ + } } } @@ -1798,9 +1829,16 @@ adjust_pc_after_break (struct execution_ breakpoint_pc = regcache_read_pc (regcache) - gdbarch_decr_pc_after_break (gdbarch); - /* Check whether there actually is a software breakpoint inserted - at that location. */ - if (software_breakpoint_inserted_here_p (breakpoint_pc)) + /* Check whether there actually is a software breakpoint inserted at + that location. + + If in non-stop mode, a race condition is possible where we've + removed a breakpoint, but stop events for that breakpoint were + already queued and arrive later. To suppress those spurious + SIGTRAPs, we keep a list of such breakpoint locations for a bit, + and retire them after a number of stop events are reported. */ + if (software_breakpoint_inserted_here_p (breakpoint_pc) + || (non_stop && moribund_breakpoint_here_p (breakpoint_pc))) { /* When using hardware single-step, a SIGTRAP is reported for both a completed single-step and a software breakpoint. Need to @@ -1873,8 +1911,6 @@ handle_inferior_event (struct execution_ else stop_soon = NO_STOP_QUIETLY; - breakpoint_retire_moribund (); - /* Cache the last pid/waitstatus. */ target_last_wait_ptid = ecs->ptid; target_last_waitstatus = ecs->ws; @@ -1902,6 +1938,8 @@ handle_inferior_event (struct execution_ if (ecs->ws.kind != TARGET_WAITKIND_IGNORE) { + breakpoint_retire_moribund (); + /* Mark the non-executing threads accordingly. */ if (!non_stop || ecs->ws.kind == TARGET_WAITKIND_EXITED