The gdb demangler installs a SIGSEGV handler in order to protect gdb
from demangler bugs. However, this is not thread-safe, as signal
handlers are global to the process.
This patch changes gdb to always install a global SIGSEGV handler, and
then lets threads indicate their interest in handling the signal by
setting a thread-local variable.
This patch then arranges for the demangler code to use this; being
sure to arrange for calls to warning and the like to be done on the
main thread.
One thing I wondered while writing this patch is if there are any
systems that do not have "sigaction". If gdb could assume this, it
would simplify this code.
gdb/ChangeLog
2019-10-01 Tom Tromey <tom@tromey.com>
* event-top.h (thread_local_segv_handler): Declare.
* event-top.c (thread_local_segv_handler): New global.
(install_handle_sigsegv, handle_sigsegv): New functions.
(async_init_signals): Install SIGSEGV handler.
* cp-support.c (gdb_demangle_jmp_buf): Change type. Now
thread-local.
(report_failed_demangle): New function.
(gdb_demangle): Make core_dump_allowed atomic. Remove signal
handler-setting code, instead use segv_handler. Run warning code
on main thread.
---
gdb/ChangeLog | 13 +++++
gdb/cp-support.c | 143 +++++++++++++++++++++++------------------------
gdb/event-top.c | 41 ++++++++++++++
gdb/event-top.h | 6 ++
4 files changed, 130 insertions(+), 73 deletions(-)
@@ -37,6 +37,9 @@
#include "gdbsupport/gdb_setjmp.h"
#include "safe-ctype.h"
#include "gdbsupport/selftest.h"
+#include <atomic>
+#include "event-top.h"
+#include "ser-event.h"
#define d_left(dc) (dc)->u.s_binary.left
#define d_right(dc) (dc)->u.s_binary.right
@@ -1476,11 +1479,11 @@ static bool catch_demangler_crashes = true;
/* Stack context and environment for demangler crash recovery. */
-static SIGJMP_BUF gdb_demangle_jmp_buf;
+static thread_local SIGJMP_BUF *gdb_demangle_jmp_buf;
/* If nonzero, attempt to dump core from the signal handler. */
-static int gdb_demangle_attempt_core_dump = 1;
+static std::atomic<bool> gdb_demangle_attempt_core_dump;
/* Signal handler for gdb_demangle. */
@@ -1492,10 +1495,46 @@ gdb_demangle_signal_handler (int signo)
if (fork () == 0)
dump_core ();
- gdb_demangle_attempt_core_dump = 0;
+ gdb_demangle_attempt_core_dump = false;
}
- SIGLONGJMP (gdb_demangle_jmp_buf, signo);
+ SIGLONGJMP (*gdb_demangle_jmp_buf, signo);
+}
+
+/* A helper for gdb_demangle that reports a demangling failure. */
+
+static void
+report_failed_demangle (const char *name, bool core_dump_allowed,
+ int crash_signal)
+{
+ static bool error_reported = false;
+
+ if (!error_reported)
+ {
+ std::string short_msg
+ = string_printf (_("unable to demangle '%s' "
+ "(demangler failed with signal %d)"),
+ name, crash_signal);
+
+ std::string long_msg
+ = string_printf ("%s:%d: %s: %s", __FILE__, __LINE__,
+ "demangler-warning", short_msg.c_str ());
+
+ target_terminal::scoped_restore_terminal_state term_state;
+ target_terminal::ours_for_output ();
+
+ begin_line ();
+ if (core_dump_allowed)
+ fprintf_unfiltered (gdb_stderr,
+ _("%s\nAttempting to dump core.\n"),
+ long_msg.c_str ());
+ else
+ warn_cant_dump_core (long_msg.c_str ());
+
+ demangler_warning (__FILE__, __LINE__, "%s", short_msg.c_str ());
+
+ error_reported = true;
+ }
}
#endif
@@ -1509,38 +1548,18 @@ gdb_demangle (const char *name, int options)
int crash_signal = 0;
#ifdef HAVE_WORKING_FORK
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
- struct sigaction sa, old_sa;
-#else
- sighandler_t ofunc;
-#endif
- static int core_dump_allowed = -1;
-
- if (core_dump_allowed == -1)
- {
- core_dump_allowed = can_dump_core (LIMIT_CUR);
-
- if (!core_dump_allowed)
- gdb_demangle_attempt_core_dump = 0;
- }
-
+ scoped_restore restore_segv
+ = make_scoped_restore (&thread_local_segv_handler,
+ catch_demangler_crashes
+ ? gdb_demangle_signal_handler
+ : nullptr);
+
+ bool core_dump_allowed = gdb_demangle_attempt_core_dump;
+ SIGJMP_BUF jmp_buf;
+ scoped_restore restore_jmp_buf
+ = make_scoped_restore (&gdb_demangle_jmp_buf, &jmp_buf);
if (catch_demangler_crashes)
- {
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
- sa.sa_handler = gdb_demangle_signal_handler;
- sigemptyset (&sa.sa_mask);
-#ifdef HAVE_SIGALTSTACK
- sa.sa_flags = SA_ONSTACK;
-#else
- sa.sa_flags = 0;
-#endif
- sigaction (SIGSEGV, &sa, &old_sa);
-#else
- ofunc = signal (SIGSEGV, gdb_demangle_signal_handler);
-#endif
-
- crash_signal = SIGSETJMP (gdb_demangle_jmp_buf);
- }
+ crash_signal = SIGSETJMP (jmp_buf);
#endif
if (crash_signal == 0)
@@ -1549,45 +1568,20 @@ gdb_demangle (const char *name, int options)
#ifdef HAVE_WORKING_FORK
if (catch_demangler_crashes)
{
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
- sigaction (SIGSEGV, &old_sa, NULL);
-#else
- signal (SIGSEGV, ofunc);
-#endif
-
if (crash_signal != 0)
- {
- static int error_reported = 0;
-
- if (!error_reported)
- {
- std::string short_msg
- = string_printf (_("unable to demangle '%s' "
- "(demangler failed with signal %d)"),
- name, crash_signal);
-
- std::string long_msg
- = string_printf ("%s:%d: %s: %s", __FILE__, __LINE__,
- "demangler-warning", short_msg.c_str ());
-
- target_terminal::scoped_restore_terminal_state term_state;
- target_terminal::ours_for_output ();
-
- begin_line ();
- if (core_dump_allowed)
- fprintf_unfiltered (gdb_stderr,
- _("%s\nAttempting to dump core.\n"),
- long_msg.c_str ());
- else
- warn_cant_dump_core (long_msg.c_str ());
-
- demangler_warning (__FILE__, __LINE__, "%s", short_msg.c_str ());
-
- error_reported = 1;
- }
-
- result = NULL;
- }
+ {
+ /* If there was a failure, we can't report it here, because
+ we might be in a background thread. Instead, arrange for
+ the reporting to happen on the main thread. */
+ std::string copy = name;
+ run_on_main_thread ([=] ()
+ {
+ report_failed_demangle (copy.c_str (), core_dump_allowed,
+ crash_signal);
+ });
+
+ result = NULL;
+ }
}
#endif
@@ -2194,4 +2188,7 @@ display the offending symbol."),
selftests::register_test ("cp_remove_params",
selftests::test_cp_remove_params);
#endif
+
+ if (!can_dump_core (LIMIT_CUR))
+ gdb_demangle_attempt_core_dump = false;
}
@@ -847,6 +847,45 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
}
+/* See event-top.h. */
+
+thread_local void (*thread_local_segv_handler) (int);
+
+static void handle_sigsegv (int sig);
+
+/* Install the SIGSEGV handler. */
+static void
+install_handle_sigsegv ()
+{
+#if defined (HAVE_SIGACTION)
+ struct sigaction sa;
+ sa.sa_handler = handle_sigsegv;
+ sigemptyset (&sa.sa_mask);
+#ifdef HAVE_SIGALTSTACK
+ sa.sa_flags = SA_ONSTACK;
+#else
+ sa.sa_flags = 0;
+#endif
+ sigaction (SIGSEGV, &sa, nullptr);
+#else
+ signal (SIGSEGV, handle_sigsegv);
+#endif
+}
+
+/* Handler for SIGSEGV. */
+
+static void
+handle_sigsegv (int sig)
+{
+ install_handle_sigsegv ();
+
+ if (thread_local_segv_handler == nullptr)
+ abort ();
+ thread_local_segv_handler (sig);
+}
+
+
+
/* The serial event associated with the QUIT flag. set_quit_flag sets
this, and check_quit_flag clears it. Used by interruptible_select
to be able to do interruptible I/O with no race with the SIGINT
@@ -914,6 +953,8 @@ async_init_signals (void)
sigtstp_token =
create_async_signal_handler (async_sigtstp_handler, NULL);
#endif
+
+ install_handle_sigsegv ();
}
/* See defs.h. */
@@ -70,4 +70,10 @@ extern void gdb_rl_callback_handler_install (const char *prompt);
currently installed. */
extern void gdb_rl_callback_handler_reinstall (void);
+/* The SIGSEGV handler for this thread, or NULL if there is none. GDB
+ always installs a global SIGSEGV handler, and then lets threads
+ indicate their interest in handling the signal by setting this
+ thread-local variable. */
+extern thread_local void (*thread_local_segv_handler) (int);
+
#endif