Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 09/23] switch inferior/thread before calling target methods
Date: Fri, 06 Sep 2019 23:28:00 -0000	[thread overview]
Message-ID: <20190906232807.6191-10-palves@redhat.com> (raw)
In-Reply-To: <20190906232807.6191-1-palves@redhat.com>

Once each inferior has its own target stack, we'll need to make sure
that the right inferior is selected before we call into target
methods.

It kind of sounds worse than it is in practice.  Not that many places
need to be concerned.

In thread.c, we add a new switch_to_thread_if_alive function that
centralizes the switching before calls to target_thread_alive.  Other
cases are handled with explicit switching.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdbthread.h (scoped_restore_current_thread)
	<dont_restore, restore, m_dont_restore>: Declare.
	* thread.c (thread_alive): Add assertion.  Return bool.
	(switch_to_thread_if_alive): New.
	(prune_threads): Switch inferior/thread.
	(print_thread_info_1): Switch thread before calling target methods.
	(scoped_restore_current_thread::restore): New, factored out from
	...
	(scoped_restore_current_thread::~scoped_restore_current_thread):
	... this.
	(scoped_restore_current_thread::scoped_restore_current_thread):
	Add assertion.
	(thread_apply_all_command, thread_select): Use
	switch_to_thread_if_alive.
	* infrun.c (proceed, restart_threads, handle_signal_stop)
	(switch_back_to_stepped_thread): Switch current thread before
	calling target methods.
---
 gdb/infrun.c | 16 ++++++++++++--
 gdb/thread.c | 72 +++++++++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 66 insertions(+), 22 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9c888aa72f..61c99e36c4 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2949,6 +2949,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
     {
       for (thread_info *tp : all_non_exited_threads (resume_ptid))
 	{
+	  switch_to_thread_no_regs (tp);
+
 	  /* Ignore the current thread here.  It's handled
 	     afterwards.  */
 	  if (tp == cur_thr)
@@ -2966,6 +2968,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
 	  thread_step_over_chain_enqueue (tp);
 	}
+
+      switch_to_thread (cur_thr);
     }
 
   /* Enqueue the current thread last, so that we move all other
@@ -3002,6 +3006,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 	   Start all other threads that are implicitly resumed too.  */
       for (thread_info *tp : all_non_exited_threads (resume_ptid))
         {
+	  switch_to_thread_no_regs (tp);
+
 	  if (tp->resumed)
 	    {
 	      if (debug_infrun)
@@ -4347,6 +4353,7 @@ stop_all_threads (void)
 					    "infrun:   %s executing, "
 					    "need stop\n",
 					    target_pid_to_str (t->ptid).c_str ());
+		      switch_to_thread_no_regs (t);
 		      target_stop (t->ptid);
 		      t->stop_requested = 1;
 		    }
@@ -5191,6 +5198,8 @@ restart_threads (struct thread_info *event_thread)
 
   for (thread_info *tp : all_non_exited_threads ())
     {
+      switch_to_thread_no_regs (tp);
+
       if (tp == event_thread)
 	{
 	  if (debug_infrun)
@@ -5449,9 +5458,8 @@ handle_signal_stop (struct execution_control_state *ecs)
     {
       struct regcache *regcache = get_thread_regcache (ecs->event_thread);
       struct gdbarch *reg_gdbarch = regcache->arch ();
-      scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
 
-      inferior_ptid = ecs->ptid;
+      switch_to_thread (ecs->event_thread);
 
       fprintf_unfiltered (gdb_stdlog, "infrun: stop_pc = %s\n",
 			  paddress (reg_gdbarch,
@@ -6885,6 +6893,8 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 
       for (thread_info *tp : all_non_exited_threads ())
         {
+	  switch_to_thread_no_regs (tp);
+
 	  /* Ignore threads of processes the caller is not
 	     resuming.  */
 	  if (!sched_multi
@@ -6936,6 +6946,8 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 	      return 1;
 	    }
 	}
+
+      switch_to_thread (ecs->event_thread);
     }
 
   return 0;
diff --git a/gdb/thread.c b/gdb/thread.c
index f79507716b..9a5867306b 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -62,8 +62,6 @@ static int highest_thread_num;
    spawned new threads we haven't heard of yet.  */
 static int threads_executing;
 
-static int thread_alive (struct thread_info *);
-
 /* RAII type used to increase / decrease the refcount of each thread
    in a given list of threads.  */
 
@@ -679,14 +677,38 @@ any_live_thread_of_inferior (inferior *inf)
 }
 
 /* Return true if TP is an active thread.  */
-static int
-thread_alive (struct thread_info *tp)
+static bool
+thread_alive (thread_info *tp)
 {
   if (tp->state == THREAD_EXITED)
-    return 0;
-  if (!target_thread_alive (tp->ptid))
-    return 0;
-  return 1;
+    return false;
+
+  /* Ensure we're looking at the right target stack.  */
+  gdb_assert (tp->inf == current_inferior ());
+
+  return target_thread_alive (tp->ptid);
+}
+
+/* Switch to thread TP if it is alive.  Returns true if successfully
+   switched, false otherwise.  */
+
+static bool
+switch_to_thread_if_alive (thread_info *thr)
+{
+  scoped_restore_current_thread restore_thread;
+
+  /* Switch inferior first, so that we're looking at the right target
+     stack.  */
+  switch_to_inferior_no_thread (thr->inf);
+
+  if (thread_alive (thr))
+    {
+      switch_to_thread (thr);
+      restore_thread.dont_restore ();
+      return true;
+    }
+
+  return false;
 }
 
 /* See gdbthreads.h.  */
@@ -694,9 +716,15 @@ thread_alive (struct thread_info *tp)
 void
 prune_threads (void)
 {
+  scoped_restore_current_thread restore_thread;
+
   for (thread_info *tp : all_threads_safe ())
-    if (!thread_alive (tp))
-      delete_thread (tp);
+    {
+      switch_to_inferior_no_thread (tp->inf);
+
+      if (!thread_alive (tp))
+	delete_thread (tp);
+    }
 }
 
 /* See gdbthreads.h.  */
@@ -1037,6 +1065,9 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
     gdb::optional<ui_out_emit_list> list_emitter;
     gdb::optional<ui_out_emit_table> table_emitter;
 
+    /* We'll be switching threads temporarily below.  */
+    scoped_restore_current_thread restore_thread;
+
     if (uiout->is_mi_like_p ())
       list_emitter.emplace (uiout, "threads");
     else
@@ -1054,6 +1085,10 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
 
 	    if (!uiout->is_mi_like_p ())
 	      {
+		/* Switch inferiors so we're looking at the right
+		   target stack.  */
+		switch_to_inferior_no_thread (tp->inf);
+
 		target_id_col_width
 		  = std::max (target_id_col_width,
 			      thread_target_id_str (tp).size ());
@@ -1085,9 +1120,6 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
 	uiout->table_body ();
       }
 
-    /* We'll be switching threads temporarily.  */
-    scoped_restore_current_thread restore_thread;
-
     for (inferior *inf : all_inferiors ())
       for (thread_info *tp : inf->threads ())
       {
@@ -1116,6 +1148,9 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
 	if (show_global_ids || uiout->is_mi_like_p ())
 	  uiout->field_signed ("id", tp->global_num);
 
+	/* Switch to the thread (and inferior / target).  */
+	switch_to_thread (tp);
+
 	/* For the CLI, we stuff everything into the target-id field.
 	   This is a gross hack to make the output come out looking
 	   correct.  The underlying problem here is that ui-out has no
@@ -1147,9 +1182,8 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
 	  uiout->text ("(running)\n");
 	else
 	  {
-	    /* The switch below puts us at the top of the stack (leaf
+	    /* The switch above put us at the top of the stack (leaf
 	       frame).  */
-	    switch_to_thread (tp);
 	    print_stack_frame (get_selected_frame (NULL),
 			       /* For MI output, print frame level.  */
 			       uiout->is_mi_like_p (),
@@ -1662,7 +1696,7 @@ thread_apply_all_command (const char *cmd, int from_tty)
       scoped_restore_current_thread restore_thread;
 
       for (thread_info *thr : thr_list_cpy)
-	if (thread_alive (thr))
+	if (switch_to_thread_if_alive (thr))
 	  thr_try_catch_cmd (thr, cmd, from_tty, flags);
     }
 }
@@ -1819,7 +1853,7 @@ thread_apply_command (const char *tidlist, int from_tty)
 	  continue;
 	}
 
-      if (!thread_alive (tp))
+      if (!switch_to_thread_if_alive (tp))
 	{
 	  warning (_("Thread %s has terminated."), print_thread_id (tp));
 	  continue;
@@ -1983,11 +2017,9 @@ show_print_thread_events (struct ui_file *file, int from_tty,
 void
 thread_select (const char *tidstr, thread_info *tp)
 {
-  if (!thread_alive (tp))
+  if (!switch_to_thread_if_alive (tp))
     error (_("Thread ID %s has terminated."), tidstr);
 
-  switch_to_thread (tp);
-
   annotate_thread_changed ();
 
   /* Since the current thread may have changed, see if there is any
-- 
2.14.5


  parent reply	other threads:[~2019-09-06 23:28 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-06 23:28 [PATCH 00/23] Multi-target support Pedro Alves
2019-09-06 23:28 ` [PATCH 02/23] Don't rely on inferior_ptid in record_full_wait Pedro Alves
2020-07-31  3:17   ` Tom Tromey
2020-08-01 16:14     ` Simon Marchi
2020-08-01 19:32       ` John Baldwin
2020-08-01 20:47         ` Tom Tromey
2020-08-01 20:46       ` Tom Tromey
2020-08-01 22:56         ` Simon Marchi
2020-08-02 17:52           ` Tom Tromey
2020-08-03  0:08             ` Simon Marchi
2019-09-06 23:28 ` [PATCH 13/23] Delete exit_inferior_silent(int pid) Pedro Alves
2019-09-06 23:28 ` [PATCH 15/23] Fix reconnecting to a gdbserver already debugging multiple processes, I Pedro Alves
2019-09-06 23:28 ` [PATCH 10/23] Some get_last_target_status tweaks Pedro Alves
2019-09-09 18:53   ` Tom Tromey
2019-10-17  1:14     ` Pedro Alves
2019-09-06 23:28 ` [PATCH 01/23] Preserve selected thread in all-stop w/ background execution Pedro Alves
2019-10-09  9:36   ` Aktemur, Tankut Baris
2019-10-16 23:54     ` [PATCH v1.1 " Pedro Alves
2019-10-17 10:21       ` Aktemur, Tankut Baris
2019-09-06 23:28 ` [PATCH 06/23] Don't check target is running in remote_target::mourn_inferior Pedro Alves
2019-09-06 23:28 ` [PATCH 16/23] Fix reconnecting to a gdbserver already debugging multiple processes, II Pedro Alves
2019-09-06 23:28 ` [PATCH 03/23] Make "show remote exec-file" inferior-aware Pedro Alves
2019-09-06 23:28 ` [PATCH 19/23] gdbarch-selftests.c: No longer error out if debugging something Pedro Alves
2019-09-06 23:28 ` [PATCH 20/23] Revert 'Remove unused struct serial::name field' Pedro Alves
2019-09-06 23:47   ` Christian Biesinger via gdb-patches
2019-09-08 19:30     ` Pedro Alves
2019-09-06 23:28 ` Pedro Alves [this message]
2019-09-06 23:28 ` [PATCH 08/23] Introduce switch_to_inferior_no_thread Pedro Alves
2019-09-09 18:42   ` Tom Tromey
2019-10-17  1:07     ` Pedro Alves
2019-09-06 23:28 ` [PATCH 11/23] tfile_target::close: trace_fd can't be -1 Pedro Alves
2019-09-06 23:28 ` [PATCH 18/23] Add multi-target tests Pedro Alves
2019-10-09 16:01   ` Aktemur, Tankut Baris
2019-10-17  0:55     ` Pedro Alves
2019-09-06 23:28 ` [PATCH 17/23] Multi-target support Pedro Alves
2019-09-11 17:11   ` Tom Tromey
2019-10-17  1:54     ` Pedro Alves
2019-09-06 23:33 ` [PATCH 23/23] Multi-target: NEWS and user manual Pedro Alves
2019-09-07  6:33   ` Eli Zaretskii
2019-10-17  2:08     ` Pedro Alves
2019-10-17  7:55       ` Eli Zaretskii
2019-10-17  2:42     ` Pedro Alves
2019-10-17  8:14       ` Eli Zaretskii
2019-10-17 15:31         ` Pedro Alves
2019-09-06 23:34 ` [PATCH 04/23] exceptions.c:print_flush: Remove obsolete check Pedro Alves
2019-09-09 18:07   ` Tom Tromey
2019-09-06 23:35 ` [PATCH 05/23] Make target_ops::has_execution take an 'inferior *' instead of a ptid_t Pedro Alves
2019-09-09 18:12   ` Tom Tromey
2019-09-06 23:36 ` [PATCH 14/23] Tweak handling of remote errors in response to resumption packet Pedro Alves
2019-10-09 13:35   ` Aktemur, Tankut Baris
2019-10-17  0:54     ` [PATCH 14.5/23] Avoid another inferior_ptid reference in gdb/remote.c (Re: [PATCH 14/23] Tweak handling of remote errors in response to resumption packet) Pedro Alves
2019-09-06 23:36 ` [PATCH 12/23] Use all_non_exited_inferiors in infrun.c Pedro Alves
2019-09-06 23:36 ` [PATCH 07/23] Delete unnecessary code from kill_command Pedro Alves
2019-10-01 10:19   ` Aktemur, Tankut Baris
2019-10-01 13:28     ` Aktemur, Tankut Baris
2019-09-06 23:37 ` [PATCH 22/23] Require always-non-stop for multi-target resumptions Pedro Alves
2019-09-06 23:37 ` [PATCH 21/23] Add "info connections" command, "info inferiors" connection number/string Pedro Alves
2019-09-09 20:18   ` Tom Tromey
2019-10-17  2:21     ` Pedro Alves
2019-10-17 14:23       ` Tom Tromey
2019-09-07 11:19 ` [PATCH 00/23] Multi-target support Philippe Waroquiers
2019-09-08 20:06   ` Pedro Alves
2019-09-08 20:50     ` Philippe Waroquiers
2019-10-16 19:08       ` Pedro Alves
2019-10-16 19:14       ` [PATCH] Avoid inferior_ptid reference in gdb/remote.c (Re: [PATCH 00/23] Multi-target support) Pedro Alves
2019-09-09 19:09 ` [PATCH 00/23] Multi-target support Tom Tromey
2019-09-09 20:22 ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190906232807.6191-10-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox