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  <cbiesinger@google.com>

	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.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f6732b853be0f2c4d433c3520e70ce0f3177f4a5..0157dea61580e93206262ae2c374689ec5c9e105 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2021-03-12  Christian Biesinger  <cbiesinger@google.com>
+
+       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  <tromey@adacore.com>
 
        * parser-defs.h (parser_state): Change completion to bool.

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 1abc3e22a7206d35cfe8af2afd66522578acf5d9..fb4e4289777aac9ddcc97faf6155bf8701b9ed52 100644 (file)
--- 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 a33f64251405d0031cd07bb78e9942f786ba9b75..fb0df943f650cb3b3a084f50b34033de78d5d18e 100644 (file)
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -850,9 +850,17 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
 }
 \f
 
-/* 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 c52826eb06385d237386e1b6116697041ef6aabc..b036f1385c591d87ac1cac4a9a359d76896eb116 100644 (file)
--- 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