Re: [PATCH 17/23] Multi-target support

[Pedro Alves <palves@redhat.com>]

On 9/11/19 6:11 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> This commit adds multi-target support to GDB.  What this means is that
> Pedro> with this commit, GDB can now be connected to different targets at the
> Pedro> same time.  E.g., you can debug a live native process and a core dump
> Pedro> at the same time, connect to multiple gdbservers, etc.
> 
> Awesome!
> 
> I read through the patch, but I can't really claim to understand how all
> the parts interrelate.  However your overview made sense to me, and I
> don't have any conceptual concerns; just the usual sort of thing that
> there are lurking bugs.  I guess my main fear is that this will
> introduce regressions... but on the other hand, I think it's important
> direction for gdb, so I'd rather err on the side of moving forward with
> it.
> 
> Pedro> To fix this,
> Pedro> this commit renames gdbserver's target_ops to process_stratum_target.
> Pedro> I think this makes sense.  There's no concept of target stack in
> Pedro> gdbserver, and gdbserver's target_ops really implements a
> Pedro> process_stratum-like target.
> 
> Makes sense.  Sometimes I dream about re-merging the target stacks,
> further shrinking the size of the gdbserver fork.

Yeah, that's been a long term goal of mine too.

> 
> Pedro>  - you can only resume more that one target at the same time if all
> Pedro>    targets support asynchronous debugging, and support non-stop mode.
> Pedro>    It should be possible to support mixed all-stop + non-stop
> Pedro>    backends, but that is left for another time.  This means that
> Pedro>    currently in order to do multi-target with gdbserver you need to
> Pedro>    issue "maint set target-non-stop on".  I would like to make that
> Pedro>    mode be the default, but we're not there yet.  Note that I'm
> Pedro>    talking about how the target backend works, only.  User-visible
> Pedro>    all-stop mode works just fine.
> 
> Pedro>  - as explained above, connecting to different remote servers at the
> Pedro>    same time is likely to produce bad results if they don't support the
> Pedro>    exact set of RSP features.
> 
> Are these limitations something that can be noticed when making the
> remote connection, or do they require the user to be careful?  What
> happens if the user forgets or just doesn't know?

I'm not seeing how to detect that easily&gracefully, unfortunately.  If the
user forgets, gdb might misbehave.  I'm hoping that the limitation
will be temporary, though.

> Pedro> -  thread_info *tp = find_thread_ptid (task_info->ptid);
> Pedro> +  thread_info *tp = find_thread_ptid (inf->process_target (), task_info->ptid);
> 
> There are a few spots in the patch that could use the overload that
> accepts an inferior, but instead call the process_target method
> directly.

Thanks.  I fixed that one.  I skimmed other calls and didn't
see any that was obvious it could be changed.

> 
> Pedro> +/* Calls target_commit_resume on all targets.  */
> Pedro> +
> Pedro> +static void
> Pedro> +commit_resume_all_targets ()
> Pedro> +{
> Pedro> +  scoped_restore_current_thread restore_thread;
> Pedro> +
> Pedro> +  for (inferior *inf : all_non_exited_inferiors ())
> Pedro> +    if (inf->has_execution ())
> Pedro> +      {
> Pedro> +	switch_to_inferior_no_thread (inf);
> Pedro> +	target_commit_resume ();
> Pedro> +      }
> 
> IIUC this can cause multiple calls to a given target's commit_resume
> method.  That seems fine (assuming you audited these implementations)
> but I suppose it would be good to document this somewhere.
> 
> Alternatively I guess gdb would need some kind of iterator that ensures
> it only visits each target once.

You're right.  I didn't worry about it because resumptions are
supposed to be idempotent, but, reducing number of redundant
vCont packets can only be a good thing.  I did not add an iterator,
instead fixed it locally.  Thinking that we may end up changing
the data inferior/process_stratum_target structures to include


> 
> Pedro> -	  event_ptid = wait_one (&ws);
> Pedro> +	  wait_one_event event = wait_one ();
> Pedro> +	  target_waitstatus &ws = event.ws;
> Pedro> +	  ptid_t &event_ptid = event.ptid;
> 
> I was wondering if these need to be references.  It seemed like maybe
> they could be const references but I couldn't immediately tell.
> 

Good point.  They can, if I constify save_waitstatus as well:

  static void
 -save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
 +save_waitstatus (struct thread_info *tp, const target_waitstatus *ws)

But I decided to just get rid of them and adjust the code to refer
to event.ws / event.ptid directly.

> Pedro> -     For all-stop targets, we only step INFERIOR_PTID and continue others.  */
> Pedro> +      For all-stop targets, we only step INFERIOR_PTID and continue others.  */
> 
> This looks like extra indentation was added.

Thanks.

> 
> Pedro>  struct regcache *
> Pedro>  get_thread_regcache_for_ptid (ptid_t ptid)
> Pedro>  {
> Pedro> -  return get_thread_regcache (ptid);
> Pedro> +  /* This function doesn't take a process_stratum_target parameter
> Pedro> +     because it's a common/ routine implemented by both gdb and
> Pedro> +     gdbserver.  It always refers to a ptid of the current target. */
> 
> It's "gdbsupport/" now.

Indeed.  :-)  Fixed, and also fixed double space at the end.

> 
> Pedro>    /* The status of the stub support for the various vCont actions.  */
> Pedro>    vCont_action_support supports_vCont;
> Pedro> +  /* Whether vCont support was probed already.  */
> Pedro> +  bool supports_vCont_probed;
> 
> If it's the case that this is only a temporary measure, then I think
> this comment should mention that.

Fixed.

> 
> Pedro> @@ -6601,6 +6622,8 @@ remote_target::commit_resume ()
> Pedro>      }
>  
> Pedro>    vcont_builder.flush ();
> Pedro> +
> Pedro> +  target_async (1);
> Pedro>  }
> 
> I didn't understand this.  Like, is it always ok to do this?

I had to back this one out to remember why it was needed.
It is simply that we now loop over various targets, and nothing
is calling target_async on each of them to register them
on the event loop.  I've moved this to common code now
(in do_target_resume), where I think it's more obvious.

> 
> Pedro>  /* Callback for iterate_over_inferiors.  Gets rid of the given
> Pedro>     inferior.  */
>  
> Pedro> -static int
> Pedro> -dispose_inferior (struct inferior *inf, void *args)
> Pedro> +static void
> Pedro> +dispose_inferior (inferior *inf)
> Pedro>  {
> Pedro>    /* Not all killed inferiors can, or will ever be, removed from the
> Pedro>       inferior list.  Killed inferiors clearly don't need to be killed
> Pedro>       again, so, we're done.  */
> Pedro>    if (inf->pid == 0)
> Pedro> -    return 0;
> Pedro> +    return;
> 
> I think the comments here (both the intro comment and the one in the
> function) need to be updated, since it seems that just a single inferior
> is handled here now, and this is no longer a callback for
> iterate_over_inferiors.

Indeed.  I think it's better to just delete the function an inline
its body in the caller.

> 
> I didn't understand this change.  Why did it used to iterate but now
> does not?

So before there was only ever one target stack.  If you were e.g., connected
to the native target, and did "tar rem :9999", then we'd need to discard all
the native inferiors that were debugging.  Since now each inferior has its
own connection, we only discard the current inferior, all other inferiors
keep using their current connections.

> 
> Pedro>  target_pass_ctrlc (void)
> Pedro>  {
> Pedro> -  current_top_target ()->pass_ctrlc ();
> Pedro> +  /* Pass the Ctrl-C to the first inferior that has a thread
> Pedro> +     running.  */
> Pedro> +  for (inferior *inf : all_inferiors ())
> Pedro> +    {
> [...]
> Pedro> +	      return;
> 
> I didn't understand this.  It seemed to me that a C-c should maybe be
> sent to all running inferiors?

This is like what the kernel does -- The C-c is sent to one process
(the one in the foreground) and then when gdb intercepts the SIGINT, it
pauses all processes/threads.  If we sent SIGINT to all running inferiors,
then we'd end up presenting one SIGINT stop to the user for each
inferior.  Like:

* running *
Press C-c
Thread 1.1 stopped with SIGINT
(gdb) c
Thread 2.1 stopped with SIGINT
(gdb) c
Thread 3.1 stopped with SIGINT
(gdb) c
Thread 4.1 stopped with SIGINT
(gdb) c
* all running again *

The comment has a typo, which I fixed now.  It should read:

  /* Pass the Ctrl-C to the first target that has a thread
     running.  */

It doesn't matter which inferior is selected for a target,
as target_pass_ctrlc forwards the Ctrl-C to the process
in the foreground.

> 
> I don't actually know this area.  Maybe that's obvious now :)
> 
> Pedro>  /* See target.h.  */
> Pedro> @@ -3987,10 +4047,8 @@ set_write_memory_permission (const char *args, int from_tty,
> Pedro>  }
>  
> Pedro>  void
> Pedro> -initialize_targets (void)
> Pedro> +_initialize_target (void)
> 
> You might as well remove the 'void' when touching the line.

Done.

> 
> Pedro> -  explicit all_matching_threads_iterator (ptid_t filter_ptid);
> Pedro> +  explicit all_matching_threads_iterator (process_stratum_target *filter_target,
> Pedro> +					  ptid_t filter_ptid);
> 
> I guess this could drop the "explicit".

Indeed, done.

> 
> Pedro> -  explicit all_matching_threads_range (ptid_t filter_ptid)
> Pedro> -    : m_filter_ptid (filter_ptid)
> Pedro> +  explicit all_matching_threads_range (process_stratum_target *filter_target,
> Pedro> +				       ptid_t filter_ptid)
> 
> Here too.
> 
> Pedro> -  explicit all_non_exited_threads_range (ptid_t filter_ptid)
> Pedro> -    : m_filter_ptid (filter_ptid)
> Pedro> +  explicit all_non_exited_threads_range (process_stratum_target *filter_target,
> Pedro> +					 ptid_t filter_ptid)
> Pedro> +    : m_filter_target (filter_target), m_filter_ptid (filter_ptid)
> 
> And here.

All fixed.

Here's the patch that I'm folding into the main patch, FYI.

I've also went through a number of native targets and fixed/adjusted
them AIX, Solaris, Windows, etc.

From 53aacc25ea7e3856588e9ea9fe47a2e22bbedaf4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 17 Oct 2019 02:48:50 +0100
Subject: [PATCH] Address Tromey's review & more

---
 gdb/ada-tasks.c           |  2 +-
 gdb/aix-thread.c          | 24 ++++++++++++++------
 gdb/gdbserver/lynx-low.c  |  2 +-
 gdb/gdbserver/nto-low.c   |  2 +-
 gdb/gdbserver/target.h    |  2 +-
 gdb/gdbserver/win32-low.c |  4 ++--
 gdb/inferior-iter.h       |  3 +--
 gdb/infrun.c              | 56 ++++++++++++++++++++++++++++-------------------
 gdb/nto-procfs.c          |  2 +-
 gdb/procfs.c              | 49 ++++++++++++++++++++++-------------------
 gdb/ravenscar-thread.c    |  5 ++---
 gdb/record-btrace.c       |  2 +-
 gdb/regcache.c            |  4 ++--
 gdb/remote.c              |  5 ++---
 gdb/sol-thread.c          | 28 +++++++++++++++++-------
 gdb/target.c              | 36 ++++++++----------------------
 gdb/thread-iter.h         | 12 +++++-----
 gdb/windows-nat.c         | 20 +++++++++--------
 18 files changed, 138 insertions(+), 120 deletions(-)

diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 0c85b0f7fa..c43369c662 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -1340,7 +1340,7 @@ task_command_1 (const char *taskno_str, int from_tty, struct inferior *inf)
      computed if target_get_ada_task_ptid has not been implemented for
      our target (yet).  Rather than cause an assertion error in that case,
      it's nicer for the user to just refuse to perform the task switch.  */
-  thread_info *tp = find_thread_ptid (inf->process_target (), task_info->ptid);
+  thread_info *tp = find_thread_ptid (inf, task_info->ptid);
   if (tp == NULL)
     error (_("Unable to compute thread ID for task %s.\n"
              "Cannot switch to this task."),
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index ffa3352d03..ec545221fb 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -805,7 +805,11 @@ sync_threadlists (void)
 	  priv->pdtid = pbuf[pi].pdtid;
 	  priv->tid = pbuf[pi].tid;
 
-	  thread = add_thread_with_info (ptid_t (infpid, 0, pbuf[pi].pthid), priv);
+	  process_stratum_target *proc_target
+	    = current_inferior ()->process_target ();
+	  thread = add_thread_with_info (proc_target,
+					 ptid_t (infpid, 0, pbuf[pi].pthid),
+					 priv);
 
 	  pi++;
 	}
@@ -837,7 +841,9 @@ sync_threadlists (void)
 	    }
 	  else
 	    {
-	      thread = add_thread (pptid);
+	      process_stratum_target *proc_target
+		= current_inferior ()->process_target ();
+	      thread = add_thread (proc_target, pptid);
 
 	      aix_thread_info *priv = new aix_thread_info;
 	      thread->priv.reset (priv);
@@ -1043,7 +1049,7 @@ aix_thread_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
     }
   else
     {
-      thread = find_thread_ptid (ptid);
+      thread = find_thread_ptid (current_inferior (), ptid);
       if (!thread)
 	error (_("aix-thread resume: unknown pthread %ld"),
 	       ptid.lwp ());
@@ -1089,7 +1095,9 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
   if (!pd_active && status->kind == TARGET_WAITKIND_STOPPED
       && status->value.sig == GDB_SIGNAL_TRAP)
     {
-      struct regcache *regcache = get_thread_regcache (ptid);
+      process_stratum_target *proc_target
+	= current_inferior ()->process_target ();
+      struct regcache *regcache = get_thread_regcache (proc_target, ptid);
       struct gdbarch *gdbarch = regcache->arch ();
 
       if (regcache_read_pc (regcache)
@@ -1354,7 +1362,7 @@ aix_thread_target::fetch_registers (struct regcache *regcache, int regno)
     beneath ()->fetch_registers (regcache, regno);
   else
     {
-      thread = find_thread_ptid (regcache->ptid ());
+      thread = find_thread_ptid (current_inferior (), regcache->ptid ());
       aix_thread_info *priv = get_aix_thread_info (thread);
       tid = priv->tid;
 
@@ -1692,7 +1700,7 @@ aix_thread_target::store_registers (struct regcache *regcache, int regno)
     beneath ()->store_registers (regcache, regno);
   else
     {
-      thread = find_thread_ptid (regcache->ptid ());
+      thread = find_thread_ptid (current_inferior (), regcache->ptid ());
       aix_thread_info *priv = get_aix_thread_info (thread);
       tid = priv->tid;
 
@@ -1740,7 +1748,9 @@ aix_thread_target::thread_alive (ptid_t ptid)
 
   /* We update the thread list every time the child stops, so all
      valid threads should be in the thread list.  */
-  return in_thread_list (ptid);
+  process_stratum_target *proc_target
+    = current_inferior ()->process_target ();
+  return in_thread_list (proc_target, ptid);
 }
 
 /* Return a printable representation of composite PID for use in
diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c
index 2bd24e7cee..ea328d7433 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -721,7 +721,7 @@ lynx_request_interrupt (void)
 
 /* The LynxOS target_ops vector.  */
 
-static struct target_ops lynx_target_ops = {
+static process_stratum_target lynx_target_ops = {
   lynx_create_inferior,
   NULL,  /* post_create_inferior */
   lynx_attach,
diff --git a/gdb/gdbserver/nto-low.c b/gdb/gdbserver/nto-low.c
index d77fda54b8..f950cd0458 100644
--- a/gdb/gdbserver/nto-low.c
+++ b/gdb/gdbserver/nto-low.c
@@ -931,7 +931,7 @@ nto_sw_breakpoint_from_kind (int kind, int *size)
 }
 
 
-static struct target_ops nto_target_ops = {
+static process_stratum_target nto_target_ops = {
   nto_create_inferior,
   NULL,  /* post_create_inferior */
   nto_attach,
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 6f7dd98764..ba14210826 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -705,7 +705,7 @@ ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
 	       int connected_wait);
 
 /* Prepare to read or write memory from the inferior process.  See the
-   corresponding target_ops methods for more details.  */
+   corresponding process_stratum_target methods for more details.  */
 
 int prepare_to_access_memory (void);
 void done_accessing_memory (void);
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 7088ba4dd1..c915bcda09 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -306,7 +306,7 @@ win32_stopped_data_address (void)
 /* Transfer memory from/to the debugged process.  */
 static int
 child_xfer_memory (CORE_ADDR memaddr, char *our, int len,
-		   int write, struct target_ops *target)
+		   int write, process_stratum_target *target)
 {
   BOOL success;
   SIZE_T done = 0;
@@ -1777,7 +1777,7 @@ win32_sw_breakpoint_from_kind (int kind, int *size)
   return the_low_target.breakpoint;
 }
 
-static struct target_ops win32_target_ops = {
+static process_stratum_target win32_target_ops = {
   win32_create_inferior,
   NULL,  /* post_create_inferior */
   win32_attach,
diff --git a/gdb/inferior-iter.h b/gdb/inferior-iter.h
index 0b2a3e55d8..0f484cde3e 100644
--- a/gdb/inferior-iter.h
+++ b/gdb/inferior-iter.h
@@ -36,8 +36,7 @@ public:
   typedef int difference_type;
 
   /* Create an iterator pointing at HEAD.  */
-  explicit all_inferiors_iterator (process_stratum_target *proc_target,
-				   inferior *head)
+  all_inferiors_iterator (process_stratum_target *proc_target, inferior *head)
     : m_proc_target (proc_target)
   {
     /* Advance M_INF to the first inferior's position.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 473a5e0b85..e86c22ca31 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -70,6 +70,7 @@
 #include "gdbsupport/scope-exit.h"
 #include "gdbsupport/forward-scope-exit.h"
 #include "gdb_select.h"
+#include <unordered_map>
 
 /* Prototypes for local functions */
 
@@ -2248,6 +2249,9 @@ do_target_resume (ptid_t resume_ptid, int step, enum gdb_signal sig)
   target_resume (resume_ptid, step, sig);
 
   target_commit_resume ();
+
+  if (target_can_async_p ())
+    target_async (1);
 }
 
 /* Resume the inferior.  SIG is the signal to give the inferior
@@ -2866,12 +2870,22 @@ commit_resume_all_targets ()
 {
   scoped_restore_current_thread restore_thread;
 
+  /* Map between process_target and a representative inferior.  This
+     is to avoid committing a resume in the same target more than
+     once.  Resumptions must be idempotent, so this is an
+     optimization.  */
+  std::unordered_map<process_stratum_target *, inferior *> conn_inf;
+
   for (inferior *inf : all_non_exited_inferiors ())
     if (inf->has_execution ())
-      {
-	switch_to_inferior_no_thread (inf);
-	target_commit_resume ();
-      }
+      conn_inf[inf->process_target ()] = inf;
+
+  for (const auto &ci : conn_inf)
+    {
+      inferior *inf = ci.second;
+      switch_to_inferior_no_thread (inf);
+      target_commit_resume ();
+    }
 }
 
 /* Basic routine for continuing the program in various fashions.
@@ -4509,7 +4523,7 @@ THREAD_STOPPED_BY (hw_breakpoint)
 /* Save the thread's event and stop reason to process it later.  */
 
 static void
-save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
+save_waitstatus (struct thread_info *tp, const target_waitstatus *ws)
 {
   if (debug_infrun)
     {
@@ -4667,30 +4681,28 @@ stop_all_threads (void)
 	    pass = -1;
 
 	  wait_one_event event = wait_one ();
-	  target_waitstatus &ws = event.ws;
-	  ptid_t &event_ptid = event.ptid;
 
 	  if (debug_infrun)
 	    {
 	      fprintf_unfiltered (gdb_stdlog,
 				  "infrun: stop_all_threads %s %s\n",
-				  target_waitstatus_to_string (&ws).c_str (),
-				  target_pid_to_str (event_ptid).c_str ());
+				  target_waitstatus_to_string (&event.ws).c_str (),
+				  target_pid_to_str (event.ptid).c_str ());
 	    }
 
-	  if (ws.kind == TARGET_WAITKIND_NO_RESUMED
-	      || ws.kind == TARGET_WAITKIND_THREAD_EXITED
-	      || ws.kind == TARGET_WAITKIND_EXITED
-	      || ws.kind == TARGET_WAITKIND_SIGNALLED)
+	  if (event.ws.kind == TARGET_WAITKIND_NO_RESUMED
+	      || event.ws.kind == TARGET_WAITKIND_THREAD_EXITED
+	      || event.ws.kind == TARGET_WAITKIND_EXITED
+	      || event.ws.kind == TARGET_WAITKIND_SIGNALLED)
 	    {
 	      /* All resumed threads exited
 		 or one thread/process exited/signalled.  */
 	    }
 	  else
 	    {
-	      thread_info *t = find_thread_ptid (event.target, event_ptid);
+	      thread_info *t = find_thread_ptid (event.target, event.ptid);
 	      if (t == NULL)
-		t = add_thread (event.target, event_ptid);
+		t = add_thread (event.target, event.ptid);
 
 	      t->stop_requested = 0;
 	      t->executing = 0;
@@ -4699,15 +4711,15 @@ stop_all_threads (void)
 
 	      /* This may be the first time we see the inferior report
 		 a stop.  */
-	      inferior *inf = find_inferior_ptid (event.target, event_ptid);
+	      inferior *inf = find_inferior_ptid (event.target, event.ptid);
 	      if (inf->needs_setup)
 		{
 		  switch_to_thread_no_regs (t);
 		  setup_inferior (0);
 		}
 
-	      if (ws.kind == TARGET_WAITKIND_STOPPED
-		  && ws.value.sig == GDB_SIGNAL_0)
+	      if (event.ws.kind == TARGET_WAITKIND_STOPPED
+		  && event.ws.value.sig == GDB_SIGNAL_0)
 		{
 		  /* We caught the event that we intended to catch, so
 		     there's no event pending.  */
@@ -4736,7 +4748,7 @@ stop_all_threads (void)
 
 		  if (debug_infrun)
 		    {
-		      std::string statstr = target_waitstatus_to_string (&ws);
+		      std::string statstr = target_waitstatus_to_string (&event.ws);
 
 		      fprintf_unfiltered (gdb_stdlog,
 					  "infrun: target_wait %s, saving "
@@ -4748,10 +4760,10 @@ stop_all_threads (void)
 		    }
 
 		  /* Record for later.  */
-		  save_waitstatus (t, &ws);
+		  save_waitstatus (t, &event.ws);
 
-		  sig = (ws.kind == TARGET_WAITKIND_STOPPED
-			 ? ws.value.sig : GDB_SIGNAL_0);
+		  sig = (event.ws.kind == TARGET_WAITKIND_STOPPED
+			 ? event.ws.value.sig : GDB_SIGNAL_0);
 
 		  if (displaced_step_fixup (t, sig) < 0)
 		    {
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index b9866c2259..ad33914c32 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -409,7 +409,7 @@ nto_procfs_target::update_thread_list ()
 	   (e.g. thread exited).  */
 	continue;
       ptid = ptid_t (pid, 0, tid);
-      new_thread = find_thread_ptid (ptid);
+      new_thread = find_thread_ptid (this, ptid);
       if (!new_thread)
 	new_thread = add_thread (ptid);
       update_thread_private_data (new_thread, tid, status.state, 0);
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 848ac7d6e7..f6b9bfbcfc 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -160,6 +160,8 @@ public:
 
   int can_use_hw_breakpoint (enum bptype, int, int) override;
   bool stopped_data_address (CORE_ADDR *) override;
+
+  void procfs_init_inferior (int pid);
 };
 
 static procfs_target the_procfs_target;
@@ -1315,6 +1317,7 @@ proc_set_current_signal (procinfo *pi, int signo)
     char sinfo[sizeof (siginfo_t)];
   } arg;
   siginfo_t mysinfo;
+  process_stratum_target *wait_target;
   ptid_t wait_ptid;
   struct target_waitstatus wait_status;
 
@@ -1327,8 +1330,9 @@ proc_set_current_signal (procinfo *pi, int signo)
     pi = find_procinfo_or_die (pi->pid, 0);
 
   /* The pointer is just a type alias.  */
-  get_last_target_status (&wait_ptid, &wait_status);
-  if (wait_ptid == inferior_ptid
+  get_last_target_status (&wait_target, &wait_ptid, &wait_status);
+  if (wait_target == &the_procfs_target
+      && wait_ptid == inferior_ptid
       && wait_status.kind == TARGET_WAITKIND_STOPPED
       && wait_status.value.sig == gdb_signal_from_host (signo)
       && proc_get_status (pi)
@@ -1988,7 +1992,7 @@ do_attach (ptid_t ptid)
 
   /* Add it to gdb's thread list.  */
   ptid = ptid_t (pi->pid, lwpid, 0);
-  add_thread (ptid);
+  add_thread (&the_procfs_target, ptid);
 
   return ptid;
 }
@@ -2286,7 +2290,7 @@ wait_again:
 		    if (print_thread_events)
 		      printf_unfiltered (_("[%s exited]\n"),
 					 target_pid_to_str (retval).c_str ());
-		    delete_thread (find_thread_ptid (retval));
+		    delete_thread (find_thread_ptid (this, retval));
 		    status->kind = TARGET_WAITKIND_SPURIOUS;
 		    return retval;
 		  }
@@ -2308,7 +2312,7 @@ wait_again:
 		    if (!proc_run_process (pi, 0, 0))
 		      proc_error (pi, "target_wait, run_process", __LINE__);
 
-		    inf = find_inferior_pid (pi->pid);
+		    inf = find_inferior_pid (this, pi->pid);
 		    if (inf->attach_flag)
 		      {
 			/* Don't call wait: simulate waiting for exit,
@@ -2395,8 +2399,8 @@ wait_again:
 
 		    temp_ptid = ptid_t (pi->pid, temp_tid, 0);
 		    /* If not in GDB's thread list, add it.  */
-		    if (!in_thread_list (temp_ptid))
-		      add_thread (temp_ptid);
+		    if (!in_thread_list (this, temp_ptid))
+		      add_thread (this, temp_ptid);
 
 		    /* Return to WFI, but tell it to immediately resume.  */
 		    status->kind = TARGET_WAITKIND_SPURIOUS;
@@ -2407,7 +2411,7 @@ wait_again:
 		    if (print_thread_events)
 		      printf_unfiltered (_("[%s exited]\n"),
 					 target_pid_to_str (retval).c_str ());
-		    delete_thread (find_thread_ptid (retval));
+		    delete_thread (find_thread_ptid (this, retval));
 		    status->kind = TARGET_WAITKIND_SPURIOUS;
 		    return retval;
 		  }
@@ -2464,8 +2468,8 @@ wait_again:
 
 		    /* If not in GDB's thread list, add it.  */
 		    temp_ptid = ptid_t (pi->pid, temp_tid, 0);
-		    if (!in_thread_list (temp_ptid))
-		      add_thread (temp_ptid);
+		    if (!in_thread_list (this, temp_ptid))
+		      add_thread (this, temp_ptid);
 
 		    status->kind = TARGET_WAITKIND_STOPPED;
 		    status->value.sig = GDB_SIGNAL_0;
@@ -2493,12 +2497,12 @@ wait_again:
 		 threads database, add it.  */
 	      if (retval.pid () > 0
 		  && retval != inferior_ptid
-		  && !in_thread_list (retval))
+		  && !in_thread_list (this, retval))
 		{
 		  /* We have a new thread.  We need to add it both to
 		     GDB's list and to our own.  If we don't create a
 		     procinfo, resume may be unhappy later.  */
-		  add_thread (retval);
+		  add_thread (this, retval);
 		  if (find_procinfo (retval.pid (),
 				     retval.lwp ()) == NULL)
 		    create_procinfo (retval.pid (),
@@ -2851,8 +2855,8 @@ procfs_target::mourn_inferior ()
    whatever is necessary to make the child ready to be debugged, and
    then wait for the child to synchronize.  */
 
-static void
-procfs_init_inferior (struct target_ops *ops, int pid)
+void
+procfs_target::procfs_init_inferior (int pid)
 {
   procinfo *pi;
   int fail;
@@ -2860,8 +2864,8 @@ procfs_init_inferior (struct target_ops *ops, int pid)
 
   /* This routine called on the parent side (GDB side)
      after GDB forks the inferior.  */
-  if (!target_is_pushed (ops))
-    push_target (ops);
+  if (!target_is_pushed (this))
+    push_target (this);
 
   pi = create_procinfo (pid, 0);
   if (pi == NULL)
@@ -2922,8 +2926,7 @@ procfs_init_inferior (struct target_ops *ops, int pid)
   /* We already have a main thread registered in the thread table at
      this point, but it didn't have any lwp info yet.  Notify the core
      about it.  This changes inferior_ptid as well.  */
-  thread_change_ptid (ptid_t (pid),
-		      ptid_t (pid, lwpid, 0));
+  thread_change_ptid (this, ptid_t (pid), ptid_t (pid, lwpid, 0));
 
   gdb_startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
 }
@@ -3090,9 +3093,9 @@ procfs_target::create_inferior (const char *exec_file,
   /* We have something that executes now.  We'll be running through
      the shell at this point (if startup-with-shell is true), but the
      pid shouldn't change.  */
-  add_thread_silent (ptid_t (pid));
+  add_thread_silent (this, ptid_t (pid));
 
-  procfs_init_inferior (this, pid);
+  procfs_init_inferior (pid);
 }
 
 /* An observer for the "inferior_created" event.  */
@@ -3109,9 +3112,9 @@ procfs_notice_thread (procinfo *pi, procinfo *thread, void *ptr)
 {
   ptid_t gdb_threadid = ptid_t (pi->pid, thread->tid, 0);
 
-  thread_info *thr = find_thread_ptid (gdb_threadid);
+  thread_info *thr = find_thread_ptid (&the_procfs_target, gdb_threadid);
   if (thr == NULL || thr->state == THREAD_EXITED)
-    add_thread (gdb_threadid);
+    add_thread (&the_procfs_target, gdb_threadid);
 
   return 0;
 }
@@ -3740,7 +3743,7 @@ procfs_do_thread_registers (bfd *obfd, ptid_t ptid,
 			    char *note_data, int *note_size,
 			    enum gdb_signal stop_signal)
 {
-  struct regcache *regcache = get_thread_regcache (ptid);
+  struct regcache *regcache = get_thread_regcache (&the_procfs_target, ptid);
   gdb_gregset_t gregs;
   gdb_fpregset_t fpregs;
   unsigned long merged_pid;
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 7afced0753..5c8d537aae 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -372,9 +372,8 @@ ravenscar_thread_target::wait (ptid_t ptid,
 static void
 ravenscar_add_thread (struct ada_task_info *task)
 {
-  process_stratum_target *targ = current_inferior ()->process_target ();
-  if (find_thread_ptid (targ, task->ptid) == NULL)
-    add_thread (targ, task->ptid);
+  if (find_thread_ptid (current_inferior (), task->ptid) == NULL)
+    add_thread (current_inferior ()->process_target (), task->ptid);
 }
 
 void
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 45bba8d146..936facbf1a 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2155,7 +2155,7 @@ record_btrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
   /* We just indicate the resume intent here.  The actual stepping happens in
      record_btrace_wait below.
 
-      For all-stop targets, we only step INFERIOR_PTID and continue others.  */
+     For all-stop targets, we only step INFERIOR_PTID and continue others.  */
 
   process_stratum_target *proc_target = current_inferior ()->process_target ();
 
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 7120882b10..ecea0aad0e 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -397,8 +397,8 @@ struct regcache *
 get_thread_regcache_for_ptid (ptid_t ptid)
 {
   /* This function doesn't take a process_stratum_target parameter
-     because it's a common/ routine implemented by both gdb and
-     gdbserver.  It always refers to a ptid of the current target. */
+     because it's a gdbsupport/ routine implemented by both gdb and
+     gdbserver.  It always refers to a ptid of the current target.  */
   process_stratum_target *proc_target = current_inferior ()->process_target ();
   return get_thread_regcache (proc_target, ptid);
 }
diff --git a/gdb/remote.c b/gdb/remote.c
index d86d5cfd9b..92af4df099 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -277,7 +277,8 @@ public: /* data */
 
   /* The status of the stub support for the various vCont actions.  */
   vCont_action_support supports_vCont;
-  /* Whether vCont support was probed already.  */
+  /* Whether vCont support was probed already.  This is a workaround
+     until packet_support is per-connection.  */
   bool supports_vCont_probed;
 
   /* True if the user has pressed Ctrl-C, but the target hasn't
@@ -6616,8 +6617,6 @@ remote_target::commit_resume ()
     }
 
   vcont_builder.flush ();
-
-  target_async (1);
 }
 
 \f
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index 68fa85130a..869a88acb8 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -461,9 +461,13 @@ sol_thread_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
       /* See if we have a new thread.  */
       if (rtnval.tid_p () && rtnval != save_ptid)
 	{
-	  thread_info *thr = find_thread_ptid (rtnval);
+	  thread_info *thr = find_thread_ptid (current_inferior (), rtnval);
 	  if (thr == NULL || thr->state == THREAD_EXITED)
-	    add_thread (rtnval);
+	    {
+	      process_stratum_target *proc_target
+		= current_inferior ()->process_target ();
+	      add_thread (proc_target, rtnval);
+	    }
 	}
     }
 
@@ -853,7 +857,8 @@ ps_lgetregs (struct ps_prochandle *ph, lwpid_t lwpid, prgregset_t gregset)
 {
   ptid_t ptid = ptid_t (inferior_ptid.pid (), lwpid, 0);
   struct regcache *regcache
-    = get_thread_arch_regcache (ptid, target_gdbarch ());
+    = get_thread_arch_regcache (current_inferior ()->process_target (),
+				ptid, target_gdbarch ());
 
   target_fetch_registers (regcache, -1);
   fill_gregset (regcache, (gdb_gregset_t *) gregset, -1);
@@ -869,7 +874,8 @@ ps_lsetregs (struct ps_prochandle *ph, lwpid_t lwpid,
 {
   ptid_t ptid = ptid_t (inferior_ptid.pid (), lwpid, 0);
   struct regcache *regcache
-    = get_thread_arch_regcache (ptid, target_gdbarch ());
+    = get_thread_arch_regcache (current_inferior ()->process_target (),
+				ptid, target_gdbarch ());
 
   supply_gregset (regcache, (const gdb_gregset_t *) gregset);
   target_store_registers (regcache, -1);
@@ -921,7 +927,8 @@ ps_lgetfpregs (struct ps_prochandle *ph, lwpid_t lwpid,
 {
   ptid_t ptid = ptid_t (inferior_ptid.pid (), lwpid, 0);
   struct regcache *regcache
-    = get_thread_arch_regcache (ptid, target_gdbarch ());
+    = get_thread_arch_regcache (current_inferior ()->process_target (),
+				ptid, target_gdbarch ());
 
   target_fetch_registers (regcache, -1);
   fill_fpregset (regcache, (gdb_fpregset_t *) fpregset, -1);
@@ -937,7 +944,8 @@ ps_lsetfpregs (struct ps_prochandle *ph, lwpid_t lwpid,
 {
   ptid_t ptid = ptid_t (inferior_ptid.pid (), lwpid, 0);
   struct regcache *regcache
-    = get_thread_arch_regcache (ptid, target_gdbarch ());
+    = get_thread_arch_regcache (current_inferior ()->process_target (),
+				ptid, target_gdbarch ());
 
   supply_fpregset (regcache, (const gdb_fpregset_t *) fpregset);
   target_store_registers (regcache, -1);
@@ -1037,9 +1045,13 @@ sol_update_thread_list_callback (const td_thrhandle_t *th, void *ignored)
     return -1;
 
   ptid_t ptid = ptid_t (inferior_ptid.pid (), 0, ti.ti_tid);
-  thread_info *thr = find_thread_ptid (ptid);
+  thread_info *thr = find_thread_ptid (current_inferior (), ptid);
   if (thr == NULL || thr->state == THREAD_EXITED)
-    add_thread (ptid);
+    {
+      process_stratum_target *proc_target
+	= current_inferior ()->process_target ();
+      add_thread (proc_target, ptid);
+    }
 
   return 0;
 }
diff --git a/gdb/target.c b/gdb/target.c
index 0b4a7dfa1d..c62a45b2f7 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1969,31 +1969,6 @@ target_pre_inferior (int from_tty)
   agent_capability_invalidate ();
 }
 
-/* Callback for iterate_over_inferiors.  Gets rid of the given
-   inferior.  */
-
-static void
-dispose_inferior (inferior *inf)
-{
-  /* Not all killed inferiors can, or will ever be, removed from the
-     inferior list.  Killed inferiors clearly don't need to be killed
-     again, so, we're done.  */
-  if (inf->pid == 0)
-    return;
-
-  thread_info *thread = any_thread_of_inferior (inf);
-  if (thread != NULL)
-    {
-      switch_to_thread (thread);
-
-      /* Core inferiors actually should be detached, not killed.  */
-      if (target_has_execution)
-	target_kill ();
-      else
-	target_detach (inf, 0);
-    }
-}
-
 /* This is to be called by the open routine before it does
    anything.  */
 
@@ -2007,7 +1982,14 @@ target_preopen (int from_tty)
       if (!from_tty
 	  || !target_has_execution
 	  || query (_("A program is being debugged already.  Kill it? ")))
-	dispose_inferior (current_inferior ());
+	{
+	  /* Core inferiors actually should be detached, not
+	     killed.  */
+	  if (target_has_execution)
+	    target_kill ();
+	  else
+	    target_detach (current_inferior (), 0);
+	}
       else
 	error (_("Program not killed."));
     }
@@ -4045,7 +4027,7 @@ set_write_memory_permission (const char *args, int from_tty,
 }
 
 void
-_initialize_target (void)
+_initialize_target ()
 {
   the_debug_target = new debug_target ();
 
diff --git a/gdb/thread-iter.h b/gdb/thread-iter.h
index 8586653df2..445a349797 100644
--- a/gdb/thread-iter.h
+++ b/gdb/thread-iter.h
@@ -92,8 +92,8 @@ public:
 
   /* Creates an iterator that iterates over all threads that match
      FILTER_PTID.  */
-  explicit all_matching_threads_iterator (process_stratum_target *filter_target,
-					  ptid_t filter_ptid);
+  all_matching_threads_iterator (process_stratum_target *filter_target,
+				 ptid_t filter_ptid);
 
   /* Create a one-past-end iterator.  */
   all_matching_threads_iterator ()
@@ -214,8 +214,8 @@ struct all_threads_safe_range
 struct all_matching_threads_range
 {
 public:
-  explicit all_matching_threads_range (process_stratum_target *filter_target,
-				       ptid_t filter_ptid)
+  all_matching_threads_range (process_stratum_target *filter_target,
+			      ptid_t filter_ptid)
     : m_filter_target (filter_target), m_filter_ptid (filter_ptid)
   {}
   all_matching_threads_range ()
@@ -241,8 +241,8 @@ private:
 class all_non_exited_threads_range
 {
 public:
-  explicit all_non_exited_threads_range (process_stratum_target *filter_target,
-					 ptid_t filter_ptid)
+  all_non_exited_threads_range (process_stratum_target *filter_target,
+				ptid_t filter_ptid)
     : m_filter_target (filter_target), m_filter_ptid (filter_ptid)
   {}
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a756913cab..7c10f83ddb 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -349,6 +349,8 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
   bool get_tib_address (ptid_t ptid, CORE_ADDR *addr) override;
 
   const char *thread_name (struct thread_info *) override;
+
+  int get_windows_debug_event (int pid, struct target_waitstatus *ourstatus);
 };
 
 static windows_nat_target the_windows_nat_target;
@@ -457,9 +459,9 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
      the main thread silently (in reality, this thread is really
      more of a process to the user than a thread).  */
   if (main_thread_p)
-    add_thread_silent (ptid);
+    add_thread_silent (&the_windows_nat_target, ptid);
   else
-    add_thread (ptid);
+    add_thread (&the_windows_nat_target, ptid);
 
   /* Set the debug registers for the new thread if they are used.  */
   if (debug_registers_used)
@@ -528,7 +530,7 @@ windows_delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p)
 		       target_pid_to_str (ptid).c_str (),
 		       (unsigned) exit_code);
 
-  delete_thread (find_thread_ptid (ptid));
+  delete_thread (find_thread_ptid (&the_windows_nat_target, ptid));
 
   for (th = &thread_head;
        th->next != NULL && th->next->id != id;
@@ -1516,9 +1518,10 @@ ctrl_c_handler (DWORD event_type)
 
 /* Get the next event from the child.  Returns a non-zero thread id if the event
    requires handling by WFI (or whatever).  */
-static int
-get_windows_debug_event (struct target_ops *ops,
-			 int pid, struct target_waitstatus *ourstatus)
+
+int
+windows_nat_target::get_windows_debug_event (int pid,
+					     struct target_waitstatus *ourstatus)
 {
   BOOL debug_event;
   DWORD continue_status, event_code;
@@ -1548,8 +1551,7 @@ get_windows_debug_event (struct target_ops *ops,
 		     "CREATE_THREAD_DEBUG_EVENT"));
       if (saw_create != 1)
 	{
-	  struct inferior *inf;
-	  inf = find_inferior_pid (current_event.dwProcessId);
+	  inferior *inf = find_inferior_pid (this, current_event.dwProcessId);
 	  if (!saw_create && inf->attach_flag)
 	    {
 	      /* Kludge around a Windows bug where first event is a create
@@ -1756,7 +1758,7 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	     the user tries to resume the execution in the inferior.
 	     This is a classic race that we should try to fix one day.  */
       SetConsoleCtrlHandler (&ctrl_c_handler, TRUE);
-      retval = get_windows_debug_event (this, pid, ourstatus);
+      retval = get_windows_debug_event (pid, ourstatus);
       SetConsoleCtrlHandler (&ctrl_c_handler, FALSE);
 
       if (retval)
-- 
2.14.5