From patchwork Fri Oct 3 13:54:17 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 3090 Received: (qmail 19477 invoked by alias); 3 Oct 2014 13:54:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 19371 invoked by uid 89); 3 Oct 2014 13:54:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 03 Oct 2014 13:54:30 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s93DsRBb006783 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 3 Oct 2014 09:54:28 -0400 Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s93DsJ7f025501 for ; Fri, 3 Oct 2014 09:54:27 -0400 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 7/8] Make single-step breakpoints be per-thread Date: Fri, 3 Oct 2014 14:54:17 +0100 Message-Id: <1412344458-31774-8-git-send-email-palves@redhat.com> In-Reply-To: <1412344458-31774-1-git-send-email-palves@redhat.com> References: <1412344458-31774-1-git-send-email-palves@redhat.com> This patch finally makes each thread have its own set of single-step breakpoints. This paves the way to have multiple threads software single-stepping, though this patch doesn't flip that switch on yet. That'll be done on a subsequent patch. gdb/ 2014-10-03 Pedro Alves * breakpoint.c (single_step_breakpoints): Delete global. (insert_single_step_breakpoint): Adjust to store the breakpoint pointer in the current thread. (single_step_breakpoints_inserted, remove_single_step_breakpoints) (cancel_single_step_breakpoints): Delete functions. (breakpoint_has_location_inserted_here): Make extern. (single_step_breakpoint_inserted_here_p): Adjust to walk the breakpoint list. * breakpoint.h (breakpoint_has_location_inserted_here): New declaration. (single_step_breakpoints_inserted, remove_single_step_breakpoints) (cancel_single_step_breakpoints): Remove declarations. * gdbthread.h (struct thread_control_state) : New field. (delete_single_step_breakpoints) (thread_has_single_step_breakpoints_set) (thread_has_single_step_breakpoint_here): New declarations. * infrun.c (follow_exec): Also clear the single-step breakpoints. (singlestep_breakpoints_inserted_p, singlestep_ptid) (singlestep_pc): Delete globals. (infrun_thread_ptid_changed): Remove references to removed globals. (resume_cleanups): Delete the current thread's single-step breakpoints. (maybe_software_singlestep): Remove references to removed globals. (resume): Adjust to use thread_has_single_step_breakpoints_set and delete_single_step_breakpoints. (init_wait_for_inferior): Remove references to removed globals. (delete_thread_infrun_breakpoints): Delete the thread's single-step breakpoints too. (delete_just_stopped_threads_infrun_breakpoints): Don't delete single-step breakpoints here. (delete_stopped_threads_single_step_breakpoints): New function. (adjust_pc_after_break): Adjust to use thread_has_single_step_breakpoints_set. (handle_inferior_event): Remove references to removed globals. Use delete_stopped_threads_single_step_breakpoints. (handle_signal_stop): Adjust to per-thread single-step breakpoints. Swap test order to do cheaper tests first. (switch_back_to_stepped_thread): Extend debug output. Remove references to removed globals. * record-full.c (record_full_wait_1): Adjust to per-thread single-step breakpoints. * thread.c (delete_single_step_breakpoints) (thread_has_single_step_breakpoints_set) (thread_has_single_step_breakpoint_here): New functions. (clear_thread_inferior_resources): Also delete the thread's single-step breakpoints. --- gdb/breakpoint.c | 64 ++++++++----------------------- gdb/breakpoint.h | 10 +++-- gdb/gdbthread.h | 20 ++++++++++ gdb/infrun.c | 113 ++++++++++++++++++++++-------------------------------- gdb/record-full.c | 8 ++-- gdb/thread.c | 31 +++++++++++++++ 6 files changed, 123 insertions(+), 123 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 90a8c43..be3f80a 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -326,11 +326,6 @@ static struct breakpoint_ops bkpt_probe_breakpoint_ops; /* Dynamic printf class type. */ struct breakpoint_ops dprintf_breakpoint_ops; -/* One (or perhaps two) breakpoints used for software single - stepping. */ - -static struct breakpoint *single_step_breakpoints; - /* The style in which to perform a dynamic printf. This is a user option because different output options have different tradeoffs; if GDB does the printing, there is better error handling if there @@ -15318,57 +15313,24 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal; CORE_ADDR pc = next_pc; - if (single_step_breakpoints == NULL) - single_step_breakpoints = new_single_step_breakpoint (tp->num, gdbarch); + if (tp->control.single_step_breakpoints == NULL) + { + tp->control.single_step_breakpoints + = new_single_step_breakpoint (tp->num, gdbarch); + } sal = find_pc_line (pc, 0); sal.pc = pc; sal.section = find_pc_overlay (pc); sal.explicit_pc = 1; - add_location_to_breakpoint (single_step_breakpoints, &sal); + add_location_to_breakpoint (tp->control.single_step_breakpoints, &sal); update_global_location_list (UGLL_INSERT); } -/* Check if the breakpoints used for software single stepping - were inserted or not. */ +/* See breakpoint.h. */ int -single_step_breakpoints_inserted (void) -{ - return (single_step_breakpoints != NULL); -} - -/* Remove and delete any breakpoints used for software single step. */ - -void -remove_single_step_breakpoints (void) -{ - gdb_assert (single_step_breakpoints != NULL); - - delete_breakpoint (single_step_breakpoints); - - single_step_breakpoints = NULL; -} - -/* Delete software single step breakpoints without removing them from - the inferior. This is intended to be used if the inferior's address - space where they were inserted is already gone, e.g. after exit or - exec. */ - -void -cancel_single_step_breakpoints (void) -{ - /* We don't really need to (or should) delete them here. After an - exit, breakpoint_init_inferior deletes it. After an exec, - update_breakpoints_after_exec does it. Just clear our - reference. */ - single_step_breakpoints = NULL; -} - -/* Check whether any location of BP is inserted at PC. */ - -static int breakpoint_has_location_inserted_here (struct breakpoint *bp, struct address_space *aspace, CORE_ADDR pc) @@ -15390,9 +15352,15 @@ int single_step_breakpoint_inserted_here_p (struct address_space *aspace, CORE_ADDR pc) { - return (single_step_breakpoints != NULL - && breakpoint_has_location_inserted_here (single_step_breakpoints, - aspace, pc)); + struct breakpoint *bpt; + + ALL_BREAKPOINTS (bpt) + { + if (bpt->type == bp_single_step + && breakpoint_has_location_inserted_here (bpt, aspace, pc)) + return 1; + } + return 0; } /* Returns 0 if 'bp' is NOT a syscall catchpoint, diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 8b833bd..cd3e7ee 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1129,6 +1129,12 @@ extern int regular_breakpoint_inserted_here_p (struct address_space *, extern int software_breakpoint_inserted_here_p (struct address_space *, CORE_ADDR); +/* Check whether any location of BP is inserted at PC. */ + +extern int breakpoint_has_location_inserted_here (struct breakpoint *bp, + struct address_space *aspace, + CORE_ADDR pc); + extern int single_step_breakpoint_inserted_here_p (struct address_space *, CORE_ADDR); @@ -1463,10 +1469,6 @@ extern void delete_command (char *arg, int from_tty); extern void insert_single_step_breakpoint (struct gdbarch *, struct address_space *, CORE_ADDR); -extern int single_step_breakpoints_inserted (void); -extern void remove_single_step_breakpoints (void); -extern void cancel_single_step_breakpoints (void); - /* Check if any hardware watchpoints have triggered, according to the target. */ int watchpoints_triggered (struct target_waitstatus *); diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index 36ad5a7..178fd4a 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -52,6 +52,13 @@ struct thread_control_state /* Exception-resume breakpoint. */ struct breakpoint *exception_resume_breakpoint; + /* Breakpoints used for software single stepping. Plural, because + it may have multiple locations. E.g., if stepping over a + conditional branch instruction we can't decode the condition for, + we'll need to put a breakpoint at the branch destination, and + another at the instruction after the branch. */ + struct breakpoint *single_step_breakpoints; + /* Range to single step within. If this is nonzero, respond to a single-step signal by continuing @@ -285,6 +292,19 @@ extern void delete_step_resume_breakpoint (struct thread_info *); /* Delete an exception_resume_breakpoint from the thread database. */ extern void delete_exception_resume_breakpoint (struct thread_info *); +/* Delete the single-step breakpoints of thread TP, if any. */ +extern void delete_single_step_breakpoints (struct thread_info *tp); + +/* Check if the thread has software single stepping breakpoints + set. */ +extern int thread_has_single_step_breakpoints_set (struct thread_info *tp); + +/* Check whether the thread has software single stepping breakpoints + set at PC. */ +extern int thread_has_single_step_breakpoint_here (struct thread_info *tp, + struct address_space *aspace, + CORE_ADDR addr); + /* Translate the integer thread id (GDB's homegrown id, not the system's) into a "pid" (which may be overloaded with extra thread information). */ extern ptid_t thread_id_to_pid (int); diff --git a/gdb/infrun.c b/gdb/infrun.c index 0948a5b..9ad3beb 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1083,6 +1083,7 @@ follow_exec (ptid_t pid, char *execd_pathname) statement through an exec(). */ th->control.step_resume_breakpoint = NULL; th->control.exception_resume_breakpoint = NULL; + th->control.single_step_breakpoints = NULL; th->control.step_range_start = 0; th->control.step_range_end = 0; @@ -1196,17 +1197,6 @@ follow_exec (ptid_t pid, char *execd_pathname) matically get reset there in the new process.). */ } -/* Non-zero if we just simulating a single-step. This is needed - because we cannot remove the breakpoints in the inferior process - until after the `wait' in `wait_for_inferior'. */ -static int singlestep_breakpoints_inserted_p = 0; - -/* The thread we inserted single-step breakpoints for. */ -static ptid_t singlestep_ptid; - -/* PC when we started this single-step. */ -static CORE_ADDR singlestep_pc; - /* Info about an instruction that is being stepped over. */ struct step_over_info @@ -1896,9 +1886,6 @@ infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid) if (ptid_equal (inferior_ptid, old_ptid)) inferior_ptid = new_ptid; - if (ptid_equal (singlestep_ptid, old_ptid)) - singlestep_ptid = new_ptid; - for (displaced = displaced_step_inferior_states; displaced; displaced = displaced->next) @@ -1919,8 +1906,8 @@ infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid) static void resume_cleanups (void *ignore) { - if (single_step_breakpoints_inserted ()) - remove_single_step_breakpoints (); + if (!ptid_equal (inferior_ptid, null_ptid)) + delete_single_step_breakpoints (inferior_thread ()); normal_stop (); } @@ -1976,11 +1963,6 @@ maybe_software_singlestep (struct gdbarch *gdbarch, CORE_ADDR pc) && gdbarch_software_single_step (gdbarch, get_current_frame ())) { hw_step = 0; - /* Do not pull these breakpoints until after a `wait' in - `wait_for_inferior'. */ - singlestep_breakpoints_inserted_p = 1; - singlestep_ptid = inferior_ptid; - singlestep_pc = pc; } return hw_step; } @@ -2168,7 +2150,7 @@ a command like `return' or `jump' to continue execution.")); at the current address, deliver the signal without stepping, and once we arrive back at the step-resume breakpoint, actually step over the breakpoint we originally wanted to step over. */ - if (singlestep_breakpoints_inserted_p + if (thread_has_single_step_breakpoints_set (tp) && sig != GDB_SIGNAL_0 && step_over_info_valid_p ()) { @@ -2183,8 +2165,7 @@ a command like `return' or `jump' to continue execution.")); tp->step_after_step_resume_breakpoint = 1; } - remove_single_step_breakpoints (); - singlestep_breakpoints_inserted_p = 0; + delete_single_step_breakpoints (tp); clear_step_over_info (); tp->control.trap_expected = 0; @@ -2195,7 +2176,7 @@ a command like `return' or `jump' to continue execution.")); /* If STEP is set, it's a request to use hardware stepping facilities. But in that case, we should never use singlestep breakpoint. */ - gdb_assert (!(singlestep_breakpoints_inserted_p && step)); + gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step)); /* Decide the set of threads to ask the target to resume. Start by assuming everything will be resumed, than narrow the set @@ -2211,7 +2192,7 @@ a command like `return' or `jump' to continue execution.")); set_running (resume_ptid, 1); /* Maybe resume a single thread after all. */ - if ((step || singlestep_breakpoints_inserted_p) + if ((step || thread_has_single_step_breakpoints_set (tp)) && tp->control.trap_expected) { /* We're allowing a thread to run past a breakpoint it has @@ -2680,9 +2661,6 @@ init_wait_for_inferior (void) /* Discard any skipped inlined frames. */ clear_inline_frame_state (minus_one_ptid); - - singlestep_ptid = null_ptid; - singlestep_pc = 0; } @@ -2858,6 +2836,7 @@ delete_thread_infrun_breakpoints (struct thread_info *tp) { delete_step_resume_breakpoint (tp); delete_exception_resume_breakpoint (tp); + delete_single_step_breakpoints (tp); } /* If the target still has execution, call FUNC for each thread that @@ -2897,9 +2876,15 @@ static void delete_just_stopped_threads_infrun_breakpoints (void) { for_each_just_stopped_thread (delete_thread_infrun_breakpoints); +} + +/* Delete the single-step breakpoints of the threads that just + stopped. */ - if (single_step_breakpoints_inserted ()) - remove_single_step_breakpoints (); +static void +delete_just_stopped_threads_single_step_breakpoints (void) +{ + for_each_just_stopped_thread (delete_single_step_breakpoints); } /* A cleanup wrapper. */ @@ -3387,7 +3372,7 @@ adjust_pc_after_break (struct execution_control_state *ecs) software breakpoint. In this case (prev_pc == breakpoint_pc), we also need to back up to the breakpoint address. */ - if (singlestep_breakpoints_inserted_p + if (thread_has_single_step_breakpoints_set (ecs->event_thread) || !ptid_equal (ecs->ptid, inferior_ptid) || !currently_stepping (ecs->event_thread) || ecs->event_thread->prev_pc == breakpoint_pc) @@ -3773,8 +3758,6 @@ Cannot fill $_exitsignal with the correct signal number.\n")); gdb_flush (gdb_stdout); target_mourn_inferior (); - singlestep_breakpoints_inserted_p = 0; - cancel_single_step_breakpoints (); stop_print_frame = 0; stop_waiting (ecs); return; @@ -3867,12 +3850,7 @@ Cannot fill $_exitsignal with the correct signal number.\n")); detach_breakpoints (ecs->ws.value.related_pid); } - if (singlestep_breakpoints_inserted_p) - { - /* Pull the single step breakpoints out of the target. */ - remove_single_step_breakpoints (); - singlestep_breakpoints_inserted_p = 0; - } + delete_just_stopped_threads_single_step_breakpoints (); /* In case the event is caught by a catchpoint, remember that the event is to be followed at the next resume of the thread, @@ -3959,9 +3937,6 @@ Cannot fill $_exitsignal with the correct signal number.\n")); if (!ptid_equal (ecs->ptid, inferior_ptid)) context_switch (ecs->ptid); - singlestep_breakpoints_inserted_p = 0; - cancel_single_step_breakpoints (); - stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid)); /* Do whatever is necessary to the parent branch of the vfork. */ @@ -4027,14 +4002,7 @@ Cannot fill $_exitsignal with the correct signal number.\n")); fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_NO_HISTORY\n"); /* Reverse execution: target ran out of history info. */ - /* Pull the single step breakpoints out of the target. */ - if (singlestep_breakpoints_inserted_p) - { - if (!ptid_equal (ecs->ptid, inferior_ptid)) - context_switch (ecs->ptid); - remove_single_step_breakpoints (); - singlestep_breakpoints_inserted_p = 0; - } + delete_just_stopped_threads_single_step_breakpoints (); stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid)); observer_notify_no_history (); stop_waiting (ecs); @@ -4173,13 +4141,9 @@ handle_signal_stop (struct execution_control_state *ecs) gdbarch = get_frame_arch (frame); /* Pull the single step breakpoints out of the target. */ - if (singlestep_breakpoints_inserted_p) + if (gdbarch_software_single_step_p (gdbarch)) { - /* However, before doing so, if this single-step breakpoint was - actually for another thread, set this thread up for moving - past it. */ - if (!ptid_equal (ecs->ptid, singlestep_ptid) - && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP) + if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP) { struct regcache *regcache; struct address_space *aspace; @@ -4188,22 +4152,38 @@ handle_signal_stop (struct execution_control_state *ecs) regcache = get_thread_regcache (ecs->ptid); aspace = get_regcache_aspace (regcache); pc = regcache_read_pc (regcache); - if (single_step_breakpoint_inserted_here_p (aspace, pc)) + + /* However, before doing so, if this single-step breakpoint was + actually for another thread, set this thread up for moving + past it. */ + if (!thread_has_single_step_breakpoint_here (ecs->event_thread, + aspace, pc)) + { + if (single_step_breakpoint_inserted_here_p (aspace, pc)) + { + if (debug_infrun) + { + fprintf_unfiltered (gdb_stdlog, + "infrun: [%s] hit another thread's " + "single-step breakpoint\n", + target_pid_to_str (ecs->ptid)); + } + ecs->hit_singlestep_breakpoint = 1; + } + } + else { if (debug_infrun) { fprintf_unfiltered (gdb_stdlog, - "infrun: [%s] hit step over single-step" - " breakpoint of [%s]\n", - target_pid_to_str (ecs->ptid), - target_pid_to_str (singlestep_ptid)); + "infrun: [%s] hit its " + "single-step breakpoint\n", + target_pid_to_str (ecs->ptid)); } - ecs->hit_singlestep_breakpoint = 1; } } - remove_single_step_breakpoints (); - singlestep_breakpoints_inserted_p = 0; + delete_just_stopped_threads_single_step_breakpoints (); } if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP @@ -5543,10 +5523,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) insert_single_step_breakpoint (get_frame_arch (frame), get_frame_address_space (frame), stop_pc); - singlestep_breakpoints_inserted_p = 1; ecs->event_thread->control.trap_expected = 1; - singlestep_ptid = inferior_ptid; - singlestep_pc = stop_pc; resume (0, GDB_SIGNAL_0); prepare_to_wait (ecs); diff --git a/gdb/record-full.c b/gdb/record-full.c index 119361f..a2257ed 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -961,7 +961,7 @@ record_full_resume (struct target_ops *ops, ptid_t ptid, int step, else { /* This arch support soft sigle step. */ - if (single_step_breakpoints_inserted ()) + if (thread_has_single_step_breakpoints_set (inferior_thread ())) { /* This is a soft single step. */ record_full_resume_step = 1; @@ -1085,6 +1085,8 @@ record_full_wait_1 (struct target_ops *ops, while (1) { + struct thread_info *tp; + ret = ops->beneath->to_wait (ops->beneath, ptid, status, options); if (status->kind == TARGET_WAITKIND_IGNORE) { @@ -1095,8 +1097,8 @@ record_full_wait_1 (struct target_ops *ops, return ret; } - if (single_step_breakpoints_inserted ()) - remove_single_step_breakpoints (); + ALL_NON_EXITED_THREADS (tp) + delete_single_step_breakpoints (tp); if (record_full_resume_step) return ret; diff --git a/gdb/thread.c b/gdb/thread.c index ed7bcfa..2446978 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -118,6 +118,15 @@ delete_exception_resume_breakpoint (struct thread_info *tp) delete_thread_breakpoint (&tp->control.exception_resume_breakpoint); } +/* See gdbthread.h. */ + +void +delete_single_step_breakpoints (struct thread_info *tp) +{ + if (tp != NULL) + delete_thread_breakpoint (&tp->control.single_step_breakpoints); +} + /* Delete the breakpoint pointed at by BP_P at the next stop, if there's one. */ @@ -131,6 +140,27 @@ delete_at_next_stop (struct breakpoint **bp) } } +/* See gdbthread.h. */ + +int +thread_has_single_step_breakpoints_set (struct thread_info *tp) +{ + return tp->control.single_step_breakpoints != NULL; +} + +/* See gdbthread.h. */ + +int +thread_has_single_step_breakpoint_here (struct thread_info *tp, + struct address_space *aspace, + CORE_ADDR addr) +{ + struct breakpoint *ss_bps = tp->control.single_step_breakpoints; + + return (ss_bps != NULL + && breakpoint_has_location_inserted_here (ss_bps, aspace, addr)); +} + static void clear_thread_inferior_resources (struct thread_info *tp) { @@ -140,6 +170,7 @@ clear_thread_inferior_resources (struct thread_info *tp) be stopped at the moment. */ delete_at_next_stop (&tp->control.step_resume_breakpoint); delete_at_next_stop (&tp->control.exception_resume_breakpoint); + delete_at_next_stop (&tp->control.single_step_breakpoints); delete_longjmp_breakpoint_at_next_stop (tp->num);