[PATCH v4 7/7] Remote fork catch

[Don Breazeal <donb@codesourcery.com>]

This patch implements catchpoints for fork events on remote and
extended-remote Linux targets.

Implementation appeared to be straightforward, requiring four new functions
in remote.c to implement insert/remove of fork/vfork catchpoints.  These
functions are essentially stubs that just return 0 ('success') if the
required events are enabled.  If the fork events are being reported, then
catchpoints are set and hit.

However, there are some extra issues that arise with catchpoints.

1) Thread creation reporting -- fork catchpoints are hit before the
   follow_fork has been completed.  In the native implementation, the new
   process is not 'reported' until after the follow is done.  It doesn't
   show up in the inferiors list or the threads list.  However, in
   gdbserver, an 'info threads' will retrieve the new thread info from the
   target and add it to GDB's data structures.  Because of this premature
   report, things on the GDB side eventually get very confused.

   So in gdbserver, in server.c:handle_qxfer_threads_worker, we check
   'last_status' and if it shows a FORK event, we know that we are in an
   unfollowed fork and we do not report the new (forked) thread to GDB.

2) Kill process before fork is followed -- on the native side in
   linux-nat.c:linux_nat_kill, there is some code to handle the case where
   a fork has occurred but follow_fork hasn't been called yet.  It does
   this by using the last status to determine if a follow is pending, and
   if it is, to kill the child task.  This patch uses similar code in
   linux-low.c:linux_kill.

3) One of the tests related to fork catchpoints,
   gdb.threads/fork-thread-pending.exp, depended on the threads being
   reported in a specific order.  GDBserver reported the threads in a
   different order, that is, 'info threads' showed the same threads, but in
   a different order.  The test used a hard-coded thread number to find a
   threads that (a) was not the main thread (blocked in pthread_join), and
   (b) was not the forking thread (stopped in fork).

   I implemented a new proc, find_unforked_thread, that uses a pretty
   brute-force method of finding a thread.  I considered just hard-coding
   another number (the native case used thread 2, which was the forking
   thread in the remote case), but that didn't seem future-proof.
   Suggestions on how to do this better would be welcome.
  
Tested on x64 Ubuntu Lucid, native, remote, extended-remote.  Tested the
case of killing the forking process before the fork has been followed
manually.  It wasn't clear to me how to check that a process had actually
been killed from a dejagnu test.

Thanks
--Don

gdb/gdbserver/
2015-01-25  Don Breazeal  <donb@codesourcery.com>

	* linux-low.c (handle_extended_wait): Fix braces in multi-line if.
	(linux_kill): Kill forked child when between fork and follow_fork.
	* server.c (handle_qxfer_threads_worker): Skip forked child thread
	when between fork and follow_fork.
	(get_last_target_status): New function.
	* server.h (get_last_target_status): Declare new function.

gdb/
2015-01-25  Don Breazeal  <donb@codesourcery.com>
	* remote.c (extended_remote_insert_fork_catchpoint): New function.
	(extended_remote_remove_fork_catchpoint): New function.
	(extended_remote_insert_vfork_catchpoint): New function.
	(extended_remote_remove_vfork_catchpoint): New function.
	(init_extended_remote_ops): Initialize target vector with
	new fork catchpoint functions.

gdb/testsuite/
2015-01-25  Don Breazeal  <donb@codesourcery.com>
	* gdb/testsuite/gdb.base/foll-fork.exp: Remove restriction for
	remote target.
	* gdb/testsuite/gdb.base/foll-vfork.exp: Likewise.
	* gdb/testsuite/gdb.threads/fork-child-threads.exp: Likewise.
	* gdb.threads/fork-thread-pending.exp: Likewise.
	(find_unforked_thread): New proc.
	* gdb/testsuite/gdb.threads/watchpoint-fork.exp: Remove restriction
	for remote target.

---
 gdb/gdbserver/linux-low.c                         |   22 +++++++++-
 gdb/gdbserver/server.c                            |   18 ++++++++
 gdb/gdbserver/server.h                            |    2 +
 gdb/remote.c                                      |   47 ++++++++++++++++++++-
 gdb/testsuite/gdb.base/foll-fork.exp              |    4 +-
 gdb/testsuite/gdb.base/foll-vfork.exp             |    4 +-
 gdb/testsuite/gdb.threads/fork-child-threads.exp  |    6 +-
 gdb/testsuite/gdb.threads/fork-thread-pending.exp |   27 ++++++++++--
 gdb/testsuite/gdb.threads/watchpoint-fork.exp     |    5 +-
 9 files changed, 117 insertions(+), 18 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 66b1729..eabf262 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -419,8 +419,8 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
 
 	  if (debug_threads)
 	    {
-	      debug_printf ("HEW: Got fork event from LWP %ld, "
-			    "new child is %d\n",
+	      debug_printf ("HEW: Got fork event "
+			    "from LWP %ld, new child is %d\n",
 			    ptid_get_lwp (ptid_of (event_thr)),
 			    ptid_get_pid (ptid));
 	    }
@@ -1045,6 +1045,24 @@ linux_kill (int pid)
 {
   struct process_info *process;
   struct lwp_info *lwp;
+  struct target_waitstatus last;
+  ptid_t last_ptid;
+
+  /* If we're stopped while forking and we haven't followed yet,
+     kill the child task.  We need to do this first because the
+     parent will be sleeping if this is a vfork.  */
+
+  get_last_target_status (&last_ptid, &last);
+
+  if (last.kind == TARGET_WAITKIND_FORKED
+      || last.kind == TARGET_WAITKIND_VFORKED)
+    {
+      lwp = find_lwp_pid (last.value.related_pid);
+      gdb_assert (lwp != NULL);
+      kill_wait_lwp (lwp);
+      process = find_process_pid (ptid_get_pid (last.value.related_pid));
+      the_target->mourn (process);
+    }
 
   process = find_process_pid (pid);
   if (process == NULL)
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 57190f6..f525d3e 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1301,6 +1301,15 @@ handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg)
   int core = target_core_of_thread (ptid);
   char core_s[21];
 
+  /* Skip new threads created as the result of a fork if we are not done
+     handling that fork event.  We won't know whether to tell GDB about
+     the new thread until we are done following the fork.  */
+  if ((last_status.kind == TARGET_WAITKIND_FORKED
+       || last_status.kind == TARGET_WAITKIND_VFORKED)
+      && (ptid_get_pid (last_status.value.related_pid)
+	  == ptid_get_pid (ptid)))
+    return;
+
   write_ptid (ptid_s, ptid);
 
   if (core != -1)
@@ -3992,3 +4001,12 @@ handle_target_event (int err, gdb_client_data client_data)
 
   return 0;
 }
+
+/* Retrieve the last waitstatus reported to GDB.  */
+
+void
+get_last_target_status (ptid_t *ptid, struct target_waitstatus *last)
+{
+  *ptid = last_ptid;
+  *last = last_status;
+}
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 5284dac..86e584e 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -102,6 +102,8 @@ typedef int gdb_fildes_t;
 /* Functions from server.c.  */
 extern int handle_serial_event (int err, gdb_client_data client_data);
 extern int handle_target_event (int err, gdb_client_data client_data);
+extern void get_last_target_status (ptid_t *ptid,
+				    struct target_waitstatus *last);
 
 #include "remote-utils.h"
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 6ea3c60..d59309f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1449,7 +1449,6 @@ remote_fork_event_p (struct remote_state *rs)
   return packet_support (PACKET_fork_event_feature) == PACKET_ENABLE;
 }
 
-#if PTRACE_FORK_EVENTS
 /* Returns true if vfork events are supported.  */
 
 static int
@@ -1457,7 +1456,7 @@ remote_vfork_event_p (struct remote_state *rs)
 {
   return packet_support (PACKET_vfork_event_feature) == PACKET_ENABLE;
 }
-#endif
+
 
 /* Target follow-fork function for remote targets.  On entry, and
    at return, the current inferior is the fork parent.
@@ -1509,6 +1508,46 @@ remote_follow_fork (struct target_ops *target, int follow_child,
   return 0;
 }
 
+/* Insert fork catchpoint target routine.  If fork events are enabled
+   then return success, nothing more to do.  */
+
+static int
+remote_insert_fork_catchpoint (struct target_ops *ops, int pid)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  return !remote_fork_event_p (rs);
+}
+
+/* Remove fork catchpoint target routine.  Nothing to do, just
+   return success.  */
+
+static int
+remote_remove_fork_catchpoint (struct target_ops *ops, int pid)
+{
+  return 0;
+}
+
+/* Insert vfork catchpoint target routine.  If vfork events are enabled
+   then return success, nothing more to do.  */
+
+static int
+remote_insert_vfork_catchpoint (struct target_ops *ops, int pid)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  return !remote_vfork_event_p (rs);
+}
+
+/* Remove vfork catchpoint target routine.  Nothing to do, just
+   return success.  */
+
+static int
+remote_remove_vfork_catchpoint (struct target_ops *ops, int pid)
+{
+  return 0;
+}
+
 /* Tokens for use by the asynchronous signal handlers for SIGINT.  */
 static struct async_signal_handler *async_sigint_remote_twice_token;
 static struct async_signal_handler *async_sigint_remote_token;
@@ -11766,6 +11805,10 @@ Specify the serial device it is connected to\n\
   remote_ops.to_augmented_libraries_svr4_read =
     remote_augmented_libraries_svr4_read;
   remote_ops.to_follow_fork = remote_follow_fork;
+  remote_ops.to_insert_fork_catchpoint = remote_insert_fork_catchpoint;
+  remote_ops.to_remove_fork_catchpoint = remote_remove_fork_catchpoint;
+  remote_ops.to_insert_vfork_catchpoint = remote_insert_vfork_catchpoint;
+  remote_ops.to_remove_vfork_catchpoint = remote_remove_vfork_catchpoint;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index 3f2d224..697570d 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -13,8 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-if { [is_remote target] || ![isnative] } then {
-    continue
+if {![isnative]} then {
+    return 0
 }
 
 # Until "set follow-fork-mode" and "catch fork" are implemented on
diff --git a/gdb/testsuite/gdb.base/foll-vfork.exp b/gdb/testsuite/gdb.base/foll-vfork.exp
index 78c5cc8..ec8db33 100644
--- a/gdb/testsuite/gdb.base/foll-vfork.exp
+++ b/gdb/testsuite/gdb.base/foll-vfork.exp
@@ -18,8 +18,8 @@
 # either execs or exits --- since those events take somewhat different
 # code paths in GDB, both variants are exercised.
 
-if { [is_remote target] || ![isnative] } then {
-    continue
+if {![isnative]} then {
+    return 0
 }
 
 # Until "set follow-fork-mode" and "catch vfork" are implemented on
diff --git a/gdb/testsuite/gdb.threads/fork-child-threads.exp b/gdb/testsuite/gdb.threads/fork-child-threads.exp
index 75e60e1..3815c92 100644
--- a/gdb/testsuite/gdb.threads/fork-child-threads.exp
+++ b/gdb/testsuite/gdb.threads/fork-child-threads.exp
@@ -13,12 +13,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Only GNU/Linux is known to support `set follow-fork-mode child'.
-if { ! [istarget "*-*-linux*"] } {
+if {![isnative] } then {
     return 0
 }
 
-if { [is_remote target] || ![isnative] } then {
+# Only GNU/Linux is known to support `set follow-fork-mode child'.
+if { ! [istarget "*-*-linux*"] } {
     return 0
 }
 
diff --git a/gdb/testsuite/gdb.threads/fork-thread-pending.exp b/gdb/testsuite/gdb.threads/fork-thread-pending.exp
index d229232..bbd980a 100644
--- a/gdb/testsuite/gdb.threads/fork-thread-pending.exp
+++ b/gdb/testsuite/gdb.threads/fork-thread-pending.exp
@@ -13,9 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# There's no support for `set follow-fork-mode' in the remote
-# protocol.
-if { [is_remote target] } {
+if {![isnative]} {
     return 0
 }
 
@@ -31,6 +29,26 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
     return -1
 }
 
+# Find a thread that did not fork and is not the main thread and
+# return its thread number.  We can't just hard-code the thread
+# number since we have no guarantee as to the ordering of the threads
+# in gdb.  We know that the main thread is in pthread_join and the
+# forking thread is in fork, so we use this rather ungainly regexp
+# to capture an entry from 'info threads' that doesn't show one of
+# those routines, then extract the thread number.
+
+proc find_unforked_thread { } {
+    gdb_test_multiple "info threads" "find unforked thread" {
+        -re "(\[^\r]*Thread\[^\r]* in \[^fp]\[^ot]\[^rh]\[^kr]\[^e]\[^a]\[^d]\[^_]\[^j]\[^\r]*\r\n)" {
+    	    regexp "(\[ 	]*)(\[0-9]*)(\[    ]*Thread\[^\r]*\r\n)" $expect_out(0,string) ignore lead_spc threadnum rest
+        }
+        timeout {
+	    set threadnum -1
+        }
+    }
+    return $threadnum
+}
+
 clean_restart ${binfile}
 
 if ![runto_main] then {
@@ -46,7 +64,8 @@ gdb_test "continue" "Catchpoint.*" "1, get to the fork event"
 
 gdb_test "info threads" " Thread .* Thread .* Thread .* Thread .*" "1, multiple threads found"
 
-gdb_test "thread 1" ".*" "1, switched away from event thread"
+set threadnum [find_unforked_thread]
+gdb_test "thread $threadnum" ".*" "1, switched away from event thread to thread $threadnum"
 
 gdb_test "continue" "Not resuming.*" "1, refused to resume"
 
diff --git a/gdb/testsuite/gdb.threads/watchpoint-fork.exp b/gdb/testsuite/gdb.threads/watchpoint-fork.exp
index a4557f8..a1dbd10 100644
--- a/gdb/testsuite/gdb.threads/watchpoint-fork.exp
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork.exp
@@ -17,9 +17,8 @@
 
 set testfile watchpoint-fork
 
-if [is_remote target] {
-    kfail "remote/13584" "gdbserver does not support debugging across fork"
-    return
+if {![isnative]} {
+    return 0
 }
 
 proc test {type symbol} {
-- 
1.7.0.4