[PATCH 08/30] Fix signal handler/event-loop races

Pedro Alves
2016-03-18T19:25:38Z

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:
yyyy-mm-dd 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.
---
gdb/event-loop.c | 32 +++++++++++++++++++++++++++++++-
gdb/event-loop.h | 2 ++
gdb/event-top.c | 2 ++
3 files changed, 35 insertions(+), 1 deletion(-)

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);
\f

+/* 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 769485f..9fff2be 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -747,6 +747,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);
--
2.5.0