From 38c356176b5370164578c1d08e984964354b7189 Mon Sep 17 00:00:00 2001 From: Sterling Augustine Date: Fri, 8 Nov 2019 15:51:35 -0800 Subject: [PATCH] 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. --- compiler-rt/CMakeLists.txt | 2 ++ compiler-rt/lib/crt/CMakeLists.txt | 1 + compiler-rt/lib/crt/crtbegin.c | 12 ++++++++---- compiler-rt/test/crt/ctor_dtor.c | 16 +++++++++++++++- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt index f8925ab..8d768a4 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 90e94b9..1ed0482 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 2450ce5..5b56ea3 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 64f1883..69848a0 100644 --- a/compiler-rt/test/crt/ctor_dtor.c +++ b/compiler-rt/test/crt/ctor_dtor.c @@ -4,9 +4,23 @@ #include -// 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"); -- 2.7.4