This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
fix a 'set detach-on-fork off', and manual "detach" problem
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Sat, 13 Dec 2008 00:25:29 +0000
- Subject: fix a 'set detach-on-fork off', and manual "detach" problem
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. */