GDB hangs on kill or quit (after following a fork child, not detaching from the parent)

GDB hangs on kill or quit (after following a fork child, not detaching from the parent)

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 4925 bytes --]

[ Michael, you're the forks man.  CCing you in case see an issue with
the attached patch?  ]

GDB hangs if you do:

 > ./gdb -ex "set follow-fork-mode child" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
 GNU gdb (GDB) 6.8.50.20081212-cvs
 (gdb) start
 Temporary breakpoint 1 at 0x40054f: file ../../../src/gdb/testsuite/gdb.base/foll-fork.c, line 21.
 Starting program: /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.base/foll-fork

 Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.base/foll-fork.c:21
 21        int  v = 5;
 (gdb) n
 During symbol reading, incomplete CFI data; unspecified registers (e.g., rax) at 0x40054b.
 23        pid = fork ();
 (gdb)
 [Switching to process 14900]
 24        if (pid == 0)
 (gdb) kill
 Kill the program being debugged? (y or n) y
 <HANGS HERE>       

The same will happen if you issues 'quit' instead of 'kill', as quitting
tries to 'kill' the inferior.

When there are forks involved, linux_nat_kill calls into linux_fork_killall
to do the killing.  But, when following a fork child, and not
detaching from the parent, we defer adding the child fork to the
list of forks (which is confusing IMHO, see below), so linux_fork_killall
misses killing it.  Then, we hang waiting for it to die, but it
won't happen.

"kill"
     -> linux_nat_kill
          -> linux_fork_killall.

This kills all forks listed.  At this point, since only the parent was listed,
the child was left alive.

After killing, we call target_mourn_inferior, which goes:

     -> linux_nat_kill
          -> linux_nat_mourn_inferior
                  ->inf_ptrace_mourn_inferior

Now, inferior_ptid was pointing at the child (we followed it),
and, we didn't kill it so, we hang here:

184     /* Clean up a rotting corpse of an inferior after it died.  */
185
186     static void
187     inf_ptrace_mourn_inferior (struct target_ops *ops)
188     {
189       int status;
190
191       /* Wait just one more time to collect the inferior's exit status.
192          Do not check whether this succeeds though, since we may be
193          dealing with a process that we attached to.  Such a process will
194          only report its exit status to its original parent.  */
(top-gdb)
195       waitpid (ptid_get_pid (inferior_ptid), &status, 0);
          ^^^^^^^

#0  0x00007fb6a45364a5 in waitpid () from /lib/libc.so.6
#1  0x00000000005df07b in inf_ptrace_mourn_inferior (ops=0xaf4ce0) at ../../src/gdb/inf-ptrace.c:195
#2  0x0000000000477c55 in linux_nat_mourn_inferior (ops=0xaf4ce0) at ../../src/gdb/linux-nat.c:3194
#3  0x0000000000543d82 in target_mourn_inferior () at ../../src/gdb/target.c:1899
#4  0x0000000000477c09 in linux_nat_kill () at ../../src/gdb/linux-nat.c:3180
#5  0x00000000004622b6 in kill_command (arg=0x0, from_tty=1) at ../../src/gdb/inflow.c:602
...

=================

The attached patch fixes it by also adding the child to the fork
list in this case.

Also, since we're now adding the child, one bit of special casing
from linux-fork.c can be removed, as in the patch.  The 'error' call
in linux-fork.c that I'm removing is really dead code, as all callers
do the same check.

This is the current output of info forks, when following a parent,
and when following a child:

./gdb -ex "set follow-fork-mode parent" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
...
 (gdb) info forks
   1 process 15396 at 0x7ffff789ac4b, <fork>
 * 0 process 15393 (main process) at 0x40055e, file foll-fork.c, line 24

./gdb -ex "set follow-fork-mode child" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
...
 (gdb) info forks
   1 process 15319 at 0x7ffff789ac4b, <fork>

Notice how the 'following the child' case is a bit convoluted:

  - Only the parent is listed, but it isn't that obvious that fork 1
    is the parent.
  - There's no indication of which fork is current.

 If you do:
 (gdb) fork 1
 Switching to process 16458
 #0  0x00007ffff789ac4b in fork () from /lib/libc.so.6
 (gdb) info forks
   2 process 16461 at 0x40055e, file foll-fork.c, line 24
 * 1 process 16458 at 0x7ffff789ac4b, <fork>
 (gdb)                               

 Voila! - the child fork was added (fork 2).  I find this confusing.

=================

This is the output after the patch is applied:

> ./gdb -ex "set follow-fork-mode child" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
...
 (gdb) info forks
 * 2 process 15934 at 0x40055e, file foll-fork.c, line 24
   1 process 15927 at 0x7ffff789ac4b, <fork>

>./gdb -ex "set follow-fork-mode parent" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
...
 (gdb) info forks
   1 process 15979 at 0x7ffff789ac4b, <fork>
 * 0 process 15974 (main process) at 0x40055e, file foll-fork.c, line 24

A bit more uniform, and the hang bug goes away, of course.  That special
casing to have a fork number 0 could go away too, IMVHO.

Tested on x86_64-linux-gnu, no regressions.

-- 
Pedro Alves

[-- Attachment #2: fix_killall.diff --]
[-- Type: text/x-diff, Size: 1870 bytes --]

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

	* linux-nat.c (linux_child_follow_fork): If following the child,
	and not detaching the parent, also add the child fork to the fork
	list.
	* linux-fork.c (linux_fork_context): Remove dead error call.
	Assert that the incoming newfp argument is not null.  Do not add a
	new fork for inferior_ptid.  Assert that there is one already.

---
 gdb/linux-fork.c |    9 ++++-----
 gdb/linux-nat.c  |    6 ++++++
 2 files changed, 10 insertions(+), 5 deletions(-)

Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c	2008-12-12 20:06:17.000000000 +0000
+++ src/gdb/linux-nat.c	2008-12-12 20:21:00.000000000 +0000
@@ -857,6 +857,12 @@ linux_child_follow_fork (struct target_o
 	  if (!fp)
 	    fp = add_fork (parent_pid);
 	  fork_save_infrun_state (fp, 0);
+
+	  /* Also add an entry for the child fork.  */
+	  fp = find_fork_pid (child_pid);
+	  if (!fp)
+	    fp = add_fork (child_pid);
+	  fork_save_infrun_state (fp, 0);
 	}
       else
 	target_detach (NULL, 0);
Index: src/gdb/linux-fork.c
===================================================================
--- src.orig/gdb/linux-fork.c	2008-12-12 20:09:43.000000000 +0000
+++ src/gdb/linux-fork.c	2008-12-12 20:09:21.000000000 +0000
@@ -600,15 +600,14 @@ static void
 linux_fork_context (struct fork_info *newfp, int from_tty)
 {
   /* Now we attempt to switch processes.  */
-  struct fork_info *oldfp = find_fork_ptid (inferior_ptid);
+  struct fork_info *oldfp;
   ptid_t ptid;
   int id, i;
 
-  if (!newfp)
-    error (_("No such fork/process"));
+  gdb_assert (newfp != NULL);
 
-  if (!oldfp)
-    oldfp = add_fork (ptid_get_pid (inferior_ptid));
+  oldfp = find_fork_ptid (inferior_ptid);
+  gdb_assert (oldfp != NULL);
 
   fork_save_infrun_state (oldfp, 1);
   remove_breakpoints ();

Re: GDB hangs on kill or quit (after following a fork child, not detaching from the parent)

[Michael Snyder]

Pedro Alves wrote:
> [ Michael, you're the forks man.  CCing you in case see an issue with
> the attached patch?  ]

That's unfortunate -- I haven't thought about this since implementing
fork-based checkpoints, 2 or 3 years ago.   ;-/


> When there are forks involved, linux_nat_kill calls into linux_fork_killall
> to do the killing.  But, when following a fork child, and not
> detaching from the parent, we defer adding the child fork to the
> list of forks (which is confusing IMHO, see below),

Do you have any intuition as to why we did that?
I don't remember.  Could it have been related to the
checkpoint case?

Otherwise it could simply have been an oversight...


I like your results, and your code changes look fine.
Can you confirm that it doesn't adversely affect the
checkpoint testsuites?

Re: GDB hangs on kill or quit (after following a fork child, not detaching from the parent)

[Pedro Alves]

On Thursday 18 December 2008 19:20:57, Michael Snyder wrote:

> > When there are forks involved, linux_nat_kill calls into linux_fork_killall
> > to do the killing.  But, when following a fork child, and not
> > detaching from the parent, we defer adding the child fork to the
> > list of forks (which is confusing IMHO, see below),
> 
> Do you have any intuition as to why we did that?
> I don't remember.  Could it have been related to the
> checkpoint case?
> 
> Otherwise it could simply have been an oversight...

Yeah, I should have mentioned it before:  At first I also
thought it was checkpoints related, then I noticed that
when 'set follow-fork-mode' is child, checkpoints are broken
for other reasons.  It may well be that it always was (broken):

gdb-6.8:

 (top-gdb) set follow-fork-mode child
 (top-gdb) start
 Breakpoint 3 at 0x4509a7: file ../../src/gdb/gdb.c, line 28.
 Starting program: /home/pedro/gdb/baseline/build/gdb/gdb
 [Thread debugging using libthread_db enabled]
 [New Thread 0x7ffff7fd36e0 (LWP 24392)]
 [Switching to Thread 0x7ffff7fd36e0 (LWP 24392)]
 main (argc=1, argv=0x7fffffffe3f8) at ../../src/gdb/gdb.c:28
 28        memset (&args, 0, sizeof args);
 (top-gdb) checkpoint
 warning: Can't attach process 24400: Operation not permitted
 /build/buildd/gdb-6.8/gdb/linux-thread-db.c:302: internal-error: thread_get_info_callback: Assertion `thread_info != NULL' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n)  

HEAD:

 (top-gdb) start
 Temporary breakpoint 3 at 0x4509a7: file ../../src/gdb/gdb.c, line 28.
 Starting program: /home/pedro/gdb/baseline/build/gdb/gdb
 [Thread debugging using libthread_db enabled]

 Temporary breakpoint 3, main (argc=
 During symbol reading, incomplete CFI data; unspecified registers (e.g., rax) at 0x45099c.
 1, argv=0x7fffffffe3f8) at ../../src/gdb/gdb.c:28
 28        memset (&args, 0, sizeof args);
 (top-gdb) set follow-fork-mode child
 (top-gdb) checkpoint
 [Switching to Thread 0x7ffff7fd36e0 (LWP 24520)]
 ../../src/gdb/regcache.c:359: internal-error: regcache_cpy_no_passthrough: Assertion `src != NULL && dst != NULL' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n) 

( this one is related to the fact that moving the infrun context
from the parent to the child isn't complete, so not completelly
checkpoints related... )

Notice that the checkpoints test never sets
follow-fork-mode to child, so this patch can't affect it.

I think that when checkpointing, we should always "follow"
the parent anyway; and that the checkpoints support should be
better insulated from the multi forks support, so that the
multi-forks support can grow into full multi-process support.

> I like your results, and your code changes look fine.
> Can you confirm that it doesn't adversely affect the
> checkpoint testsuites?

Yep, had done that.  No regressions in the checkpoints tests, or in
the rest of the testsuite.

I'll go check it in then.

Thanks!

-- 
Pedro Alves

Re: GDB hangs on kill or quit (after following a fork child, not detaching from the parent)

[Pedro Alves]

On Thursday 18 December 2008 21:03:41, Pedro Alves wrote:

> Yeah, I should have mentioned it before:  At first I also
> thought it was checkpoints related, then I noticed that
> when 'set follow-fork-mode' is child, checkpoints are broken
> for other reasons.  It may well be that it always was (broken):
> 
> gdb-6.8:
> 
>  (top-gdb) set follow-fork-mode child
>  (top-gdb) start
>  Breakpoint 3 at 0x4509a7: file ../../src/gdb/gdb.c, line 28.
>  Starting program: /home/pedro/gdb/baseline/build/gdb/gdb
>  [Thread debugging using libthread_db enabled]
>  [New Thread 0x7ffff7fd36e0 (LWP 24392)]
>  [Switching to Thread 0x7ffff7fd36e0 (LWP 24392)]
>  main (argc=1, argv=0x7fffffffe3f8) at ../../src/gdb/gdb.c:28
>  28        memset (&args, 0, sizeof args);
>  (top-gdb) checkpoint
>  warning: Can't attach process 24400: Operation not permitted
>  /build/buildd/gdb-6.8/gdb/linux-thread-db.c:302: internal-error: thread_get_info_callback: Assertion `thread_info != NULL' failed.
>  A problem internal to GDB has been detected,
>  further debugging may prove unreliable.
>  Quit this debugging session? (y or n)  
> 

That was a thread-db related issue, which you could claim is a bit
undefined for the current checkpoints support.

For completeness, here's a non-threaded example showing how
following the child behaved:

>gdb /home/pedro/gdb/tests/test
GNU gdb 6.8-debian
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu"...
Setting up the environment for debugging gdb.
Function "internal_error" not defined.
Make breakpoint pending on future shared library load? (y or [n]) [answered N; input not from terminal]
Function "info_command" not defined.
Make breakpoint pending on future shared library load? (y or [n]) [answered N; input not from terminal]
/home/pedro/gdb/baseline/build/gdb/.gdbinit:8: Error in sourced command file:
No breakpoint number 0.
(gdb) start
Breakpoint 1 at 0x40044c: file main.c, line 5.
Starting program: /home/pedro/gdb/tests/test
main () at main.c:5
5         return 0;
(gdb) maint print target-stack
The current target stack is:
  - child (Unix child process)
  - exec (Local exec file)
  - None (None)
(gdb) set follow-fork-mode child
(gdb) checkpoint
[Switching to process 28696]
checkpoint: fork returned pid 0.
Failed to find new fork
(gdb)  

-- 
Pedro Alves

Re: GDB hangs on kill or quit (after following a fork child, not detaching from the parent)

[Michael Snyder]

Pedro Alves wrote:
> On Thursday 18 December 2008 19:20:57, Michael Snyder wrote:
> 
>>> When there are forks involved, linux_nat_kill calls into linux_fork_killall
>>> to do the killing.  But, when following a fork child, and not
>>> detaching from the parent, we defer adding the child fork to the
>>> list of forks (which is confusing IMHO, see below),
>> Do you have any intuition as to why we did that?
>> I don't remember.  Could it have been related to the
>> checkpoint case?
>>
>> Otherwise it could simply have been an oversight...
> 
> Yeah, I should have mentioned it before:  At first I also
> thought it was checkpoints related, then I noticed that
> when 'set follow-fork-mode' is child, checkpoints are broken
> for other reasons.  It may well be that it always was (broken):

That would not be at all surprising.  Since checkpoints use
forks as underlying implementation, I would not expect that
checkpoint and follow-child would play well together.



> I think that when checkpointing, we should always "follow"
> the parent anyway; and that the checkpoints support should be
> better insulated from the multi forks support, so that the
> multi-forks support can grow into full multi-process support.

Agreed.  For starters, we might just document that checkpoints
are not defined to work for forking processes (or for multi-
threaded ones, for that matter).

> 
>> I like your results, and your code changes look fine.
>> Can you confirm that it doesn't adversely affect the
>> checkpoint testsuites?
> 
> Yep, had done that.  No regressions in the checkpoints tests, or in
> the rest of the testsuite.
> 
> I'll go check it in then.

OK.

Re: GDB hangs on kill or quit (after following a fork child, not detaching from the parent)

[teawater]

Hi,

This patch is for multiprocess branch right?
The code in fix_killall.diff:
+	  /* Also add an entry for the child fork.  */
+	  fp = find_fork_pid (child_pid);
+	  if (!fp)
+	    fp = add_fork (child_pid);
+	  fork_save_infrun_state (fp, 0);
is close to linux-nat-multiprocess-focus.txt in
http://sourceware.org/ml/gdb-patches/2008-12/msg00001.html:
+	  /* Retain child fork in ptrace (stopped) state.  */
+	  fp = find_fork_pid (child_pid);
+	  if (!fp)
+	    fp = add_fork (child_pid);
+	  fork_save_infrun_state (fp, 0);

So I think maybe patches in
http://sourceware.org/ml/gdb-patches/2008-12/msg00001.html and
http://sourceware.org/ml/gdb-patches/2008-12/msg00058.html can make
this branch for linux-nat work better.

Thanks,
Hui




On Sat, Dec 13, 2008 at 05:13, Pedro Alves <pedro@codesourcery.com> wrote:
> [ Michael, you're the forks man.  CCing you in case see an issue with
> the attached patch?  ]
>
> GDB hangs if you do:
>
>  > ./gdb -ex "set follow-fork-mode child" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
>  GNU gdb (GDB) 6.8.50.20081212-cvs
>  (gdb) start
>  Temporary breakpoint 1 at 0x40054f: file ../../../src/gdb/testsuite/gdb.base/foll-fork.c, line 21.
>  Starting program: /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.base/foll-fork
>
>  Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.base/foll-fork.c:21
>  21        int  v = 5;
>  (gdb) n
>  During symbol reading, incomplete CFI data; unspecified registers (e.g., rax) at 0x40054b.
>  23        pid = fork ();
>  (gdb)
>  [Switching to process 14900]
>  24        if (pid == 0)
>  (gdb) kill
>  Kill the program being debugged? (y or n) y
>  <HANGS HERE>
>
> The same will happen if you issues 'quit' instead of 'kill', as quitting
> tries to 'kill' the inferior.
>
> When there are forks involved, linux_nat_kill calls into linux_fork_killall
> to do the killing.  But, when following a fork child, and not
> detaching from the parent, we defer adding the child fork to the
> list of forks (which is confusing IMHO, see below), so linux_fork_killall
> misses killing it.  Then, we hang waiting for it to die, but it
> won't happen.
>
> "kill"
>     -> linux_nat_kill
>          -> linux_fork_killall.
>
> This kills all forks listed.  At this point, since only the parent was listed,
> the child was left alive.
>
> After killing, we call target_mourn_inferior, which goes:
>
>     -> linux_nat_kill
>          -> linux_nat_mourn_inferior
>                  ->inf_ptrace_mourn_inferior
>
> Now, inferior_ptid was pointing at the child (we followed it),
> and, we didn't kill it so, we hang here:
>
> 184     /* Clean up a rotting corpse of an inferior after it died.  */
> 185
> 186     static void
> 187     inf_ptrace_mourn_inferior (struct target_ops *ops)
> 188     {
> 189       int status;
> 190
> 191       /* Wait just one more time to collect the inferior's exit status.
> 192          Do not check whether this succeeds though, since we may be
> 193          dealing with a process that we attached to.  Such a process will
> 194          only report its exit status to its original parent.  */
> (top-gdb)
> 195       waitpid (ptid_get_pid (inferior_ptid), &status, 0);
>          ^^^^^^^
>
> #0  0x00007fb6a45364a5 in waitpid () from /lib/libc.so.6
> #1  0x00000000005df07b in inf_ptrace_mourn_inferior (ops=0xaf4ce0) at ../../src/gdb/inf-ptrace.c:195
> #2  0x0000000000477c55 in linux_nat_mourn_inferior (ops=0xaf4ce0) at ../../src/gdb/linux-nat.c:3194
> #3  0x0000000000543d82 in target_mourn_inferior () at ../../src/gdb/target.c:1899
> #4  0x0000000000477c09 in linux_nat_kill () at ../../src/gdb/linux-nat.c:3180
> #5  0x00000000004622b6 in kill_command (arg=0x0, from_tty=1) at ../../src/gdb/inflow.c:602
> ...
>
> =================
>
> The attached patch fixes it by also adding the child to the fork
> list in this case.
>
> Also, since we're now adding the child, one bit of special casing
> from linux-fork.c can be removed, as in the patch.  The 'error' call
> in linux-fork.c that I'm removing is really dead code, as all callers
> do the same check.
>
> This is the current output of info forks, when following a parent,
> and when following a child:
>
> ./gdb -ex "set follow-fork-mode parent" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
> ...
>  (gdb) info forks
>   1 process 15396 at 0x7ffff789ac4b, <fork>
>  * 0 process 15393 (main process) at 0x40055e, file foll-fork.c, line 24
>
> ./gdb -ex "set follow-fork-mode child" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
> ...
>  (gdb) info forks
>   1 process 15319 at 0x7ffff789ac4b, <fork>
>
> Notice how the 'following the child' case is a bit convoluted:
>
>  - Only the parent is listed, but it isn't that obvious that fork 1
>    is the parent.
>  - There's no indication of which fork is current.
>
>  If you do:
>  (gdb) fork 1
>  Switching to process 16458
>  #0  0x00007ffff789ac4b in fork () from /lib/libc.so.6
>  (gdb) info forks
>   2 process 16461 at 0x40055e, file foll-fork.c, line 24
>  * 1 process 16458 at 0x7ffff789ac4b, <fork>
>  (gdb)
>
>  Voila! - the child fork was added (fork 2).  I find this confusing.
>
> =================
>
> This is the output after the patch is applied:
>
>> ./gdb -ex "set follow-fork-mode child" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
> ...
>  (gdb) info forks
>  * 2 process 15934 at 0x40055e, file foll-fork.c, line 24
>   1 process 15927 at 0x7ffff789ac4b, <fork>
>
>>./gdb -ex "set follow-fork-mode parent" -ex "set detach-on-fork off" ./testsuite/gdb.base/foll-fork
> ...
>  (gdb) info forks
>   1 process 15979 at 0x7ffff789ac4b, <fork>
>  * 0 process 15974 (main process) at 0x40055e, file foll-fork.c, line 24
>
> A bit more uniform, and the hang bug goes away, of course.  That special
> casing to have a fork number 0 could go away too, IMVHO.
>
> Tested on x86_64-linux-gnu, no regressions.
>
> --
> Pedro Alves
>