fix a 'set detach-on-fork off', and manual "detach" problem

[Pedro Alves <pedro at codesourcery dot com>]

Here's the next multi-forks weirdness:

 > ./gdb -ex "set follow-fork-mode parent" -ex "set detach-on-fork off" ./testsuite/gdb.threads/fork-child-threads
 GNU gdb (GDB) 6.8.50.20081212-cvs
 ...
 (gdb) start
 Temporary breakpoint 1 at 0x400635: file ../../../src/gdb/testsuite/gdb.threads/fork-child-threads.c, line 34.
 Starting program: /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/fork-child-threads
 [Thread debugging using libthread_db enabled]

 Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.threads/fork-child-threads.c:34
 34        switch (fork ())
 (gdb) n
 49        return 0;
 (gdb) info forks
   1 process 21537 at 0x7ffff767ec4b, <fork>
 * 0 Thread 0x7ffff7fd66e0 (LWP 21529) (main process) at 0x400664, file fork-child-threads.c, line 49

Okay, two forks listed.

 (gdb) detach

This detaches from the current fork, ...

 Detaching from program: /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/fork-child-threads, process 21529
 [Switching to process 21537]

... and claims to have switched to the next fork.

There you go, we switch to the next fork.

 (gdb) info forks
 * 1 process 21537 at 0x0
   0 process 21529 (main process) at 0x0

Ooopps, two forks still listed.  Also, 'at 0x0' shows brokenness.  There
should be no forks listed at this point (when linux-forks.c detects
that only one fork is around it drops to standard no-forks mode).

 (gdb) info inferiors
 * 2 21537

Only one inferior.  This one's right.

 (gdb) info threads
 (gdb) 

But no threads. This is wrong, since we always have a
thread nowadays, and there's was indeed an inferior process
listed.

 (gdb) c
 The program is not being run.

Hmmm...

 (gdb) maint print target-stack
 The current target stack is:
   - exec (Local exec file)
   - None (None)
 (gdb)

Outch!

Notice that thread_db_detach calls target_mourn_inferior, while
linux_nat_detach doesn't.  This makes detach behave inconsistently
in a threaded vs a non-threaded inferior, since linux_nat_mourn inferior
holds the logic to switch to another fork.

I believe a thread_stratum target should never call
target_mourn_inferior itself --- that is a job for the process_stratum layer.

The target_mourn_inferior call in thread_db_detach is done
after the linux-nat target having detached and popped itself out,
and set inferior_ptid to null_ptid, so inferior_ptid is
already null_ptid when linux_nat_mourn_inferior is reached.

This means that:

thread_db_detach
 -> target_mourn_inferior
    -> linux_nat_mourn_inferior
       -> linux_fork_mourn_inferior

when we reach here, we try to delete the inferior_ptid fork, but
since inferior_ptid == null_ptid, at this point we won't be able
to find a fork to delete.

The attached patch fixes these issues, by making thread_db_detach
not call target_mourn_inferior, and adjusting linux_nat_detach to
handle the multi-forks case.  By adding a check to inf-ptrace.c to
not pop itself out if there are still inferiors to debug, we get
rid of the hack of needing to re-pushing the linux-nat target after
following a fork.

Here's what happens with the patch installed.

 >./gdb -ex "set follow-fork-mode parent" -ex "set detach-on-fork off" ./testsuite/gdb.threads/fork-child-threads
 GNU gdb (GDB) 6.8.50.20081212-cvs
 ...
 (gdb) start
 Temporary breakpoint 1 at 0x400635: file ../../../src/gdb/testsuite/gdb.threads/fork-child-threads.c, line 34.
 Starting program: /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/fork-child-threads
 [Thread debugging using libthread_db enabled]

 Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.threads/fork-child-threads.c:34
 34        switch (fork ())
 (gdb) n
 49        return 0;
 (gdb) info forks
   1 process 27925 at 0x7ffff767ec4b, <fork>
 * 0 Thread 0x7ffff7fd66e0 (LWP 27921) (main process) at 0x400664, file fork-child-threads.c, line 49

OK, parent and child.

 (gdb) info inferiors
   2 27925
 * 1 27921

There they are.

 (gdb) info threads
 * 1 Thread 0x7ffff7fd66e0 (LWP 27921)  main () at ../../../src/gdb/testsuite/gdb.threads/fork-child-threads.c:49

 (gdb) detach
 [Switching to process 27925]

 (gdb) info forks
 No forks.

 (gdb) info inferiors
 * 2 27925

 (gdb) info threads
 * 1 process 27925  0x00007ffff767ec4b in ?? ()

Of course, having linux-forks.c do the killing and detaching itself, and
the fact that it relies on inferiors being single-threaded is not
pretty, and break badly when you try to do anything more than this
with multi-threaded forks.  I'm not fond of this automatic switching done
in the targets either, as it makes some things quite ugly and hacky.  E.g., I'm
not adding a check_for_thread_db call in linux_nat_detach after switching
forks, as it would do more harm than good currently.
But, those are all pre-existing problems, that I'm not making it
worse, I think.

I'm in fact trying to incrementally fix up these target stack issues so
we can move the multi-forks support into using the common code's multi-inferior
functionality, and then have the possibility to debug more than one
multi-threaded inferior simultaneously.

Anyone thinks this patch is a bad idea?  Tested on x86_64-linux-gnu,
no regressions.

-- 
Pedro Alves

2008-12-12  Pedro Alves  <pedro@codesourcery.com>

	* linux-fork.c (linux_fork_detach): New.
	* linux-fork.h (linux_fork_detach): Declare.
	* linux-nat.c (linux_child_follow_fork): When following the fork
	child, add the child inferior before possibly detaching from the
	parent.  Don't reinstall ourselves.
	(linux_nat_detach): Call linux_fork_detach if there are other
	forks to debug.
	* linux-thread-db.c (thread_db_detach): Don't call
	target_mourn_inferior.  Instead inline the necessary bits.
	* inf-ptrace.c (inf_ptrace_detach): Don't unpush the target if
	there are other inferiors to debug.

---
 gdb/inf-ptrace.c      |    4 +++-
 gdb/linux-fork.c      |   34 ++++++++++++++++++++++++++++++++++
 gdb/linux-fork.h      |    1 +
 gdb/linux-nat.c       |   28 ++++++++++++++++++++--------
 gdb/linux-thread-db.c |   11 ++++++++---
 5 files changed, 66 insertions(+), 12 deletions(-)

Index: src/gdb/linux-fork.c
===================================================================
--- src.orig/gdb/linux-fork.c	2008-12-12 22:07:34.000000000 +0000
+++ src/gdb/linux-fork.c	2008-12-12 23:39:22.000000000 +0000
@@ -387,6 +387,40 @@ linux_fork_mourn_inferior (void)
     delete_fork (inferior_ptid);
 }
 
+/* The current inferior_ptid is being detached, but there are other
+   viable forks to debug.  Detach and delete it and context-switch to
+   the first available.  */
+
+extern void
+linux_fork_detach (char *args, int from_tty)
+{
+  /* OK, inferior_ptid is the one we are detaching from.  We need to
+     delete it from the fork_list, and switch to the next available
+     fork.  */
+
+  if (ptrace (PTRACE_DETACH, PIDGET (inferior_ptid), 0, 0))
+    error (_("Unable to detach %s"), target_pid_to_str (inferior_ptid));
+
+  delete_fork (inferior_ptid);
+  /* Delete process from GDB's inferior list.  */
+  delete_inferior (ptid_get_pid (inferior_ptid));
+
+  /* There should still be a fork - if there's only one left,
+     delete_fork won't remove it, because we haven't updated
+     inferior_ptid yet.  */
+  gdb_assert (fork_list);
+
+  fork_load_infrun_state (fork_list);
+
+  if (from_tty)
+    printf_filtered (_("[Switching to %s]\n"),
+		     target_pid_to_str (inferior_ptid));
+
+  /* If there's only one fork, switch back to non-fork mode.  */
+  if (fork_list->next == NULL)
+    delete_fork (inferior_ptid);
+}
+
 /* Fork list <-> user interface.  */
 
 static void
Index: src/gdb/linux-fork.h
===================================================================
--- src.orig/gdb/linux-fork.h	2008-12-12 22:07:34.000000000 +0000
+++ src/gdb/linux-fork.h	2008-12-12 23:01:38.000000000 +0000
@@ -23,6 +23,7 @@ extern struct fork_info *find_fork_pid (
 extern void fork_save_infrun_state (struct fork_info *, int);
 extern void linux_fork_killall (void);
 extern void linux_fork_mourn_inferior (void);
+extern void linux_fork_detach (char *, int);
 extern int  forks_exist_p (void);
 
 struct fork_info *fork_list;
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c	2008-12-12 22:07:34.000000000 +0000
+++ src/gdb/linux-nat.c	2008-12-13 00:15:07.000000000 +0000
@@ -826,6 +826,11 @@ linux_child_follow_fork (struct target_o
 			    child_pid);
 	}
 
+      /* Add the new inferior first, so that the target_detach below
+	 doesn't unpush the target.  */
+
+      add_inferior (child_pid);
+
       /* If we're vforking, we may want to hold on to the parent until
 	 the child exits or execs.  At exec time we can remove the old
 	 breakpoints from the parent and detach it; at exit time we
@@ -868,12 +873,7 @@ linux_child_follow_fork (struct target_o
 	target_detach (NULL, 0);
 
       inferior_ptid = ptid_build (child_pid, child_pid, 0);
-      add_inferior (child_pid);
-
-      /* Reinstall ourselves, since we might have been removed in
-	 target_detach (which does other necessary cleanup).  */
 
-      push_target (ops);
       linux_nat_switch_fork (inferior_ptid);
       check_for_thread_db ();
 
@@ -1621,12 +1621,24 @@ linux_nat_detach (struct target_ops *ops
   /* Destroy LWP info; it's no longer valid.  */
   init_lwp_list ();
 
-  pid = GET_PID (inferior_ptid);
-  inferior_ptid = pid_to_ptid (pid);
-  linux_ops->to_detach (ops, args, from_tty);
+  pid = ptid_get_pid (inferior_ptid);
 
   if (target_can_async_p ())
     drain_queued_events (pid);
+
+  if (forks_exist_p ())
+    {
+      /* Multi-fork case.  The current inferior_ptid is being detached
+	 from, but there are other viable forks to debug.  Detach from
+	 the current fork, and context-switch to the first
+	 available.  */
+      linux_fork_detach (args, from_tty);
+
+      if (non_stop && target_can_async_p ())
+ 	target_async (inferior_event_handler, 0);
+    }
+  else
+    linux_ops->to_detach (ops, args, from_tty);
 }
 
 /* Resume LP.  */
Index: src/gdb/linux-thread-db.c
===================================================================
--- src.orig/gdb/linux-thread-db.c	2008-12-12 22:07:34.000000000 +0000
+++ src/gdb/linux-thread-db.c	2008-12-12 23:01:38.000000000 +0000
@@ -784,10 +784,15 @@ thread_db_detach (struct target_ops *ops
 {
   disable_thread_event_reporting ();
 
-  target_beneath->to_detach (target_beneath, args, from_tty);
+  /* Forget about the child's process ID.  We shouldn't need it
+     anymore.  */
+  proc_handle.pid = 0;
 
-  /* Should this be done by detach_command?  */
-  target_mourn_inferior ();
+  /* Detach thread_db target ops.  */
+  unpush_target (&thread_db_ops);
+  using_thread_db = 0;
+
+  target_beneath->to_detach (target_beneath, args, from_tty);
 }
 
 /* Check if PID is currently stopped at the location of a thread event
Index: src/gdb/inf-ptrace.c
===================================================================
--- src.orig/gdb/inf-ptrace.c	2008-12-12 22:07:34.000000000 +0000
+++ src/gdb/inf-ptrace.c	2008-12-12 23:01:38.000000000 +0000
@@ -309,7 +309,9 @@ inf_ptrace_detach (struct target_ops *op
 
   inferior_ptid = null_ptid;
   detach_inferior (pid);
-  unpush_target (ops);
+
+  if (!have_inferiors ())
+    unpush_target (ops);
 }
 
 /* Kill the inferior.  */