[PATCH] D70034: Fix include guard and properly order __deregister_frame_info.

[Sterling Augustine via Phabricator via llvm-commits]

saugustine created this revision.
saugustine added reviewers: phosek, echristo, joerg.
Herald added projects: Sanitizers, LLVM.
Herald added subscribers: llvm-commits, Sanitizers.

This patch fixes two problems with the crtbegin.c as written:

1. In __do_init, __register_frame_info is not guarded by a #define, but in __do_fini, __deregister_frame_info is guarded by #ifndef CRT_HAS_INITFINI_ARRAY. Thus when CRT_HAS_INITFINI_ARRAY is not defined, frames are registered but then never deregistered.

  The frame registry mechanism builds a linked-list from the .so's static variable __do_init.__object, and when the .so is unloaded, this memory becomes invalid and should be deregistered.

  Further, libgcc's crtbegin treats the frame registry as independent from the initfini array mechanism.

  This patch fixes this by adding a new #define, "USING_FRAME_INFO_REGISTRY", which is set by the top-level define "USE_FRAME_INFO_REGISTRY". The indirection allows combining the first test with the backward compatibility check below.

  For backward compatibility, USING_FRAME_INFO_REGISTRY is also set when CRT_HAS_INITFINI_ARRAY is defined, as I believe this is the intent of the original code. It would be nice to make them independent, but that is tricky without knowing all users and targets.

2. Currently, __do_init calls __register_frame_info, and then calls the binary's constructors. This allows constructors to safely use libunwind.  However, __do_fini calls __deregister_frame_info and then calls the binary's destructors. This prevents destructors from safely using libunwind.

  This patch also switches that ordering, so that destructors can safely use libunwind. As it happens, this is a fairly common scenario for thread sanitizer.

I have a test written for this, but it would add a dependency to libunwind,
which may not be built or installed. Please advise.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70034

Files:
  compiler-rt/lib/crt/crtbegin.c


Index: compiler-rt/lib/crt/crtbegin.c
===================================================================
--- compiler-rt/lib/crt/crtbegin.c
+++ compiler-rt/lib/crt/crtbegin.c
@@ -10,8 +10,14 @@
 
 __attribute__((visibility("hidden"))) void *__dso_handle = &__dso_handle;
 
+#if defined(CRT_HAS_INITFINI_ARRAY) || defined(EH_USE_FRAME_REGISTRY)
+#define USING_EH_FRAME_INFO_REGISTRY
+#endif
+
+#ifdef USING_EH_FRAME_INFO_REGISTRY
 __extension__ static void *__EH_FRAME_LIST__[]
     __attribute__((section(".eh_frame"), aligned(sizeof(void *)))) = {};
+#endif
 
 extern void __register_frame_info(const void *, void *) __attribute__((weak));
 extern void *__deregister_frame_info(const void *) __attribute__((weak));
@@ -32,10 +38,11 @@
     return;
   __initialized = 1;
 
+#ifdef USING_EH_FRAME_INFO_REGISTRY
   static struct { void *p[8]; } __object;
   if (__register_frame_info)
     __register_frame_info(__EH_FRAME_LIST__, &__object);
-
+#endif
 #ifndef CRT_HAS_INITFINI_ARRAY
   const size_t n = __CTOR_LIST_END__ - __CTOR_LIST__ - 1;
   for (size_t i = n; i >= 1; i--) __CTOR_LIST__[i]();
@@ -73,12 +80,13 @@
     __cxa_finalize(__dso_handle);
 
 #ifndef CRT_HAS_INITFINI_ARRAY
-  if (__deregister_frame_info)
-    __deregister_frame_info(__EH_FRAME_LIST__);
-
   const size_t n = __DTOR_LIST_END__ - __DTOR_LIST__ - 1;
   for (size_t i = 1; i <= n; i++) __DTOR_LIST__[i]();
 #endif
+#ifdef USING_EH_FRAME_REGISTRY
+  if (__deregister_frame_info)
+    __deregister_frame_info(__EH_FRAME_LIST__);
+#endif
 }
 
 #ifdef CRT_HAS_INITFINI_ARRAY


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70034.228539.patch
Type: text/x-patch
Size: 1549 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191109/6b1b5892/attachment.bin>