gdb: register signal handler after setting up event token
authorAndrew Burgess <andrew.burgess@embecosm.com>
Tue, 15 Jun 2021 11:59:34 +0000 (12:59 +0100)
committerAndrew Burgess <andrew.burgess@embecosm.com>
Wed, 11 Aug 2021 11:35:14 +0000 (12:35 +0100)
This commit fixes the smallest of small possible bug related to signal
handling.  If we look in async_init_signals we see code like this:

  signal (SIGQUIT, handle_sigquit);
  sigquit_token =
    create_async_signal_handler (async_do_nothing, NULL, "sigquit");

Then if we look in handle_sigquit we see code like this:

  mark_async_signal_handler (sigquit_token);
  signal (sig, handle_sigquit);

Finally, in mark_async_signal_handler we have:

  async_handler_ptr->ready = 1;

Where async_handler_ptr will be sigquit_token.

What this means is that if a SIGQUIT arrive in async_init_signals
after handle_sigquit has been registered, but before sigquit_token has
been initialised, then GDB will most likely crash.

The chance of this happening is tiny, but fixing this is trivial, just
ensure we call create_async_signal_handler before calling signal, so
lets do that.

There are no tests for this.  Trying to land a signal in the right
spot is pretty hit and miss.  I did try changing the current HEAD GDB
like this:

  signal (SIGQUIT, handle_sigquit);
  raise (SIGQUIT);
  sigquit_token =
    create_async_signal_handler (async_do_nothing, NULL, "sigquit");

And confirmed that this did result in a crash, after my change I tried
this:

  sigquit_token =
    create_async_signal_handler (async_do_nothing, NULL, "sigquit");
  signal (SIGQUIT, handle_sigquit);
  raise (SIGQUIT);

And GDB now starts up just fine.

gdb/ChangeLog:

* event-top.c (async_init_signals): For each signal, call signal
only after calling create_async_signal_handler.

gdb/event-top.c

index ab5179b7d326d24500af1c960b1287dd52b6bf2c..2d3bfa6a9c99541254deb096fa3bd02f9be6438a 100644 (file)
@@ -918,12 +918,13 @@ async_init_signals (void)
 
   quit_serial_event = make_serial_event ();
 
-  signal (SIGINT, handle_sigint);
   sigint_token =
     create_async_signal_handler (async_request_quit, NULL, "sigint");
-  signal (SIGTERM, handle_sigterm);
+  signal (SIGINT, handle_sigint);
+
   async_sigterm_token
     = create_async_signal_handler (async_sigterm_handler, NULL, "sigterm");
+  signal (SIGTERM, handle_sigterm);
 
   /* If SIGTRAP was set to SIG_IGN, then the SIG_IGN will get passed
      to the inferior and breakpoints will be ignored.  */
@@ -940,10 +941,11 @@ async_init_signals (void)
      might be in memory, shared between the two).  Since we establish
      a handler for SIGQUIT, when we call exec it will set the signal
      to SIG_DFL for us.  */
-  signal (SIGQUIT, handle_sigquit);
   sigquit_token =
     create_async_signal_handler (async_do_nothing, NULL, "sigquit");
+  signal (SIGQUIT, handle_sigquit);
 #endif
+
 #ifdef SIGHUP
   if (signal (SIGHUP, handle_sighup) != SIG_IGN)
     sighup_token =