Make follow_fork not rely on get_last_target_status

Currently, if

  - you're in all-stop mode,
  - the inferior last stopped because of a fork catchpoint,

when you next resume the program, gdb checks whether it had last
stopped for a fork/vfork, and if so,

 a) if the current thread is the one that forked, gdb follows the
   parent/child, depending on "set follow-fork" mode.

 b) if the current thread is some other thread (because you switched
   threads meanwhile), gdb switches back to that thread, gdb follows
   the parent/child, and stops the resumption command.

There's a problem in b), however -- if you have "set schedule-multiple
off", which is the default, or "set scheduler-locking on", gdb will
still switch back to the forking thread, even if you didn't want to
resume it.  For example, with:

  (gdb) catch fork
  (gdb) c
  * thread 1 stops for fork
  (gdb) thread 2
  (gdb) set scheduler-locking on
  (gdb) c

gdb switches back to thread 1, and follows the fork.

Or with:

  (gdb) add-inferior -exec prog
  (gdb) inferior 2
  (gdb) start
  (gdb) inferior 1
  (gdb) catch fork
  (gdb) c
  * thread 1.1 stops for fork
  (gdb) inferior 2
  (gdb) set schedule-multiple off # this is the default
  (gdb) c

gdb switches back to thread 1.1, and follows the fork.

Another issue is that, because follow_fork relies on
get_last_target_status to find the thread that has a pending fork, it
is possible to confuse it.  For example, "run" or "start" call
init_wait_for_inferior, which clears the last target status, so this:

  (gdb) catch fork
  (gdb) c
  * thread 1 stops for fork
  (gdb) add-inferior -exec prog
  (gdb) inferior 2
  (gdb) start
  (gdb) set follow-fork child
  (gdb) inferior 1
  (gdb) n

... does not follow to the fork child of inferior 1, because the
get_last_target_status call in follow_fork doesn't return a
TARGET_WAITKIND_FORKED.  Thanks to Simon for this example.

All of the above are fixed by this patch.  It changes follow_fork to
not look at get_last_target_status, but to instead iterate over the
set of threads that the user is resuming, and find the one that has a
pending_follow kind of fork/vfork.

gdb.base/foll-fork.exp is augmented to exercise the last "start"
scenario described above.  The other cases will be exercised in the
testcase added by the following patch.

Change-Id: Ifcca77e7b2456277387f40660ef06cec2b93b97e

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 05c824d2141103619db43a757d952aa4b6896a95..04d76895f5e5ebeb95eeeea9edd1d88f52b4a158 100644 (file)
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -706,6 +706,15 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
   return false;
 }
 
+/* Set the last target status as TP having stopped.  */
+
+static void
+set_last_target_status_stopped (thread_info *tp)
+{
+  set_last_target_status (tp->inf->process_target (), tp->ptid,
+                         target_waitstatus {}.set_stopped (GDB_SIGNAL_0));
+}
+
 /* Tell the target to follow the fork we're stopped at.  Returns true
    if the inferior should be resumed; false, if the target for some
    reason decided it's best not to resume.  */
@@ -730,32 +739,33 @@ follow_fork ()
 
   if (!non_stop)
     {
-      process_stratum_target *wait_target;
-      ptid_t wait_ptid;
-      struct target_waitstatus wait_status;
+      thread_info *cur_thr = inferior_thread ();
 
-      /* Get the last target status returned by target_wait().  */
-      get_last_target_status (&wait_target, &wait_ptid, &wait_status);
-
-      /* If not stopped at a fork event, then there's nothing else to
-        do.  */
-      if (wait_status.kind () != TARGET_WAITKIND_FORKED
-         && wait_status.kind () != TARGET_WAITKIND_VFORKED)
-       return 1;
+      ptid_t resume_ptid
+       = user_visible_resume_ptid (cur_thr->control.stepping_command);
+      process_stratum_target *resume_target
+       = user_visible_resume_target (resume_ptid);
 
-      /* Check if we switched over from WAIT_PTID, since the event was
-        reported.  */
-      if (wait_ptid != minus_one_ptid
-         && (current_inferior ()->process_target () != wait_target
-             || inferior_ptid != wait_ptid))
+      /* Check if there's a thread that we're about to resume, other
+        than the current, with an unfollowed fork/vfork.  If so,
+        switch back to it, to tell the target to follow it (in either
+        direction).  We'll afterwards refuse to resume, and inform
+        the user what happened.  */
+      for (thread_info *tp : all_non_exited_threads (resume_target,
+                                                    resume_ptid))
        {
-         /* We did.  Switch back to WAIT_PTID thread, to tell the
-            target to follow it (in either direction).  We'll
-            afterwards refuse to resume, and inform the user what
-            happened.  */
-         thread_info *wait_thread = find_thread_ptid (wait_target, wait_ptid);
-         switch_to_thread (wait_thread);
-         should_resume = false;
+         if (tp == cur_thr)
+           continue;
+
+         if (tp->pending_follow.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;
+             break;
+           }
        }
     }
 
@@ -819,21 +829,16 @@ follow_fork ()
          }
        else
          {
-           /* This makes sure we don't try to apply the "Switched
-              over from WAIT_PID" logic above.  */
-           nullify_last_target_wait_ptid ();
-
            /* If we followed the child, switch to it...  */
            if (follow_child)
              {
-               thread_info *child_thr = find_thread_ptid (parent_targ, child);
-               switch_to_thread (child_thr);
+               tp = find_thread_ptid (parent_targ, child);
+               switch_to_thread (tp);
 
                /* ... and preserve the stepping state, in case the
                   user was stepping over the fork call.  */
                if (should_resume)
                  {
-                   tp = inferior_thread ();
                    tp->control.step_resume_breakpoint
                      = step_resume_breakpoint;
                    tp->control.step_range_start = step_range_start;
@@ -872,6 +877,8 @@ follow_fork ()
       break;
     }
 
+  if (!should_resume)
+    set_last_target_status_stopped (tp);
   return should_resume;
 }
 

diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index 51dfcce7ff5d40abd8bd6347452c431d44d2ed43..df5dab056c219ca2e79320057ba651b3f042dbd1 100644 (file)
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -209,15 +209,22 @@ set reading_in_symbols_re {(?:\r\nReading in symbols for [^\r\n]*)?}
 # Test the ability to catch a fork, specify that the child be
 # followed, and continue.  Make the catchpoint permanent.
 
-proc_with_prefix catch_fork_child_follow {} {
+proc_with_prefix catch_fork_child_follow {second_inferior} {
     global gdb_prompt
     global srcfile
     global reading_in_symbols_re
 
+    if { $second_inferior && [use_gdb_stub] } {
+       return
+    }
+
     if { ![setup] } {
        return
     }
 
+    # Get rid of the breakpoint at "main".
+    delete_breakpoints
+
     set bp_after_fork [gdb_get_line_number "set breakpoint here"]
 
     gdb_test "catch fork" \
@@ -246,6 +253,29 @@ proc_with_prefix catch_fork_child_follow {} {
        "Temporary breakpoint.*, line $bp_after_fork.*" \
        "set follow-fork child, tbreak"
 
+    if {$second_inferior} {
+       gdb_test "add-inferior" "Added inferior 2.*" "add inferior 2"
+
+       gdb_test "inferior 2" "Switching to inferior 2.*"
+
+       gdb_load $::binfile
+
+       # Start it.  This should not affect inferior 1, given "set
+       # schedule-multiple off" (default).  GDB used to have a bug
+       # where "start" would clear the pending follow fork
+       # information of inferior 1.
+       gdb_test "start" "Starting program.*Temporary breakpoint .*"
+
+       gdb_test "inferior 1" "Switching to inferior 1.*"
+
+       # Verify that the catchpoint is still mentioned in an "info
+       # breakpoints", and further that the catchpoint still shows
+       # the captured process id.
+       gdb_test "info breakpoints" \
+           ".*catchpoint.*keep y.*fork, process.*" \
+           "info breakpoints, after starting second inferior"
+    }
+
     set expected_re "\\\[Attaching after.* fork to.*\\\[Detaching after fork from"
     append expected_re ".* at .*$bp_after_fork.*"
     gdb_test "continue" $expected_re "set follow-fork child, hit tbreak"
@@ -428,6 +458,8 @@ foreach_with_prefix follow-fork-mode {"parent" "child"} {
 
 # Catchpoint tests.
 
-catch_fork_child_follow
+foreach_with_prefix second_inferior {false true} {
+    catch_fork_child_follow $second_inferior
+}
 catch_fork_unpatch_child
 tcatch_fork_parent_follow