[BUG] gdb: quit hangs after step into signal handler

[BUG] gdb: quit hangs after step into signal handler

[Oleg Nesterov]

In short, gdb uses ptrace(PTRACE_KILL) + wait() to detach/kill the tracee.
Well, I think PTRACE_KILL should be avoided but this is offtopic,
ptrace(PTRACE_CONT, SIGKILL) has the same problem if the tracee has
stepped into the signal handler.

	$ gdb /usr/bin/perl
	GNU gdb (GDB) 7.1
	...
	(gdb)
	(gdb) set args -e '$SIG{HUP}=sub{}; kill HUP, $$; 1 while 1'

(this sets a signal handler for SIGHUP, sends SIGHUP to itself, then
 spins forever)

	(gdb) r
	Starting program: /usr/bin/perl -e '$SIG{HUP}=sub{}; kill HUP, $$; 1 while 1'
	[Thread debugging using libthread_db enabled]

	Program received signal SIGHUP, Hangup.
	0x000000375fa32507 in kill () from /lib64/libc.so.6
	(gdb) si
	0x00000037522a2a40 in Perl_csighandler () from /usr/lib64/perl5/5.10.0/x86_64-linux-thread-multi/CORE/libperl.so

(step into handler)

	(gdb) q
	A debugging session is active.

		Inferior 1 [process 9694] will be killed.

	Quit anyway? (y or n) y

(ptrace(PTRACE_KILL) + wait)

Hangs "forever" in wait4().

At first I thought this is another bug in ptrace-utrace (and I even
started doing the patch), but then I recalled that this is what
upstream does, and ptrace-utrace just tries to be compatible.

There is no signal context after step-into-handler, ptrace(PTRACE_KILL)
or ptrace(PTRACE_CONT, SIGKILL) acts like ptrace(CONT, pid, 0,0) and
just resumes the tracee.

Probably gdb should just do kill(tracee, SIGKILL) when it wants
to terminate the tracee.

Oleg.

Re: [BUG] gdb: quit hangs after step into signal handler

[Roland McGrath]

> Probably gdb should just do kill(tracee, SIGKILL) when it wants
> to terminate the tracee.

That's what I recommend.  (You only need one SIGKILL per process, not one
for each thread.  But if you still support linuxthreads, that will probably
need one for each thread.)  Even if you were paranoid about some old kernel
where PTRACE_KILL might work better (dubious if there are any such, but
that's why it's paranoia), you could do this before PTRACE_KILL and it
should certainly be fine everywhere.


Thanks,
Roland

[patch] Re: [BUG] gdb: quit hangs after step into signal handler

[Jan Kratochvil]

On Wed, 22 Sep 2010 01:53:56 +0200, Roland McGrath wrote:
> Even if you were paranoid about some old kernel where PTRACE_KILL might work
> better (dubious if there are any such, but that's why it's paranoia), you
> could do this before PTRACE_KILL and it should certainly be fine everywhere.

Problem is the inferior will start running after PTRACE_KILL.  Current GDB:

Quit anyway? (y or n) y
<hang by sleep (600);>

read(0, "y\n", 1024)                    = 2
ptrace(PTRACE_KILL, 32336, 0, 0)        = 0
wait4(32336,

Before kill (SIGKILL) gets applied the code may do something unexpected.
Why do you consider SIGKILL first, PTRACE_KILL second as worse?

(Probably not so important now.)


Thanks,
Jan


gdb/
2011-02-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-nat.c (kill_callback): Use SIGKILL first.

gdb/testsuite/
2011-02-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/kill-after-signal.c: New file.
	* gdb.base/kill-after-signal.exp: New file.

--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3802,6 +3802,18 @@ linux_nat_wait (struct target_ops *ops,
 static int
 kill_callback (struct lwp_info *lp, void *data)
 {
+  /* PTRACE_KILL may resume the inferior.  Send SIGKILL first.  */
+
+  errno = 0;
+  kill (GET_LWP (lp->ptid), SIGKILL);
+  if (debug_linux_nat)
+    fprintf_unfiltered (gdb_stdlog,
+			"KC:  kill (SIGKILL) %s, 0, 0 (%s)\n",
+			target_pid_to_str (lp->ptid),
+			errno ? safe_strerror (errno) : "OK");
+
+  /* Some kernels ignore even SIGKILL for processes under ptrace.  */
+
   errno = 0;
   ptrace (PTRACE_KILL, GET_LWP (lp->ptid), 0, 0);
   if (debug_linux_nat)
--- /dev/null
+++ b/gdb/testsuite/gdb.base/kill-after-signal.c
@@ -0,0 +1,37 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <signal.h>
+#include <assert.h>
+#include <unistd.h>
+#include <stdio.h>
+
+void
+handler (int signo)
+{
+  sleep (600);
+  assert (0);
+}
+
+int
+main (void)
+{
+  signal (SIGUSR1, handler);
+  raise (SIGUSR1);
+  assert (0);
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/kill-after-signal.exp
@@ -0,0 +1,29 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile "kill-after-signal"
+if [prepare_for_testing ${testfile}.exp ${testfile}] {
+    return -1
+}
+
+gdb_test "handle SIGUSR1 stop print pass" "SIGUSR1\[ \t\]+Yes\[ \t\]+Yes\[ \t\]+Yes\[ \t\]+.*"
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "continue" "Program received signal SIGUSR1, .*"
+gdb_test "stepi" "\r\nhandler .*"
+gdb_test "kill" "^y" "kill" "Kill the program being debugged\\? \\(y or n\\) $" "y"

Re: [patch] Re: [BUG] gdb: quit hangs after step into signal handler

[Roland McGrath]

> On Wed, 22 Sep 2010 01:53:56 +0200, Roland McGrath wrote:
> > Even if you were paranoid about some old kernel where PTRACE_KILL might work
> > better (dubious if there are any such, but that's why it's paranoia), you
> > could do this before PTRACE_KILL and it should certainly be fine everywhere.
> 
> Problem is the inferior will start running after PTRACE_KILL.  Current GDB:
> 
> Quit anyway? (y or n) y
> <hang by sleep (600);>
> 
> read(0, "y\n", 1024)                    = 2
> ptrace(PTRACE_KILL, 32336, 0, 0)        = 0
> wait4(32336,
> 
> Before kill (SIGKILL) gets applied the code may do something unexpected.
> Why do you consider SIGKILL first, PTRACE_KILL second as worse?

What I recommended is kill/SIGKILL first, PTRACE_KILL second.  
That's not worse than anything else.