[PATCH v4 22/44] gdb, remote: don't create an inferior on attach

[Markus Metzger <markus.t.metzger@intel.com>]

When attaching to a process in an extended remote target, we pass the PID
to attach to in the vAttach packet.  Some targets may interpret that PID
differently and provide inferiors with other IDs later on.  Rely on thread
events to learn about inferiors.

Old targets that do not support multi-process or that do not support the
qAttached packet will not be able to tell us about process IDs.  For those
targets, we still create the default inferior using the PID passed to
attach.

Not creating the inferior early in extended_remote_target::attach() may
cause the inferior to which we attached to be noticed two times and each
time we will add an attach_post_wait() continuation.  The first time in
update_thread_list(), where we resume the inferior afterwards.  The second
time in attach_command() where we want to stop the inferior.

The first time had been avoided by adding the inferior without going
through remote_notice_new_inferior().

The two continuations are executed in reverse order, thus first stopping
the inferior and then resuming it again.  Fix that by adding continuations
to the end of the continuation list.

This has been exposed by gdb.base/dlmopen.exp.

When resuming in the first continuation with schedule-multiple on, we
resume not only the inferior we attached to, but also other inferiors that
might already be stopped.  In non-stop mode, we take care to stop only
that inferior, but in all-stop mode, we do not.  Temporarily disable
schedule-multiple to only resume that inferior.

This has been exposed by gdb.threads/detach-step-over.exp.

This leaves gdb.threads/detach-step-over.exp failing because we now issue
a stop notification for all threads in non-stop mode, whereas the original
behavior was to suppress the stop notification for the one thread that was
already stopped by the target.  This is a side-effect of not having the
additional attach_post_wait() continuation that initially resumes all
threads, including the one that had been stopped.

This is achieved by returning early from remote_notice_new_inferior() in
the special case that inferior_ptid has been set to ptid_t (pid):

      if (inferior_ptid.is_pid ()
          && pid == inferior_ptid.pid ())
        {
          /* inferior_ptid has no thread member yet.  This can happen
             with the vAttach -> remote_wait,"TAAthread:" path if the
             stub doesn't support qC.  This is the first stop reported
             after an attach, so this is the main thread.  Update the
             ptid in the thread list.  */
          if (in_thread_list (this, ptid_t (pid)))
            thread_change_ptid (this, inferior_ptid, currthread);
          else
            {
              thread_info *thr
                = remote_add_thread (currthread, running, executing, false);
              switch_to_thread (thr);
            }
          return;
        }

While resuming threads when noticing a new inferior only to stop them a
bit later as part of attach doesn't seem a big deal, given that we already
stopped and resumed them in update_thread_list(), it differs from the
behavior of the native target, and we want the same test to cover both.

This patch removes setting inferior_ptid and leaves it at null_ptid.
Adjust this special case to also consider null_ptid.

Since we're now avoiding a second attach_post_wait(), we wouldn't need to
change how continuations are handled.  I find it more intuitive that they
are added to the back, however, so I leave this hunk in the patch.
---
 gdb/infcmd.c   |  3 +++
 gdb/inferior.c |  2 +-
 gdb/remote.c   | 55 +++++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index c2a860cb288..3da5a1366aa 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2787,6 +2787,9 @@ attach_post_wait (int from_tty, enum attach_post_wait_mode mode)
 	{
 	  if (inferior_thread ()->stop_signal () == GDB_SIGNAL_0)
 	    {
+	      scoped_restore save_multi
+		= make_scoped_restore (&sched_multi, 0);
+
 	      clear_proceed_status (0);
 	      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
 	    }
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 229abdd1ef8..cc7d1cc4f94 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -180,7 +180,7 @@ inferior::set_arch (gdbarch *arch)
 void
 inferior::add_continuation (std::function<void ()> &&cont)
 {
-  m_continuations.emplace_front (std::move (cont));
+  m_continuations.emplace_back (std::move (cont));
 }
 
 void
diff --git a/gdb/remote.c b/gdb/remote.c
index 753b59a0819..5721e791612 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1420,6 +1420,7 @@ class remote_target : public process_stratum_target
   void remote_detach_pid (int pid);
 
   void remote_vcont_probe ();
+  void remote_qattached_probe ();
 
   void remote_resume_with_hc (ptid_t ptid, int step,
 			      gdb_signal siggnal);
@@ -3220,6 +3221,7 @@ remote_target::remote_notice_new_inferior (ptid_t currthread,
 
   if (!in_thread_list (this, currthread))
     {
+      struct remote_state *rs = get_remote_state ();
       struct inferior *inf = NULL;
       int pid = currthread.pid ();
 
@@ -3240,8 +3242,8 @@ remote_target::remote_notice_new_inferior (ptid_t currthread,
 	  target_find_description ();
 	}
 
-      if (inferior_ptid.is_pid ()
-	  && pid == inferior_ptid.pid ())
+      if (inferior_ptid == null_ptid
+	  || (inferior_ptid.is_pid () && pid == inferior_ptid.pid ()))
 	{
 	  /* inferior_ptid has no thread member yet.  This can happen
 	     with the vAttach -> remote_wait,"TAAthread:" path if the
@@ -3254,7 +3256,9 @@ remote_target::remote_notice_new_inferior (ptid_t currthread,
 	    {
 	      thread_info *thr
 		= remote_add_thread (currthread, state, internal_state, false);
-	      switch_to_thread (thr);
+
+	      if (!rs->starting_up)
+		switch_to_thread (thr);
 	    }
 	  return;
 	}
@@ -3278,13 +3282,8 @@ remote_target::remote_notice_new_inferior (ptid_t currthread,
 	 it needs to with it (e.g., read shared libraries, insert
 	 breakpoints), unless we're just setting up an all-stop
 	 connection.  */
-      if (inf != NULL)
-	{
-	  struct remote_state *rs = get_remote_state ();
-
-	  if (!rs->starting_up)
-	    notice_new_inferior (new_thr, internal_state, 0);
-	}
+      if (inf != nullptr && !rs->starting_up)
+	notice_new_inferior (new_thr, internal_state, 0);
     }
 }
 
@@ -5457,6 +5456,9 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
      attribute.  */
   remote_vcont_probe ();
 
+  /* Similarly, probe qAttached.  */
+  remote_qattached_probe ();
+
   /* If the stub wants to get a QAllow, compose one and send it.  */
   if (m_features.packet_support (PACKET_QAllow) != PACKET_DISABLE)
     set_permissions ();
@@ -7041,15 +7043,27 @@ extended_remote_target::attach (const char *args, int from_tty)
 	     target_pid_to_str (ptid_t (pid)).c_str (), result.err_msg ());
     }
 
-  switch_to_inferior_no_thread (remote_add_inferior (false, pid, 1, 0));
+  /* Do not create a process assuming PID as the process ID.  We will
+     learn about new processes from the remote target.  This allows
+     targets to provide other process IDs than the one we passed to
+     vAttach.
 
-  inferior_ptid = ptid_t (pid);
+     For targets that are not able to provide the information we need,
+     create a default inferior now.  */
+  if (!m_features.remote_multi_process_p ()
+      || !(m_features.packet_support (PACKET_qAttached) == PACKET_ENABLE))
+    switch_to_inferior_no_thread (remote_add_inferior (false, pid, 1, 0));
 
   if (target_is_non_stop_p ())
     {
       /* Get list of threads.  */
       update_thread_list ();
 
+      /* If we have got a new inferior for PID, switch to it.  */
+      inferior *inferior = find_inferior_pid (this, pid);
+      if (inferior != nullptr)
+	switch_to_inferior_no_thread (inferior);
+
       thread_info *thread = first_thread_of_inferior (current_inferior ());
       if (thread != nullptr)
 	switch_to_thread (thread);
@@ -7063,6 +7077,10 @@ extended_remote_target::attach (const char *args, int from_tty)
 	 ptid.  */
       ptid_t curr_ptid = remote_current_thread (ptid_t (pid));
 
+      /* We may not have created the inferior yet.  */
+      if (find_inferior_pid (this, curr_ptid.pid ()) == nullptr)
+	remote_add_inferior (false, curr_ptid.pid (), 1, 0);
+
       /* Add the main thread to the thread list.  We add the thread
 	 silently in this case (the final true parameter).  */
       thread_info *thr = remote_add_thread (curr_ptid, THREAD_RUNNING,
@@ -7164,6 +7182,19 @@ remote_target::remote_vcont_probe ()
   m_features.packet_ok (rs->buf, PACKET_vCont);
 }
 
+/* Probe whether the remote target supports qAttached.  */
+
+void
+remote_target::remote_qattached_probe ()
+{
+  remote_state *rs = get_remote_state ();
+
+  strcpy (rs->buf.data (), "qAttached");
+  putpkt (rs->buf);
+  getpkt (&rs->buf);
+  m_features.packet_ok (rs->buf, PACKET_qAttached);
+}
+
 /* Helper function for building "vCont" resumptions.  Write a
    resumption to P.  ENDP points to one-passed-the-end of the buffer
    we're allowed to write to.  Returns BUF+CHARACTERS_WRITTEN.  The
-- 
2.43.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.