This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: SIGCHLD ignored
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: Vladimir Prus <ghost at cs dot msu dot su>, Hamish Rodda <rodda at kde dot org>, Daniel Jacobowitz <dan at codesourcery dot com>
- Date: Thu, 12 Jun 2008 00:18:05 +0100
- Subject: Re: SIGCHLD ignored
- References: <200806112121.06783.ghost@cs.msu.su> <200806112345.22321.pedro@codesourcery.com>
[moved to gdb-patches@]
Original report here:
SIGCHLD ignored
http://sourceware.org/ml/gdb/2008-06/msg00099.html
A Wednesday 11 June 2008 23:45:22, Pedro Alves wrote:
> A Wednesday 11 June 2008 18:21:05, Vladimir Prus wrote:
> > Pedro, I think this SIGCHLD magic is your doing -- do you have any ideas
> > how to fix it?
>
> Dang, I totally missed this could be a problem.
>
> Right, so, normal_mask has SIGCHLD blocked in _initialize_linux_nat,
> and since forking a child inherits the signal mask, the child gets
> SIGCHLD blocked by default. This would have gone unnoticed
> if Qt unblocked SIGCHLD in it's setup (one may argue it should).
>
> It's not safe to just remove that SIGCHLD blocking, we're not
> taking care of blocking it in places we used to in sync mode
> (we used to block it the first time we hit linux_nat_wait, which
> is reached after forking an inferior).
>
> For async mode, we also need to do something about SIGCHLD
> blocking. We have:
>
> linux_nat_create_inferior
> -> linux_nat_async_mask
> -> linux_nat_async
> -> linux_nat_async_events
> -> block SIGCHLD.
>
> -> linux_ops->to_create_inferior (forks a child)
>
> Attached is a patch to fix this.
>
> Basically, I turned linux_nat_async_events_enabled into a
> 3-state instead of a bool, with the new state being
> "SIGCHLD unblocked with action set to whatever we get
> on startup". Most of the SIGCHLD state management stays with
> the same logic, except that linux_nat_create_inferior gets a
> switch to the new state before forking a child, and
> linux_nat_wait, gets an unconditional switch to the blocked
> state. The rest of the patch is mostly updating to the
> new interface.
>
> Tested on x86-64-unknown-linux-gnu sync and async modes
> without regressions.
Same code patch, but updated with a new testcase. I imagine
we're going to touch these issues again with full
multi-process support.
--
Pedro Alves
gdb/
2008-06-12 Pedro Alves <pedro@codesourcery.com>
* linux-nat.c (enum sigchld_state): New.
(linux_nat_async_events_state): Renamed from
linux_nat_async_events_enabled.
(linux_nat_event_pipe_push, my_waitpid): Adjust.
(sigchld_default_action): New.
(lin_lwp_attach_lwp): Adjust. Call linux_nat_async_events
unconditionally.
(linux_nat_create_inferior): Set events state to sigchld_default
state.
(linux_nat_resume): Adjust.
(linux_nat_wait): Call linux_nat_async_events unconditionally.
(sigchld_handler): Adjust.
(linux_nat_async_mask): Don't set SIGCHLD actions here.
(get_pending_events): Adjust.
(linux_nat_async_events): Rewrite to handle enum sigchld_state
instead of a boolean.
(linux_nat_async): Adjust.
(_initialize_linux_nat): Capture default SIGCHLD action into
sigchld_default_action.
gdb/testsuite/
2008-06-12 Pedro Alves <pedro@codesourcery.com>
* gdb.base/sigchld.c, gdb.base/sigchld.exp: New test.
---
gdb/linux-nat.c | 154 +++++++++++++++++++++++--------------
gdb/testsuite/gdb.base/sigchld.c | 37 ++++++++
gdb/testsuite/gdb.base/sigchld.exp | 45 ++++++++++
3 files changed, 179 insertions(+), 57 deletions(-)
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c 2008-06-11 23:37:14.000000000 +0100
+++ src/gdb/linux-nat.c 2008-06-12 00:12:14.000000000 +0100
@@ -196,11 +196,24 @@ static int linux_nat_event_pipe[2] = { -
/* Number of queued events in the pipe. */
static volatile int linux_nat_num_queued_events;
-/* If async mode is on, true if we're listening for events; false if
- target events are blocked. */
-static int linux_nat_async_events_enabled;
+/* The possible SIGCHLD handling states. */
-static int linux_nat_async_events (int enable);
+enum sigchld_state
+{
+ /* SIGCHLD disabled, with action set to sigchld_handler, for the
+ sigsuspend in linux_nat_wait. */
+ sigchld_sync,
+ /* SIGCHLD enabled, with action set to async_sigchld_handler. */
+ sigchld_async,
+ /* Set SIGCHLD to default action. Used while creating an
+ inferior. */
+ sigchld_default
+};
+
+/* The current SIGCHLD handling state. */
+static enum sigchld_state linux_nat_async_events_state;
+
+static enum sigchld_state linux_nat_async_events (enum sigchld_state enable);
static void pipe_to_local_event_queue (void);
static void local_event_queue_to_pipe (void);
static void linux_nat_event_pipe_push (int pid, int status, int options);
@@ -234,8 +247,8 @@ queued_waitpid (int pid, int *status, in
if (debug_linux_nat_async)
fprintf_unfiltered (gdb_stdlog,
"\
-QWPID: linux_nat_async_events_enabled(%d), linux_nat_num_queued_events(%d)\n",
- linux_nat_async_events_enabled,
+QWPID: linux_nat_async_events_state(%d), linux_nat_num_queued_events(%d)\n",
+ linux_nat_async_events_state,
linux_nat_num_queued_events);
if (flags & __WALL)
@@ -381,7 +394,7 @@ my_waitpid (int pid, int *status, int fl
int ret;
/* There should be no concurrent calls to waitpid. */
- gdb_assert (!linux_nat_async_events_enabled);
+ gdb_assert (linux_nat_async_events_state != sigchld_async);
ret = queued_waitpid (pid, status, flags);
if (ret != -1)
@@ -813,6 +826,9 @@ struct sigaction sync_sigchld_action;
/* SIGCHLD action for asynchronous mode. */
static struct sigaction async_sigchld_action;
+
+/* SIGCHLD default action, to pass to new inferiors. */
+static struct sigaction sigchld_default_action;
/* Prototypes for local functions. */
@@ -1136,12 +1152,11 @@ int
lin_lwp_attach_lwp (ptid_t ptid)
{
struct lwp_info *lp;
- int async_events_were_enabled = 0;
+ enum sigchld_state async_events_original_state;
gdb_assert (is_lwp (ptid));
- if (target_can_async_p ())
- async_events_were_enabled = linux_nat_async_events (0);
+ async_events_original_state = linux_nat_async_events (sigchld_sync);
lp = find_lwp_pid (ptid);
@@ -1206,9 +1221,7 @@ lin_lwp_attach_lwp (ptid_t ptid)
lp->stopped = 1;
}
- if (async_events_were_enabled)
- linux_nat_async_events (1);
-
+ linux_nat_async_events (async_events_original_state);
return 0;
}
@@ -1222,6 +1235,8 @@ linux_nat_create_inferior (char *exec_fi
we have to mask the async mode. */
if (target_can_async_p ())
+ /* Mask async mode. Creating a child requires a loop calling
+ wait_for_inferior currently. */
saved_async = linux_nat_async_mask (0);
else
{
@@ -1232,6 +1247,12 @@ linux_nat_create_inferior (char *exec_fi
sigdelset (&suspend_mask, SIGCHLD);
}
+ /* Set SIGCHLD to the default action, until after execing the child,
+ since the inferior inherits the superior's signal mask. It will
+ be blocked again in linux_nat_wait, which is only reached after
+ the inferior execing. */
+ linux_nat_async_events (sigchld_default);
+
linux_ops->to_create_inferior (exec_file, allargs, env, from_tty);
if (saved_async)
@@ -1463,7 +1484,7 @@ linux_nat_resume (ptid_t ptid, int step,
if (target_can_async_p ())
/* Block events while we're here. */
- linux_nat_async_events (0);
+ linux_nat_async_events (sigchld_sync);
/* A specific PTID means `step only this process id'. */
resume_all = (PIDGET (ptid) == -1);
@@ -2525,9 +2546,8 @@ linux_nat_wait (ptid_t ptid, struct targ
sigemptyset (&flush_mask);
- if (target_can_async_p ())
- /* Block events while we're here. */
- target_async (NULL, 0);
+ /* Block events while we're here. */
+ linux_nat_async_events (sigchld_sync);
retry:
@@ -2986,7 +3006,7 @@ static void
sigchld_handler (int signo)
{
if (linux_nat_async_enabled
- && linux_nat_async_events_enabled
+ && linux_nat_async_events_state != sigchld_sync
&& signo == SIGCHLD)
/* It is *always* a bug to hit this. */
internal_error (__FILE__, __LINE__,
@@ -3845,15 +3865,9 @@ linux_nat_async_mask (int mask)
{
linux_nat_async (NULL, 0);
linux_nat_async_mask_value = mask;
- /* We're in sync mode. Make sure SIGCHLD isn't handled by
- async_sigchld_handler when we come out of sigsuspend in
- linux_nat_wait. */
- sigaction (SIGCHLD, &sync_sigchld_action, NULL);
}
else
{
- /* Restore the async handler. */
- sigaction (SIGCHLD, &async_sigchld_action, NULL);
linux_nat_async_mask_value = mask;
linux_nat_async (inferior_event_handler, 0);
}
@@ -3911,7 +3925,8 @@ get_pending_events (void)
{
int status, options, pid;
- if (!linux_nat_async_enabled || !linux_nat_async_events_enabled)
+ if (!linux_nat_async_enabled
+ || linux_nat_async_events_state != sigchld_async)
internal_error (__FILE__, __LINE__,
"get_pending_events called with async masked");
@@ -3965,44 +3980,71 @@ async_sigchld_handler (int signo)
get_pending_events ();
}
-/* Enable or disable async SIGCHLD handling. */
+/* Set SIGCHLD handling state to STATE. Returns previous state. */
-static int
-linux_nat_async_events (int enable)
+static enum sigchld_state
+linux_nat_async_events (enum sigchld_state state)
{
- int current_state = linux_nat_async_events_enabled;
+ enum sigchld_state current_state = linux_nat_async_events_state;
if (debug_linux_nat_async)
fprintf_unfiltered (gdb_stdlog,
- "LNAE: enable(%d): linux_nat_async_events_enabled(%d), "
+ "LNAE: state(%d): linux_nat_async_events_state(%d), "
"linux_nat_num_queued_events(%d)\n",
- enable, linux_nat_async_events_enabled,
+ state, linux_nat_async_events_state,
linux_nat_num_queued_events);
- if (current_state != enable)
+ if (current_state != state)
{
sigset_t mask;
sigemptyset (&mask);
sigaddset (&mask, SIGCHLD);
- if (enable)
- {
- /* Unblock target events. */
- linux_nat_async_events_enabled = 1;
- local_event_queue_to_pipe ();
- /* While in masked async, we may have not collected all the
- pending events. Get them out now. */
- get_pending_events ();
- sigprocmask (SIG_UNBLOCK, &mask, NULL);
- }
- else
+ linux_nat_async_events_state = state;
+
+ switch (state)
{
- /* Block target events. */
- sigprocmask (SIG_BLOCK, &mask, NULL);
- linux_nat_async_events_enabled = 0;
- /* Get events out of queue, and make them available to
- queued_waitpid / my_waitpid. */
- pipe_to_local_event_queue ();
+ case sigchld_sync:
+ {
+ /* Block target events. */
+ sigprocmask (SIG_BLOCK, &mask, NULL);
+ sigaction (SIGCHLD, &sync_sigchld_action, NULL);
+ /* Get events out of queue, and make them available to
+ queued_waitpid / my_waitpid. */
+ pipe_to_local_event_queue ();
+ }
+ break;
+ case sigchld_async:
+ {
+ /* Unblock target events for async mode. */
+
+ sigprocmask (SIG_BLOCK, &mask, NULL);
+
+ /* Put events we already waited on, in the pipe first, so
+ events are FIFO. */
+ local_event_queue_to_pipe ();
+ /* While in masked async, we may have not collected all
+ the pending events. Get them out now. */
+ get_pending_events ();
+
+ /* Let'em come. */
+ sigaction (SIGCHLD, &async_sigchld_action, NULL);
+ sigprocmask (SIG_UNBLOCK, &mask, NULL);
+ }
+ break;
+ case sigchld_default:
+ {
+ /* SIGCHLD default mode. */
+ sigaction (SIGCHLD, &sigchld_default_action, NULL);
+
+ /* Get events out of queue, and make them available to
+ queued_waitpid / my_waitpid. */
+ pipe_to_local_event_queue ();
+
+ /* Unblock SIGCHLD. */
+ sigprocmask (SIG_UNBLOCK, &mask, NULL);
+ }
+ break;
}
}
@@ -4094,14 +4136,14 @@ linux_nat_async (void (*callback) (enum
add_file_handler (linux_nat_event_pipe[0],
linux_nat_async_file_handler, NULL);
- linux_nat_async_events (1);
+ linux_nat_async_events (sigchld_async);
}
else
{
async_client_callback = callback;
async_client_context = context;
- linux_nat_async_events (0);
+ linux_nat_async_events (sigchld_sync);
delete_file_handler (linux_nat_event_pipe[0]);
}
return;
@@ -4117,21 +4159,15 @@ linux_nat_set_async_mode (int on)
if (on)
{
gdb_assert (waitpid_queue == NULL);
- sigaction (SIGCHLD, &async_sigchld_action, NULL);
-
if (pipe (linux_nat_event_pipe) == -1)
internal_error (__FILE__, __LINE__,
"creating event pipe failed.");
-
fcntl (linux_nat_event_pipe[0], F_SETFL, O_NONBLOCK);
fcntl (linux_nat_event_pipe[1], F_SETFL, O_NONBLOCK);
}
else
{
- sigaction (SIGCHLD, &sync_sigchld_action, NULL);
-
drain_queued_events (-1);
-
linux_nat_num_queued_events = 0;
close (linux_nat_event_pipe[0]);
close (linux_nat_event_pipe[1]);
@@ -4248,6 +4284,10 @@ Tells gdb whether to control the GNU/Lin
&maintenance_set_cmdlist,
&maintenance_show_cmdlist);
+ /* Get the default SIGCHLD action. Used while forking an inferior
+ (see linux_nat_create_inferior/linux_nat_async_events). */
+ sigaction (SIGCHLD, NULL, &sigchld_default_action);
+
/* Block SIGCHLD by default. Doing this early prevents it getting
unblocked if an exception is thrown due to an error while the
inferior is starting (sigsetjmp/siglongjmp). */
Index: src/gdb/testsuite/gdb.base/sigchld.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/sigchld.c 2008-06-12 00:05:36.000000000 +0100
@@ -0,0 +1,37 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2008 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/>.
+*/
+
+/* Check that GDB isn't messing the SIGCHLD mask while creating an
+ inferior. */
+
+#include <signal.h>
+#include <stdlib.h>
+
+int
+main ()
+{
+ sigset_t mask;
+
+ sigemptyset (&mask);
+ sigprocmask (SIG_BLOCK, NULL, &mask);
+
+ if (!sigismember (&mask, SIGCHLD))
+ return 0; /* good, not blocked */
+ else
+ return 1; /* bad, blocked */
+}
Index: src/gdb/testsuite/gdb.base/sigchld.exp
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/sigchld.exp 2008-06-12 00:10:54.000000000 +0100
@@ -0,0 +1,45 @@
+# Copyright (C) 2008 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/>.
+
+# Check that GDB isn't messing the SIGCHLD mask while creating an
+# inferior.
+
+if [target_info exists gdb,nosignals] {
+ verbose "Skipping sigchld.exp because of nosignals."
+ continue
+}
+
+set testfile "sigchld"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+runto_main
+
+gdb_test "b [gdb_get_line_number "good, not blocked"]" \
+ ".*Breakpoint .*sigchld.*" "set breakpoint at success exit"
+
+gdb_test "b [gdb_get_line_number "bad, blocked"]" \
+ ".*Breakpoint .*sigchld.*" "set breakpoint at failure exit"
+
+gdb_test "continue" ".*good, not blocked.*" "SIGCHLD blocked in inferior"