[Openmp-commits] [openmp] b29c7fd - [OpenMP] Remove -Wl, -fini=__kmp_internal_end_fini

[Aaron Puchert via Openmp-commits]

Author: Aaron Puchert
Date: 2019-11-19T00:54:58+01:00
New Revision: b29c7fdb617cba4c83037840842a872e8bf2f6b1

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

LOG: [OpenMP] Remove -Wl,-fini=__kmp_internal_end_fini

Summary:
The termination function duplicated the functionality of the
__attribute((destructor))-annotated function __kmp_internal_end_fini,
and we have no indication that this doesn't work.

The function might cause issues with link-time optimization turned on:
until very recently, none of the usual linkers was reporting functions
named in -Wl,-fini as used to the LTO plugin, so it might be dropped.
If the function is dropped, -Wl,-fini=__kmp_internal_end_fini doesn't
do what we want: with ld.bfd and lld it drops the FINI attribute from
.dynamic and with gold we get FINI = 0x0, which leads to a crash on
cleanup. This can be reproduced by building with

    -DLLVM_ENABLE_PROJECTS="clang;openmp" \
    -DLLVM_ENABLE_LTO=Thin \
    -DLLVM_USE_LINKER=gold

The issue in lld has been fixed in f95273f75aa, but gold remains without
fix so far.

Fixes PR43927.

Reviewers: JonChesterfield, jdoerfert, AndreyChurbanov

Reviewed By: AndreyChurbanov

Differential Revision: https://reviews.llvm.org/D69927

Added: 
    

Modified: 
    openmp/runtime/cmake/LibompHandleFlags.cmake
    openmp/runtime/cmake/config-ix.cmake
    openmp/runtime/src/kmp.h
    openmp/runtime/src/kmp_runtime.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/runtime/cmake/LibompHandleFlags.cmake b/openmp/runtime/cmake/LibompHandleFlags.cmake
index 046c5d813752..0b8e3a35c888 100644
--- a/openmp/runtime/cmake/LibompHandleFlags.cmake
+++ b/openmp/runtime/cmake/LibompHandleFlags.cmake
@@ -99,7 +99,6 @@ function(libomp_get_ldflags ldflags)
   libomp_append(ldflags_local "-Wl,--version-script=${LIBOMP_SRC_DIR}/exports_so.txt" LIBOMP_HAVE_VERSION_SCRIPT_FLAG)
   libomp_append(ldflags_local -static-libgcc LIBOMP_HAVE_STATIC_LIBGCC_FLAG)
   libomp_append(ldflags_local -Wl,-z,noexecstack LIBOMP_HAVE_Z_NOEXECSTACK_FLAG)
-  libomp_append(ldflags_local -Wl,-fini=__kmp_internal_end_fini LIBOMP_HAVE_FINI_FLAG)
   libomp_append(ldflags_local -no-intel-extensions LIBOMP_HAVE_NO_INTEL_EXTENSIONS_FLAG)
   libomp_append(ldflags_local -static-intel LIBOMP_HAVE_STATIC_INTEL_FLAG)
   libomp_append(ldflags_local /SAFESEH LIBOMP_HAVE_SAFESEH_FLAG)

diff  --git a/openmp/runtime/cmake/config-ix.cmake b/openmp/runtime/cmake/config-ix.cmake
index 6a19e322f497..f00746d9eadb 100644
--- a/openmp/runtime/cmake/config-ix.cmake
+++ b/openmp/runtime/cmake/config-ix.cmake
@@ -105,7 +105,6 @@ elseif(NOT APPLE)
   libomp_check_linker_flag("-Wl,--version-script=${LIBOMP_SRC_DIR}/exports_so.txt" LIBOMP_HAVE_VERSION_SCRIPT_FLAG)
   libomp_check_linker_flag(-static-libgcc LIBOMP_HAVE_STATIC_LIBGCC_FLAG)
   libomp_check_linker_flag(-Wl,-z,noexecstack LIBOMP_HAVE_Z_NOEXECSTACK_FLAG)
-  libomp_check_linker_flag(-Wl,-fini=__kmp_internal_end_fini LIBOMP_HAVE_FINI_FLAG)
 endif()
 
 # Check Intel(R) C Compiler specific flags

diff  --git a/openmp/runtime/src/kmp.h b/openmp/runtime/src/kmp.h
index 23eebe673126..6d690fc4a709 100644
--- a/openmp/runtime/src/kmp.h
+++ b/openmp/runtime/src/kmp.h
@@ -3115,7 +3115,6 @@ extern void __kmp_internal_begin(void);
 extern void __kmp_internal_end_library(int gtid);
 extern void __kmp_internal_end_thread(int gtid);
 extern void __kmp_internal_end_atexit(void);
-extern void __kmp_internal_end_fini(void);
 extern void __kmp_internal_end_dtor(void);
 extern void __kmp_internal_end_dest(void *);
 

diff  --git a/openmp/runtime/src/kmp_runtime.cpp b/openmp/runtime/src/kmp_runtime.cpp
index dd6e0ff70193..b219c60ee653 100644
--- a/openmp/runtime/src/kmp_runtime.cpp
+++ b/openmp/runtime/src/kmp_runtime.cpp
@@ -5799,16 +5799,10 @@ void __kmp_internal_end_dest(void *specific_gtid) {
 
 #if KMP_OS_UNIX && KMP_DYNAMIC_LIB
 
-// 2009-09-08 (lev): It looks the destructor does not work. In simple test cases
-// destructors work perfectly, but in real libomp.so I have no evidence it is
-// ever called. However, -fini linker option in makefile.mk works fine.
-
 __attribute__((destructor)) void __kmp_internal_end_dtor(void) {
   __kmp_internal_end_atexit();
 }
 
-void __kmp_internal_end_fini(void) { __kmp_internal_end_atexit(); }
-
 #endif
 
 /* [Windows] josh: when the atexit handler is called, there may still be more