From bbefac7df96061a56dc4f17ef548382bdf4c3166 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Tue, 15 Jun 2021 12:59:34 +0100 Subject: [PATCH] gdb: register signal handler after setting up event token 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 | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/gdb/event-top.c b/gdb/event-top.c index ab5179b7d32..2d3bfa6a9c9 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -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 = -- 2.30.2