diff options
author | Pedro Alves <palves@redhat.com> | 2016-04-12 16:49:30 +0100 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2016-04-12 16:53:40 +0100 |
commit | 5cc3ce8b5fffa7413557b7e071d8471ae6e2fc88 (patch) | |
tree | 2ef4aa7b8103afe6c707205fb0f06328dc5222c9 | |
parent | 00340e1b916fa2d040439b101c220fae3c5834fa (diff) | |
download | gdb-5cc3ce8b5fffa7413557b7e071d8471ae6e2fc88.zip gdb-5cc3ce8b5fffa7413557b7e071d8471ae6e2fc88.tar.gz gdb-5cc3ce8b5fffa7413557b7e071d8471ae6e2fc88.tar.bz2 |
Fix signal handler/event-loop races
GDB's core signal handling suffers from a classical signal handler /
mainline code race:
int
gdb_do_one_event (void)
{
...
/* First let's see if there are any asynchronous signal handlers
that are ready. These would be the result of invoking any of the
signal handlers. */
if (invoke_async_signal_handlers ())
return 1;
...
/* Block waiting for a new event. (...). */
if (gdb_wait_for_event (1) < 0)
return -1;
...
}
If a signal is delivered while gdb is blocked in the poll/select
inside gdb_wait_for_event, then the select/poll breaks with EINTR,
we'll loop back around and call invoke_async_signal_handlers.
However, if the signal handler runs between
invoke_async_signal_handlers and gdb_wait_for_event,
gdb_wait_for_event will block, until the next unrelated event...
The fix is to a struct serial_event, and register it in the set of
files that select/poll in gdb_wait_for_event waits on. The signal
handlers that defer work to invoke_async_signal_handlers call
mark_async_signal_handler, which is adjusted to also set the new
serial event in addition to setting a flag, and is thus now is
garanteed to immediately unblock the next gdb_select/poll call, up
until invoke_async_signal_handlers is called and the event is cleared.
gdb/ChangeLog:
2016-04-12 Pedro Alves <palves@redhat.com>
* event-loop.c: Include "ser-event.h".
(async_signal_handlers_serial_event): New global.
(async_signals_handler, initialize_async_signal_handlers): New
functions.
(mark_async_signal_handler): Set
async_signal_handlers_serial_event.
(invoke_async_signal_handlers): Clear
async_signal_handlers_serial_event.
* event-top.c (async_init_signals): Call
initialize_async_signal_handlers.
-rw-r--r-- | gdb/ChangeLog | 13 | ||||
-rw-r--r-- | gdb/event-loop.c | 32 | ||||
-rw-r--r-- | gdb/event-loop.h | 2 | ||||
-rw-r--r-- | gdb/event-top.c | 2 |
4 files changed, 48 insertions, 1 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 55fd901..0b79ce1 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,18 @@ 2016-04-12 Pedro Alves <palves@redhat.com> + * event-loop.c: Include "ser-event.h". + (async_signal_handlers_serial_event): New global. + (async_signals_handler, initialize_async_signal_handlers): New + functions. + (mark_async_signal_handler): Set + async_signal_handlers_serial_event. + (invoke_async_signal_handlers): Clear + async_signal_handlers_serial_event. + * event-top.c (async_init_signals): Call + initialize_async_signal_handlers. + +2016-04-12 Pedro Alves <palves@redhat.com> + * Makefile.in (SFILES): Add ser-event.c. (HFILES_NO_SRCDIR): Add ser-event.h. (COMMON_OBS): Add ser-event.o. diff --git a/gdb/event-loop.c b/gdb/event-loop.c index 0e1cb2b..052d535 100644 --- a/gdb/event-loop.c +++ b/gdb/event-loop.c @@ -21,6 +21,7 @@ #include "event-loop.h" #include "event-top.h" #include "queue.h" +#include "ser-event.h" #ifdef HAVE_POLL #if defined (HAVE_POLL_H) @@ -262,6 +263,28 @@ static int update_wait_timeout (void); static int poll_timers (void); +/* This event is signalled whenever an asynchronous handler needs to + defer an action to the event loop. */ +static struct serial_event *async_signal_handlers_serial_event; + +/* Callback registered with ASYNC_SIGNAL_HANDLERS_SERIAL_EVENT. */ + +static void +async_signals_handler (int error, gdb_client_data client_data) +{ + /* Do nothing. Handlers are run by invoke_async_signal_handlers + from instead. */ +} + +void +initialize_async_signal_handlers (void) +{ + async_signal_handlers_serial_event = make_serial_event (); + + add_file_handler (serial_event_fd (async_signal_handlers_serial_event), + async_signals_handler, NULL); +} + /* Process one high level event. If nothing is ready at this time, wait for something to happen (via gdb_wait_for_event), then process it. Returns >0 if something was done otherwise returns <0 (this @@ -905,6 +928,7 @@ void mark_async_signal_handler (async_signal_handler * async_handler_ptr) { async_handler_ptr->ready = 1; + serial_event_set (async_signal_handlers_serial_event); } /* See event-loop.h. */ @@ -925,13 +949,19 @@ async_signal_handler_is_marked (async_signal_handler *async_handler_ptr) /* Call all the handlers that are ready. Returns true if any was indeed ready. */ + static int invoke_async_signal_handlers (void) { async_signal_handler *async_handler_ptr; int any_ready = 0; - /* Invoke ready handlers. */ + /* We're going to handle all pending signals, so no need to wake up + the event loop again the next time around. Note this must be + cleared _before_ calling the callbacks, to avoid races. */ + serial_event_clear (async_signal_handlers_serial_event); + + /* Invoke all ready handlers. */ while (1) { diff --git a/gdb/event-loop.h b/gdb/event-loop.h index 1b765e1..155dafa 100644 --- a/gdb/event-loop.h +++ b/gdb/event-loop.h @@ -143,3 +143,5 @@ extern void mark_async_event_handler (struct async_event_handler *handler); /* Mark the handler (ASYNC_HANDLER_PTR) as NOT ready. */ extern void clear_async_event_handler (struct async_event_handler *handler); + +extern void initialize_async_signal_handlers (void); diff --git a/gdb/event-top.c b/gdb/event-top.c index a119fe9..2f0a758 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -748,6 +748,8 @@ gdb_readline_no_editing_callback (gdb_client_data client_data) void async_init_signals (void) { + initialize_async_signal_handlers (); + signal (SIGINT, handle_sigint); sigint_token = create_async_signal_handler (async_request_quit, NULL); |