[compiler-rt] 38c3561 - Fix include guard and properly order __deregister_frame_info.

[Sterling Augustine via llvm-commits]

Author: Sterling Augustine
Date: 2019-11-12T14:54:41-08:00
New Revision: 38c356176b5370164578c1d08e984964354b7189

URL: https://github.com/llvm/llvm-project/commit/38c356176b5370164578c1d08e984964354b7189
DIFF: https://github.com/llvm/llvm-project/commit/38c356176b5370164578c1d08e984964354b7189.diff

LOG: Fix include guard and properly order __deregister_frame_info.

Summary:
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,
"EH_USE_FRAME_INFO_REGISTRY", which is set by the cmake option
COMPILER_RT_CRT_USE_EH_FRAME_REGISTRY 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.

Added: 
    

Modified: 
    compiler-rt/CMakeLists.txt
    compiler-rt/lib/crt/CMakeLists.txt
    compiler-rt/lib/crt/crtbegin.c
    compiler-rt/test/crt/ctor_dtor.c

Removed: 
    


################################################################################
diff  --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt
index f8925ab2d76c..8d768a404f21 100644
--- a/compiler-rt/CMakeLists.txt
+++ b/compiler-rt/CMakeLists.txt
@@ -35,6 +35,8 @@ option(COMPILER_RT_BUILD_BUILTINS "Build builtins" ON)
 mark_as_advanced(COMPILER_RT_BUILD_BUILTINS)
 option(COMPILER_RT_BUILD_CRT "Build crtbegin.o/crtend.o" ON)
 mark_as_advanced(COMPILER_RT_BUILD_CRT)
+option(COMPILER_RT_CRT_USE_EH_FRAME_REGISTRY "Use eh_frame in crtbegin.o/crtend.o" ON)
+mark_as_advanced(COMPILER_RT_CRT_USE_EH_FRAME_REGISTRY)
 option(COMPILER_RT_BUILD_SANITIZERS "Build sanitizers" ON)
 mark_as_advanced(COMPILER_RT_BUILD_SANITIZERS)
 option(COMPILER_RT_BUILD_XRAY "Build xray" ON)

diff  --git a/compiler-rt/lib/crt/CMakeLists.txt b/compiler-rt/lib/crt/CMakeLists.txt
index 90e94b93db47..1ed04827925a 100644
--- a/compiler-rt/lib/crt/CMakeLists.txt
+++ b/compiler-rt/lib/crt/CMakeLists.txt
@@ -74,6 +74,7 @@ check_cxx_section_exists(".init_array" COMPILER_RT_HAS_INITFINI_ARRAY
 
 append_list_if(COMPILER_RT_HAS_STD_C11_FLAG -std=c11 CRT_CFLAGS)
 append_list_if(COMPILER_RT_HAS_INITFINI_ARRAY -DCRT_HAS_INITFINI_ARRAY CRT_CFLAGS)
+append_list_if(COMPILER_RT_CRT_USE_EH_FRAME_REGISTRY -DEH_USE_FRAME_REGISTRY CRT_CFLAGS)
 append_list_if(COMPILER_RT_HAS_FPIC_FLAG -fPIC CRT_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WNO_PEDANTIC -Wno-pedantic CRT_CFLAGS)
 

diff  --git a/compiler-rt/lib/crt/crtbegin.c b/compiler-rt/lib/crt/crtbegin.c
index 2450ce54e31b..5b56ea3df757 100644
--- a/compiler-rt/lib/crt/crtbegin.c
+++ b/compiler-rt/lib/crt/crtbegin.c
@@ -10,11 +10,13 @@
 
 __attribute__((visibility("hidden"))) void *__dso_handle = &__dso_handle;
 
+#ifdef EH_USE_FRAME_REGISTRY
 __extension__ static void *__EH_FRAME_LIST__[]
     __attribute__((section(".eh_frame"), aligned(sizeof(void *)))) = {};
 
 extern void __register_frame_info(const void *, void *) __attribute__((weak));
 extern void *__deregister_frame_info(const void *) __attribute__((weak));
+#endif
 
 #ifndef CRT_HAS_INITFINI_ARRAY
 typedef void (*fp)(void);
@@ -32,10 +34,11 @@ static void __attribute__((used)) __do_init() {
     return;
   __initialized = 1;
 
+#ifdef EH_USE_FRAME_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 +76,13 @@ static void __attribute__((used)) __do_fini() {
     __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 EH_USE_FRAME_REGISTRY
+  if (__deregister_frame_info)
+    __deregister_frame_info(__EH_FRAME_LIST__);
+#endif
 }
 
 #ifdef CRT_HAS_INITFINI_ARRAY

diff  --git a/compiler-rt/test/crt/ctor_dtor.c b/compiler-rt/test/crt/ctor_dtor.c
index 64f188333195..69848a01c7ca 100644
--- a/compiler-rt/test/crt/ctor_dtor.c
+++ b/compiler-rt/test/crt/ctor_dtor.c
@@ -4,9 +4,23 @@
 
 #include <stdio.h>
 
-// CHECK:      ctor()
+// Ensure the various startup functions are called in the proper order.
+
+// CHECK: __register_frame_info()
+// CHECK-NEXT: ctor()
 // CHECK-NEXT: main()
 // CHECK-NEXT: dtor()
+// CHECK-NEXT: __deregister_frame_info()
+
+struct object;
+
+void __register_frame_info(const void *fi, struct object *obj) {
+  printf("__register_frame_info()\n");
+}
+
+void __deregister_frame_info(const void *fi) {
+  printf("__deregister_frame_info()\n");
+}
 
 void __attribute__((constructor)) ctor() {
   printf("ctor()\n");