From fece451c2aca57b095e7e4063e342781cf74aa75 Mon Sep 17 00:00:00 2001 From: Christian Biesinger Date: Tue, 9 Mar 2021 08:16:23 -0600 Subject: [PATCH] Use RAII to set the per-thread SIGSEGV handler This avoids using a thread-local extern variable, which causes link errors on some platforms, notably Cygwin. But I think this is a better pattern even outside of working around linker bugs because it encapsulates direct access to the variable inside the class, instead of having a global extern variable. The cygwin link error is: cp-support.o: in function `gdb_demangle(char const*, int)': /home/Christian/binutils-gdb/obj/gdb/../../gdb/cp-support.c:1619:(.text+0x6472): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for thread_local_segv_handler' /home/Christian/binutils-gdb/obj/gdb/../../gdb/cp-support.c:1619:(.text+0x648b): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for thread_local_segv_handler' collect2: error: ld returned 1 exit status 2021-03-12 Christian Biesinger PR threads/27239 * cp-support.c: Use scoped_segv_handler_restore. * event-top.c (thread_local_segv_handler): Made static. (scoped_segv_handler_restore::scoped_segv_handler_restore): New function. (scoped_segv_handler_restore::~scoped_segv_handler_restore): New function. * event-top.h (class scoped_segv_handler_restore): New class. (thread_local_segv_handler): Removed. --- gdb/ChangeLog | 12 ++++++++++++ gdb/cp-support.c | 9 ++++----- gdb/event-top.c | 23 +++++++++++++++++++++-- gdb/event-top.h | 19 ++++++++++++++----- 4 files changed, 51 insertions(+), 12 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f6732b853be..0157dea6158 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2021-03-12 Christian Biesinger + + PR threads/27239 + * cp-support.c: Use scoped_segv_handler_restore. + * event-top.c (thread_local_segv_handler): Made static. + (scoped_segv_handler_restore::scoped_segv_handler_restore): + New function. + (scoped_segv_handler_restore::~scoped_segv_handler_restore): New + function. + * event-top.h (class scoped_segv_handler_restore): New class. + (thread_local_segv_handler): Removed. + 2021-03-10 Tom Tromey * parser-defs.h (parser_state): Change completion to bool. diff --git a/gdb/cp-support.c b/gdb/cp-support.c index 1abc3e22a72..fb4e4289777 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -1615,11 +1615,10 @@ gdb_demangle (const char *name, int options) int crash_signal = 0; #ifdef HAVE_WORKING_FORK - scoped_restore restore_segv - = make_scoped_restore (&thread_local_segv_handler, - catch_demangler_crashes - ? gdb_demangle_signal_handler - : nullptr); + scoped_segv_handler_restore restore_segv + (catch_demangler_crashes + ? gdb_demangle_signal_handler + : nullptr); bool core_dump_allowed = gdb_demangle_attempt_core_dump; SIGJMP_BUF jmp_buf; diff --git a/gdb/event-top.c b/gdb/event-top.c index a33f6425140..fb0df943f65 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -850,9 +850,17 @@ gdb_readline_no_editing_callback (gdb_client_data client_data) } -/* See event-top.h. */ +/* 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. -thread_local void (*thread_local_segv_handler) (int); + This is a static variable instead of extern because on various platforms + (notably Cygwin) extern thread_local variables cause link errors. So + instead, we have scoped_segv_handler_restore, which also makes it impossible + to accidentally forget to restore it to the original value. */ + +static thread_local void (*thread_local_segv_handler) (int); static void handle_sigsegv (int sig); @@ -1288,6 +1296,17 @@ gdb_disable_readline (void) delete_file_handler (ui->input_fd); } +scoped_segv_handler_restore::scoped_segv_handler_restore (segv_handler_t new_handler) +{ + m_old_handler = thread_local_segv_handler; + thread_local_segv_handler = new_handler; +} + +scoped_segv_handler_restore::~scoped_segv_handler_restore() +{ + thread_local_segv_handler = m_old_handler; +} + static const char debug_event_loop_off[] = "off"; static const char debug_event_loop_all_except_ui[] = "all-except-ui"; static const char debug_event_loop_all[] = "all"; diff --git a/gdb/event-top.h b/gdb/event-top.h index c52826eb063..b036f1385c5 100644 --- a/gdb/event-top.h +++ b/gdb/event-top.h @@ -70,10 +70,19 @@ 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); +typedef void (*segv_handler_t) (int); + +/* On construction, replaces the current thread's SIGSEGV handler with + the provided one. On destruction, restores the handler to the + original one. */ +class scoped_segv_handler_restore +{ + public: + scoped_segv_handler_restore (segv_handler_t new_handler); + ~scoped_segv_handler_restore (); + + private: + segv_handler_t m_old_handler; +}; #endif -- 2.30.2