Skip to content

Conversation

@SchrodingerZhu
Copy link
Contributor

@SchrodingerZhu SchrodingerZhu commented Mar 31, 2025

On top of #133641, we now provides _malloc_thread_cleanup as an option that will be called after all other cleanup hooks but before TLS tearing down.

@llvmbot llvmbot added the libc label Mar 31, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 31, 2025

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes
  • [libc] ensure tls dtors are called in main thread
  • [libc] add _malloc_thread_cleanup option

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

12 Files Affected:

  • (modified) libc/config/config.json (+4)
  • (modified) libc/docs/configure.rst (+1)
  • (modified) libc/src/__support/threads/linux/CMakeLists.txt (+7)
  • (modified) libc/src/__support/threads/linux/thread.cpp (+9)
  • (modified) libc/src/__support/threads/thread.cpp (+5)
  • (modified) libc/src/__support/threads/thread.h (+4-2)
  • (modified) libc/src/stdlib/CMakeLists.txt (+8)
  • (modified) libc/src/stdlib/atexit.cpp (+4)
  • (modified) libc/src/stdlib/exit.cpp (+4)
  • (modified) libc/test/integration/src/__support/threads/CMakeLists.txt (+21)
  • (added) libc/test/integration/src/__support/threads/double_exit_test.cpp (+23)
  • (added) libc/test/integration/src/__support/threads/main_exit_test.cpp (+30)
diff --git a/libc/config/config.json b/libc/config/config.json index d738aade74427..b2688f1b29309 100644 --- a/libc/config/config.json +++ b/libc/config/config.json @@ -89,6 +89,10 @@ "LIBC_CONF_RWLOCK_DEFAULT_SPIN_COUNT": { "value": 100, "doc": "Default number of spins before blocking if a rwlock is in contention (default to 100)." + }, + "LIBC_CONF_ENABLE_MALLOC_THREAD_CLEANUP": { + "value": false, + "doc": "Enable the `_malloc_thread_cleanup` weak symbol. When defined, this is function is called after `__cxa` and pthread-specific dtors. On main thread, this will be called after `atexit` functions and `.fini` dtors, right before TLS tearing down. This function can be overridden by allocators to perform cleanup. Allocators can use this symbol to avoid registering thread dtors using potentially reentrant routines." } }, "math": { diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst index dee9a63101eb9..182d373c075f6 100644 --- a/libc/docs/configure.rst +++ b/libc/docs/configure.rst @@ -47,6 +47,7 @@ to learn about the defaults for your platform and target. - ``LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE``: Use large table for better printf long double performance. - ``LIBC_CONF_PRINTF_RUNTIME_DISPATCH``: Use dynamic dispatch for the output mechanism to reduce code size. * **"pthread" options** + - ``LIBC_CONF_ENABLE_MALLOC_THREAD_CLEANUP``: Enable the `_malloc_thread_cleanup` weak symbol. When defined, this is function is called after `__cxa` and pthread-specific dtors. On main thread, this will be called after `atexit` functions and `.fini` dtors, right before TLS tearing down. This function can be overridden by allocators to perform cleanup. Allocators can use this symbol to avoid registering thread dtors using potentially reentrant routines. - ``LIBC_CONF_RAW_MUTEX_DEFAULT_SPIN_COUNT``: Default number of spins before blocking if a mutex is in contention (default to 100). - ``LIBC_CONF_RWLOCK_DEFAULT_SPIN_COUNT``: Default number of spins before blocking if a rwlock is in contention (default to 100). - ``LIBC_CONF_TIMEOUT_ENSURE_MONOTONICITY``: Automatically adjust timeout to CLOCK_MONOTONIC (default to true). POSIX API may require CLOCK_REALTIME, which can be unstable and leading to unexpected behavior. This option will convert the real-time timestamp to monotonic timestamp relative to the time of call. diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt index 364e7e2b90585..3e7c16afe0f6e 100644 --- a/libc/src/__support/threads/linux/CMakeLists.txt +++ b/libc/src/__support/threads/linux/CMakeLists.txt @@ -71,6 +71,12 @@ add_header_library( libc.src.__support.threads.mutex_common ) +if (LIBC_CONF_ENABLE_MALLOC_THREAD_CLEANUP) + set(malloc_cleanup_flags -DLIBC_COPT_ENABLE_MALLOC_THREAD_CLEANUP) +else() + set(malloc_cleanup_flags) +endif() + add_object_library( thread SRCS @@ -89,6 +95,7 @@ add_object_library( libc.src.__support.threads.thread_common COMPILE_OPTIONS ${libc_opt_high_flag} + ${malloc_cleanup_flags} -fno-omit-frame-pointer # This allows us to sniff out the thread args from # the new thread's stack reliably. -Wno-frame-address # Yes, calling __builtin_return_address with a diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp index c531d74c53355..2d6d4e517064d 100644 --- a/libc/src/__support/threads/linux/thread.cpp +++ b/libc/src/__support/threads/linux/thread.cpp @@ -482,6 +482,10 @@ int Thread::get_name(cpp::StringStream &name) const { return 0; } +#ifdef LIBC_COPT_ENABLE_MALLOC_THREAD_CLEANUP +extern "C" [[gnu::weak]] void _malloc_thread_cleanup(); +#endif // LIBC_COPT_ENABLE_MALLOC_THREAD_CLEANUP + void thread_exit(ThreadReturnValue retval, ThreadStyle style) { auto attrib = self.attrib; @@ -494,6 +498,11 @@ void thread_exit(ThreadReturnValue retval, ThreadStyle style) { // different thread. The destructors of thread local and TSS objects should // be called by the thread which owns them. internal::call_atexit_callbacks(attrib); +#ifdef LIBC_COPT_ENABLE_MALLOC_THREAD_CLEANUP + // call _malloc_thread_cleanup after the atexit callbacks + if (_malloc_thread_cleanup) + _malloc_thread_cleanup(); +#endif // LIBC_COPT_ENABLE_MALLOC_THREAD_CLEANUP uint32_t joinable_state = uint32_t(DetachState::JOINABLE); if (!attrib->detach_state.compare_exchange_strong( diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp index 6f6b75be5766d..c7135596622c6 100644 --- a/libc/src/__support/threads/thread.cpp +++ b/libc/src/__support/threads/thread.cpp @@ -154,6 +154,9 @@ ThreadAtExitCallbackMgr *get_thread_atexit_callback_mgr() { } void call_atexit_callbacks(ThreadAttributes *attrib) { + if (attrib->dtors_called) + return; + attrib->dtors_called = true; attrib->atexit_callback_mgr->call(); for (size_t i = 0; i < TSS_KEY_COUNT; ++i) { TSSValueUnit &unit = tss_values[i]; @@ -163,6 +166,8 @@ 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/__support/threads/thread.h b/libc/src/__support/threads/thread.h index f2b1f6bbb253d..f7710fde2c70d 100644 --- a/libc/src/__support/threads/thread.h +++ b/libc/src/__support/threads/thread.h @@ -109,12 +109,14 @@ struct alignas(STACK_ALIGNMENT) ThreadAttributes { ThreadReturnValue retval; ThreadAtExitCallbackMgr *atexit_callback_mgr; void *platform_data; + bool dtors_called; - constexpr ThreadAttributes() + LIBC_INLINE constexpr ThreadAttributes() : detach_state(uint32_t(DetachState::DETACHED)), stack(nullptr), stacksize(0), guardsize(0), tls(0), tls_size(0), owned_stack(false), tid(-1), style(ThreadStyle::POSIX), retval(), - atexit_callback_mgr(nullptr), platform_data(nullptr) {} + atexit_callback_mgr(nullptr), platform_data(nullptr), + dtors_called(false) {} }; using TSSDtor = void(void *); diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt index 74ae864f72e23..7dd0c969cf9b2 100644 --- a/libc/src/stdlib/CMakeLists.txt +++ b/libc/src/stdlib/CMakeLists.txt @@ -589,6 +589,12 @@ add_header_library( ) endif() +if (LIBC_CONF_ENABLE_MALLOC_THREAD_CLEANUP) + set(malloc_cleanup_flags -DLIBC_COPT_ENABLE_MALLOC_THREAD_CLEANUP) +else() + set(malloc_cleanup_flags) +endif() + add_entrypoint_object( atexit SRCS @@ -599,6 +605,8 @@ add_entrypoint_object( 20 # For constinit DEPENDS .exit_handler + COMPILE_OPTIONS + ${malloc_cleanup_flags} ) add_entrypoint_object( diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp index 799aad136bda5..979ed1f29b642 100644 --- a/libc/src/stdlib/atexit.cpp +++ b/libc/src/stdlib/atexit.cpp @@ -18,6 +18,10 @@ constinit ExitCallbackList atexit_callbacks; Mutex handler_list_mtx(false, false, false, false); [[gnu::weak]] extern void teardown_main_tls(); +namespace internal { +[[gnu::weak]] extern void call_atexit_callbacks(); +} + extern "C" { int __cxa_atexit(AtExitCallback *callback, void *payload, void *) { diff --git a/libc/src/stdlib/exit.cpp b/libc/src/stdlib/exit.cpp index 28a6f8a63c0c6..097a52339e5e8 100644 --- a/libc/src/stdlib/exit.cpp +++ b/libc/src/stdlib/exit.cpp @@ -14,8 +14,12 @@ namespace LIBC_NAMESPACE_DECL { extern "C" void __cxa_finalize(void *); +extern "C" [[gnu::weak]] void __cxa_thread_finalize(); +// TODO: use recursive mutex to protect this routine. [[noreturn]] LLVM_LIBC_FUNCTION(void, exit, (int status)) { + if (__cxa_thread_finalize) + __cxa_thread_finalize(); __cxa_finalize(nullptr); internal::exit(status); } diff --git a/libc/test/integration/src/__support/threads/CMakeLists.txt b/libc/test/integration/src/__support/threads/CMakeLists.txt index 5a12d28ada3fd..40e96681b1207 100644 --- a/libc/test/integration/src/__support/threads/CMakeLists.txt +++ b/libc/test/integration/src/__support/threads/CMakeLists.txt @@ -25,3 +25,24 @@ add_integration_test( DEPENDS libc.src.__support.threads.thread ) + +add_integration_test( + main_exit_test + SUITE + libc-support-threads-integration-tests + SRCS + main_exit_test.cpp + DEPENDS + libc.src.__support.threads.thread +) + +add_integration_test( + double_exit_test + SUITE + libc-support-threads-integration-tests + SRCS + double_exit_test.cpp + DEPENDS + libc.src.__support.threads.thread + libc.src.stdlib.exit +) diff --git a/libc/test/integration/src/__support/threads/double_exit_test.cpp b/libc/test/integration/src/__support/threads/double_exit_test.cpp new file mode 100644 index 0000000000000..e4a163644a970 --- /dev/null +++ b/libc/test/integration/src/__support/threads/double_exit_test.cpp @@ -0,0 +1,23 @@ +//===-- Test handling of thread local data --------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/__support/threads/thread.h" +#include "src/stdlib/exit.h" +#include "test/IntegrationTest/test.h" + +extern "C" { +[[gnu::weak]] +void *__dso_handle = nullptr; +int __cxa_thread_atexit_impl(void (*func)(void *), void *arg, void *dso); +} + +TEST_MAIN() { + __cxa_thread_atexit_impl([](void *) { LIBC_NAMESPACE::exit(0); }, nullptr, + __dso_handle); + return 0; +} diff --git a/libc/test/integration/src/__support/threads/main_exit_test.cpp b/libc/test/integration/src/__support/threads/main_exit_test.cpp new file mode 100644 index 0000000000000..c90e4e569cfba --- /dev/null +++ b/libc/test/integration/src/__support/threads/main_exit_test.cpp @@ -0,0 +1,30 @@ +//===-- Test handling of thread local data --------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/__support/threads/thread.h" +#include "test/IntegrationTest/test.h" + +bool called = false; + +extern "C" { +[[gnu::weak]] +void *__dso_handle = nullptr; +int __cxa_thread_atexit_impl(void (*func)(void *), void *arg, void *dso); +} + +[[gnu::destructor]] +void destructor() { + if (!called) + __builtin_trap(); +} + +TEST_MAIN() { + __cxa_thread_atexit_impl([](void *) { called = true; }, nullptr, + __dso_handle); + return 0; +} 
@SchrodingerZhu SchrodingerZhu force-pushed the libc/malloc-thread-cleanup branch from d3a4c5e to 6e82eab Compare March 31, 2025 15:15
@github-actions
Copy link

github-actions bot commented Mar 31, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@SchrodingerZhu SchrodingerZhu force-pushed the libc/malloc-thread-cleanup branch from 6e82eab to 1afb0e0 Compare March 31, 2025 16:13
@SchrodingerZhu SchrodingerZhu changed the title [libc][patch 2/n] provide _malloc_thread_cleanup option [libc] provide _malloc_thread_cleanup option Apr 10, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 8 out of 13 changed files in this pull request and generated no comments.

Files not reviewed (5)
  • libc/config/config.json: Language not supported
  • libc/docs/configure.rst: Language not supported
  • libc/src/__support/threads/linux/CMakeLists.txt: Language not supported
  • libc/src/stdlib/CMakeLists.txt: Language not supported
  • libc/test/integration/src/__support/threads/CMakeLists.txt: Language not supported
@lntue lntue requested a review from frobtech August 27, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants