all-stop "follow-fork parent" and selecting another thread

With:

 - catch a fork in thread 1
 - select thread 2
 - set follow-fork child
 - next

... follow_fork notices that thread 1 had last stopped for a fork
which hasn't been followed yet, and because thread 1 is not the
current thread, GDB aborts the execution command, presenting the stop
in thread 1.

That makes sense, as only the forking thread (thread 1) survives in
the child, so better stop and let the user decide how to proceed.

However, with:

 - catch a fork in thread 1
 - select thread 2
 - set follow-fork parent << note difference here
 - next

... GDB does the same: follow_fork notices that thread 1 had last
stopped for a fork which hasn't been followed yet, and because thread
1 is not the current thread, GDB aborts the execution command,
presenting the stop in thread 1.

Aborting/stopping in this case doesn't make sense to me.  As we're
following the parent, thread 2 will still continue to exist in the
parent.  What the child does after we've followed the parent shouldn't
matter -- it can go on running free, be detached, etc., depending on
"set schedule-multiple", "set detach-on-fork", etc.  That does not
influence the execution command that the user issued for the parent
thread.

So this patch changes GDB in that direction -- in follow_fork, if
following the parent, and we've switched threads meanwhile, switch
back to the unfollowed thread, follow it (stay with the parent), and
don't abort/stop.  If we're following a fork (as opposed to vfork),
then switch back again to the thread that the user was trying to
resume.  If following a vfork, however, stay with the vforking-thread
selected, as we will need to see a vfork_done event first, before we
can resume any other thread.

As I was working on this, I managed to end up calling target_resume
for a solo-thread resume (to collect the vfork_done event), with
scope_ptid pointing at the vfork parent thread, and inferior_ptid
pointing to the vfork child.  For a solo-thread resume, the scope_ptid
argument to target_resume must the same as inferior_ptid.  The mistake
was caught by the assertion in target_resume, like so:

...
  [infrun] resume_1: step=0, signal=GDB_SIGNAL_0, trap_expected=0, current thread [1722839.1722839.0] at 0x5555555553c3
  [infrun] do_target_resume: resume_ptid=1722839.1722939.0, step=0, sig=GDB_SIGNAL_0
../../src/gdb/target.c:2661: internal-error: target_resume: Assertion `inferior_ptid.matches (scope_ptid)' failed.
...

but I think it doesn't hurt to catch such a mistake earlier, hence the
change in internal_resume_ptid.

Change-Id: I896705506a16d2488b1bfb4736315dd966f4e412

1 files changed, 79 insertions, 9 deletions

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 04d76895f5e..beb9ca79389 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -757,13 +757,56 @@ follow_fork ()
 	  if (tp == cur_thr)
 	    continue;
 
-	  if (tp->pending_follow.kind () != TARGET_WAITKIND_SPURIOUS)
+	  /* follow_fork_inferior clears tp->pending_follow, and below
+	     we'll need the value after the follow_fork_inferior
+	     call.  */
+	  target_waitkind kind = tp->pending_follow.kind ();
+
+	  if (kind != TARGET_WAITKIND_SPURIOUS)
 	    {
 	      infrun_debug_printf ("need to follow-fork [%s] first",
 				   tp->ptid.to_string ().c_str ());
 
 	      switch_to_thread (tp);
-	      should_resume = false;
+
+	      /* Set up inferior(s) as specified by the caller, and
+		 tell the target to do whatever is necessary to follow
+		 either parent or child.  */
+	      if (follow_child)
+		{
+		  /* The thread that started the execution command
+		     won't exist in the child.  Abort the command and
+		     immediately stop in this thread, in the child,
+		     inside fork.  */
+		  should_resume = false;
+		}
+	      else
+		{
+		  /* Following the parent, so let the thread fork its
+		     child freely, it won't influence the current
+		     execution command.  */
+		  if (follow_fork_inferior (follow_child, detach_fork))
+		    {
+		      /* Target refused to follow, or there's some
+			 other reason we shouldn't resume.  */
+		      switch_to_thread (cur_thr);
+		      set_last_target_status_stopped (cur_thr);
+		      return false;
+		    }
+
+		  /* If we're following a vfork, when we need to leave
+		     the just-forked thread as selected, as we need to
+		     solo-resume it to collect the VFORK_DONE event.
+		     If we're following a fork, however, switch back
+		     to the original thread that we continue stepping
+		     it, etc.  */
+		  if (kind != TARGET_WAITKIND_VFORKED)
+		    {
+		      gdb_assert (kind == TARGET_WAITKIND_FORKED);
+		      switch_to_thread (cur_thr);
+		    }
+		}
+
 	      break;
 	    }
 	}
@@ -2201,6 +2244,29 @@ user_visible_resume_target (ptid_t resume_ptid)
 	  : current_inferior ()->process_target ());
 }
 
+/* Find a thread from the inferiors that we'll resume that is waiting
+   for a vfork-done event.  */
+
+static thread_info *
+find_thread_waiting_for_vfork_done ()
+{
+  gdb_assert (!target_is_non_stop_p ());
+
+  if (sched_multi)
+    {
+      for (inferior *inf : all_non_exited_inferiors ())
+	if (inf->thread_waiting_for_vfork_done != nullptr)
+	  return inf->thread_waiting_for_vfork_done;
+    }
+  else
+    {
+      inferior *cur_inf = current_inferior ();
+      if (cur_inf->thread_waiting_for_vfork_done != nullptr)
+	return cur_inf->thread_waiting_for_vfork_done;
+    }
+  return nullptr;
+}
+
 /* Return a ptid representing the set of threads that we will resume,
    in the perspective of the target, assuming run control handling
    does not require leaving some threads stopped (e.g., stepping past
@@ -2241,14 +2307,18 @@ internal_resume_ptid (int user_step)
      Since we don't have that flexibility (we can only pass one ptid), just
      resume the first thread waiting for a vfork-done event we find (e.g. thread
      2.1).  */
-  if (sched_multi)
-    {
-      for (inferior *inf : all_non_exited_inferiors ())
-	if (inf->thread_waiting_for_vfork_done != nullptr)
-	  return inf->thread_waiting_for_vfork_done->ptid;
+  thread_info *thr = find_thread_waiting_for_vfork_done ();
+  if (thr != nullptr)
+    {
+      /* If we have a thread that is waiting for a vfork-done event,
+	 then we should have switched to it earlier.  Calling
+	 target_resume with thread scope is only possible when the
+	 current thread matches the thread scope.  */
+      gdb_assert (thr->ptid == inferior_ptid);
+      gdb_assert (thr->inf->process_target ()
+		  == inferior_thread ()->inf->process_target ());
+      return thr->ptid;
     }
-  else if (current_inferior ()->thread_waiting_for_vfork_done != nullptr)
-    return current_inferior ()->thread_waiting_for_vfork_done->ptid;
 
   return user_visible_resume_ptid (user_step);
 }