Fix relocation of R_X86_64_PC32 against `TLS init function for thread_local_segv_handler' [Cygwin]

[Christian Biesinger]

On Thu, Nov 26, 2020 at 10:38 PM Александр Сергеевич Герасимов via
Gdb-patches <gdb-patches@sourceware.org> wrote:
>
> This path fixes the following error while building GDB on Windows under Cygwin:
> CXXLD  gdb.exe
> cp-support.o: in function `gdb_demangle(char const*, int)':
>     /home/mario/Gcc101_maker/build/gdb/gdb/../../../source/gdb-9.1/gdb/cp-support.c:1552:(.text+0x1d14): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for thread_local_segv_handler'
>     /home/mario/Gcc101_maker/build/gdb/gdb/../../../source/gdb-9.1/gdb/cp-support.c:1552:(.text+0x1d2d): 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
>     make[2]: *** [Makefile:1908: gdb.exe] Error 1
>     make[2]: Leaving directory '/home/mario/Gcc101_maker/build/gdb/gdb'
>     make[1]: *** [Makefile:9567: all-gdb] Error 2
>     make[1]: Leaving directory '/home/mario/Gcc101_maker/build/gdb'
>     make: *** [Makefile:855: all] Error 2
> Issue reacked on GDB versions a least 9.1 and 9.2.

Hi Александр,

thanks for working on this. I've seen this error also on other, more
unusual, toolchains. I can't approve patches, but here are some
comments:

I think using a setter works well; however, I don't think it makes
sense to put this in an #ifdef -- it can just be used everywhere.

+  thread_local void (*thread_local_segv_handler_l) (int);
+  thread_local_segv_handler_l = NULL;
+
   scoped_restore restore_segv
+    = make_scoped_restore (&thread_local_segv_handler_l,
+              catch_demangler_crashes
+              ? gdb_demangle_signal_handler
+              : nullptr);
+  set_segv_handler((void*)thread_local_segv_handler_l);

How does this work...? This will just restore
thread_local_segv_handler_l to the original value, not the "real" segv
handler, right? It seems this could be fixed in one of two ways: Make
a special restore class, implemented in event-top.c, that knows how to
restore the variable; or else expose a getter function for the handler
and adapt scoped_restore for that.

+void set_segv_handler(void* hdl)

This should take a function pointer.

   if (thread_local_segv_handler == nullptr)
+  {
     abort ();
+  }

This change seems pointless...?

Also, this patch is missing a changelog entry:
https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog

Thanks,
Christian