This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] Fix signal handler/event-loop races


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5cc3ce8b5fffa7413557b7e071d8471ae6e2fc88

commit 5cc3ce8b5fffa7413557b7e071d8471ae6e2fc88
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Apr 12 16:49:30 2016 +0100

    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.

Diff:
---
 gdb/ChangeLog    | 13 +++++++++++++
 gdb/event-loop.c | 32 +++++++++++++++++++++++++++++++-
 gdb/event-loop.h |  2 ++
 gdb/event-top.c  |  2 ++
 4 files changed, 48 insertions(+), 1 deletion(-)

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]