[libc-commits] [libc] [libc] Remove (weak) __cxa_thread_finalize (PR #216081)

[via libc-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Pavel Labath (labath)

<details>
<summary>Changes</summary>

Unlike __cxa_thread_atexit(_impl), this function is not a part of the ABI as the process of calling the thread exit callbacks is an implementation detail. Additionally, the weak definition gets in the way of refactoring the thread code as the linker will not extract an object from the archive if the dependency is already satisfied by a weak definition.

Instead of a weak definition, I use a preprocessor macro to determine whether we need to call the thread cleanup function (i.e., whether the target supports threads).

---
Full diff: https://github.com/llvm/llvm-project/pull/216081.diff


3 Files Affected:

- (modified) libc/src/__support/threads/thread.cpp (-2) 
- (modified) libc/src/stdlib/CMakeLists.txt (+10) 
- (modified) libc/src/stdlib/exit.cpp (+7-17) 


``````````diff
diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp
index babcfdc5aea15..31c230a45b573 100644
--- a/libc/src/__support/threads/thread.cpp
+++ b/libc/src/__support/threads/thread.cpp
@@ -164,8 +164,6 @@ void call_atexit_callbacks(ThreadAttributes *attrib) {
   }
 }
 
-extern "C" void __cxa_thread_finalize() { call_atexit_callbacks(self.attrib); }
-
 } // namespace internal
 
 cpp::optional<unsigned int> new_tss_key(TSSDtor *dtor) {
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index a0e8cd356a212..1d45a70d1a26f 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -811,12 +811,22 @@ if(NOT LIBC_TARGET_OS_IS_BAREMETAL)
   )
 endif()
 
+set(exit_compile_options "")
+if(TARGET libc.src.__support.threads.thread)
+  list(APPEND exit_deps
+    libc.src.__support.threads.thread
+  )
+  libc_add_definition(exit_compile_options "LIBC_COPT_SUPPORT_THREADS")
+endif()
+
 add_entrypoint_object(
   exit
   SRCS
     exit.cpp
   HDRS
     exit.h
+  COMPILE_OPTIONS
+    ${exit_compile_options}
   DEPENDS
     ${exit_deps}
 )
diff --git a/libc/src/stdlib/exit.cpp b/libc/src/stdlib/exit.cpp
index db6a8f1280902..6f6758aba5896 100644
--- a/libc/src/stdlib/exit.cpp
+++ b/libc/src/stdlib/exit.cpp
@@ -11,29 +11,19 @@
 #include "src/__support/common.h"
 #include "src/__support/macros/config.h"
 
+#ifdef LIBC_COPT_SUPPORT_THREADS
+#include "src/__support/threads/thread.h"
+#endif
+
 namespace LIBC_NAMESPACE_DECL {
 
 extern "C" void __cxa_finalize(void *);
 
-// exit() needs to clean up TLS and call associated destructors.
-//
-// The weak no-op implementation is in the same TU with its caller for the case
-// where the real definition is not linked in. This is preferable over weak
-// undefined symbol with a check since that requires a GOT slot.
-//
-// The strong implementation is in libc/src/__support/threads/thread.cpp
-// but not every platform supports threads (e.g. baremetal) in which case the
-// the no-op implementation is sufficient.
-//
-// TODO: Strictly speaking, it is not valid to call exit in overlay mode
-//       as we have no way to ensure system libc will call the TLS destructors.
-//       We should run exit related tests in hermetic mode but this is currently
-//       blocked by https://github.com/llvm/llvm-project/issues/133925.
-extern "C" [[gnu::weak]] void __cxa_thread_finalize() {}
-
 // TODO: use recursive mutex to protect this routine.
 [[noreturn]] LLVM_LIBC_FUNCTION(void, exit, (int status)) {
-  __cxa_thread_finalize();
+#ifdef LIBC_COPT_SUPPORT_THREADS
+  internal::call_atexit_callbacks(self.attrib);
+#endif
   __cxa_finalize(nullptr);
   internal::exit(status);
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/216081