From patchwork Tue Mar 26 14:44:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 31989 Received: (qmail 81534 invoked by alias); 26 Mar 2019 14:44:14 -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 81330 invoked by uid 89); 26 Mar 2019 14:44:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=locked, resume, destruction, sk:clear_s X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 26 Mar 2019 14:44:11 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 797C1116C7A; Tue, 26 Mar 2019 10:44:09 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id OG5jKpyK3GeP; Tue, 26 Mar 2019 10:44:09 -0400 (EDT) Received: from murgatroyd.Home (174-29-37-56.hlrn.qwest.net [174.29.37.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id 244EF116C5F; Tue, 26 Mar 2019 10:44:09 -0400 (EDT) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA 7/8] Avoid crashes when stepping through ravenscar context-switching Date: Tue, 26 Mar 2019 08:44:03 -0600 Message-Id: <20190326144404.6670-8-tromey@adacore.com> In-Reply-To: <20190326144404.6670-1-tromey@adacore.com> References: <20190326144404.6670-1-tromey@adacore.com> MIME-Version: 1.0 When stepping through the ravenscar context-switching code, gdb will try to single-step the current thread, while leaving the other threads stopped. This results in an assertion failure in finish_step_over, because the event thread is not the request thread. This patch introduces a new target method so that infrun can ask the target whether this can happen; and then changes infrun to handle this case. When the target can have an unexpected thread switch, single-stepping may also switch threads. gdb/ChangeLog 2019-03-26 Tom Tromey * infrun.c (finish_step_over): Check target_can_randomly_thread_switch. (handle_signal_stop): Handle random thread switches. * ravenscar-thread.c (struct ravenscar_thread_target) : New method. * target-delegates.c: Rebuild. * target.h (struct target_ops) : New method. (target_can_randomly_thread_switch): New define. --- gdb/ChangeLog | 12 ++++++++++++ gdb/infrun.c | 39 ++++++++++++++++++++++++++++++++------- gdb/ravenscar-thread.c | 5 +++++ gdb/target-delegates.c | 27 +++++++++++++++++++++++++++ gdb/target.h | 12 ++++++++++++ 5 files changed, 88 insertions(+), 7 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index ad7892105a4..bfc909415c5 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -5328,7 +5328,8 @@ finish_step_over (struct execution_control_state *ecs) /* If we're stepping over a breakpoint with all threads locked, then only the thread that was stepped should be reporting back an event. */ - gdb_assert (ecs->event_thread->control.trap_expected); + gdb_assert (ecs->event_thread->control.trap_expected + || target_can_randomly_thread_switch ()); clear_step_over_info (); } @@ -5567,14 +5568,38 @@ handle_signal_stop (struct execution_control_state *ecs) { if (single_step_breakpoint_inserted_here_p (aspace, pc)) { - if (debug_infrun) + if (target_can_randomly_thread_switch ()) { - fprintf_unfiltered (gdb_stdlog, - "infrun: [%s] hit another thread's " - "single-step breakpoint\n", - target_pid_to_str (ecs->ptid).c_str ()); + if (debug_infrun) + fprintf_unfiltered (gdb_stdlog, + "infrun: [%s] random thread switch\n", + (target_pid_to_str + (ecs->ptid).c_str ())); + + /* Here the step was intended for some other thread; + but instead we saw a context switch. This can + happen with "green" threads in Ravenscar, and the + recourse is to pretend we wanted to step the new + thread and got an ordinary stop there. */ + for (thread_info *th : all_threads ()) + if (th->control.trap_expected) + { + std::swap (th->control, ecs->event_thread->control); + break; + } + } + else + { + if (debug_infrun) + { + fprintf_unfiltered (gdb_stdlog, + "infrun: [%s] hit another thread's " + "single-step breakpoint\n", + (target_pid_to_str + (ecs->ptid).c_str ())); + } + ecs->hit_singlestep_breakpoint = 1; } - ecs->hit_singlestep_breakpoint = 1; } } else diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c index f36ee3ea240..4e35026e6e2 100644 --- a/gdb/ravenscar-thread.c +++ b/gdb/ravenscar-thread.c @@ -89,6 +89,11 @@ struct ravenscar_thread_target final : public target_ops ptid_t wait (ptid_t, struct target_waitstatus *, int) override; void resume (ptid_t, int, enum gdb_signal) override; + bool can_randomly_thread_switch () override + { + return true; + } + void fetch_registers (struct regcache *, int) override; void store_registers (struct regcache *, int) override; diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 3654f02e63b..8c0d135d010 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -19,6 +19,7 @@ struct dummy_target : public target_ops void fetch_registers (struct regcache *arg0, int arg1) override; void store_registers (struct regcache *arg0, int arg1) override; void prepare_to_store (struct regcache *arg0) override; + bool can_randomly_thread_switch () override; void files_info () override; int insert_breakpoint (struct gdbarch *arg0, struct bp_target_info *arg1) override; int remove_breakpoint (struct gdbarch *arg0, struct bp_target_info *arg1, enum remove_bp_reason arg2) override; @@ -186,6 +187,7 @@ struct debug_target : public target_ops void fetch_registers (struct regcache *arg0, int arg1) override; void store_registers (struct regcache *arg0, int arg1) override; void prepare_to_store (struct regcache *arg0) override; + bool can_randomly_thread_switch () override; void files_info () override; int insert_breakpoint (struct gdbarch *arg0, struct bp_target_info *arg1) override; int remove_breakpoint (struct gdbarch *arg0, struct bp_target_info *arg1, enum remove_bp_reason arg2) override; @@ -551,6 +553,31 @@ debug_target::prepare_to_store (struct regcache *arg0) fputs_unfiltered (")\n", gdb_stdlog); } +bool +target_ops::can_randomly_thread_switch () +{ + return this->beneath ()->can_randomly_thread_switch (); +} + +bool +dummy_target::can_randomly_thread_switch () +{ + return false; +} + +bool +debug_target::can_randomly_thread_switch () +{ + bool result; + fprintf_unfiltered (gdb_stdlog, "-> %s->can_randomly_thread_switch (...)\n", this->beneath ()->shortname ()); + result = this->beneath ()->can_randomly_thread_switch (); + fprintf_unfiltered (gdb_stdlog, "<- %s->can_randomly_thread_switch (", this->beneath ()->shortname ()); + fputs_unfiltered (") = ", gdb_stdlog); + target_debug_print_bool (result); + fputs_unfiltered ("\n", gdb_stdlog); + return result; +} + void target_ops::files_info () { diff --git a/gdb/target.h b/gdb/target.h index 4f7a43e1a8b..ad3a8be4c18 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -491,6 +491,13 @@ struct target_ops virtual void prepare_to_store (struct regcache *) TARGET_DEFAULT_NORETURN (noprocess ()); + /* True if single-stepping on this target can cause an + un-asked-for thread switch. This is normal on "green" thread + targets, like Ravenscar, when single-stepping through the + context-switching code. */ + virtual bool can_randomly_thread_switch () + TARGET_DEFAULT_RETURN (false); + virtual void files_info () TARGET_DEFAULT_IGNORE (); virtual int insert_breakpoint (struct gdbarch *, @@ -1402,6 +1409,11 @@ extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal); coalesce multiple resumption requests in a single vCont packet. */ extern void target_commit_resume (); +/* A wrapper for target_ops::can_randomly_thread_switch. */ + +#define target_can_randomly_thread_switch() \ + (current_top_target ()->can_randomly_thread_switch) () + /* Setup to defer target_commit_resume calls, and reactivate target_commit_resume on destruction, if it was previously active. */