Fix include guard and properly order __deregister_frame_info.
authorSterling Augustine <saugustine@google.com>
Fri, 8 Nov 2019 23:51:35 +0000 (15:51 -0800)
committerSterling Augustine <saugustine@google.com>
Tue, 12 Nov 2019 22:54:41 +0000 (14:54 -0800)
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.

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

index f8925ab2d76c2775d7dc873119da049d4c04cffc..8d768a404f210e7c917bfb4205c67115f2a99c36 100644 (file)
@@ -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)
index 90e94b93db4774d2cfe089711c1fe9a442fbbc96..1ed04827925a75d4a1f21984bc2591d1a1837386 100644 (file)
@@ -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)
 
index 2450ce54e31b6dfaca959c903f04697f9fac7e82..5b56ea3df757f711c735da2c6f388e3aeb2c19b7 100644 (file)
 
 __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
index 64f1883331957f724044deb1c5fc86ecedfd2e54..69848a01c7ca09be9cf591ad2a90958bcbeb3c2f 100644 (file)
@@ -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");