[libc-commits] [libc] [libc][patch 1/n] ensure tls dtors are called in main thread (PR #133641)
Schrodinger ZHU Yifan via libc-commits
libc-commits at lists.llvm.org
Wed Apr 2 06:43:54 PDT 2025
https://github.com/SchrodingerZhu updated https://github.com/llvm/llvm-project/pull/133641
>From 175ae9a1e62fd06977a4646d46aaca1b9fa526e3 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i at zhuyi.fan>
Date: Sun, 30 Mar 2025 12:15:40 -0400
Subject: [PATCH 1/6] [libc] ensure tls dtors are called in main thread
---
libc/src/__support/threads/thread.cpp | 5 ++++
libc/src/__support/threads/thread.h | 6 ++--
libc/src/stdlib/exit.cpp | 4 +++
.../src/__support/threads/CMakeLists.txt | 21 +++++++++++++
.../__support/threads/double_exit_test.cpp | 23 ++++++++++++++
.../src/__support/threads/main_exit_test.cpp | 30 +++++++++++++++++++
6 files changed, 87 insertions(+), 2 deletions(-)
create mode 100644 libc/test/integration/src/__support/threads/double_exit_test.cpp
create mode 100644 libc/test/integration/src/__support/threads/main_exit_test.cpp
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/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;
+}
>From 0836f08ac42fdebbaf06cf9bed01f7b72bc7125f Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Tue, 1 Apr 2025 10:34:48 -0400
Subject: [PATCH 2/6] address CR
---
libc/src/stdlib/CMakeLists.txt | 1 +
libc/src/stdlib/exit.cpp | 5 ++---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 74ae864f72e23..a434a8a397966 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -631,6 +631,7 @@ add_entrypoint_object(
HDRS
exit.h
DEPENDS
+ libc.src.__support.threads.thread
${exit_deps}
)
diff --git a/libc/src/stdlib/exit.cpp b/libc/src/stdlib/exit.cpp
index 097a52339e5e8..ac1b7a998d532 100644
--- a/libc/src/stdlib/exit.cpp
+++ b/libc/src/stdlib/exit.cpp
@@ -14,12 +14,11 @@
namespace LIBC_NAMESPACE_DECL {
extern "C" void __cxa_finalize(void *);
-extern "C" [[gnu::weak]] void __cxa_thread_finalize();
+extern "C" 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_thread_finalize();
__cxa_finalize(nullptr);
internal::exit(status);
}
>From a3bd867e4669c072aa7bc384c3bb56e5dab53950 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Tue, 1 Apr 2025 10:50:53 -0400
Subject: [PATCH 3/6] remove extra check
---
libc/src/__support/threads/thread.cpp | 3 ---
libc/src/__support/threads/thread.h | 4 +---
.../src/__support/threads/double_exit_test.cpp | 12 +++++++++++-
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp
index c7135596622c6..9618d7829161a 100644
--- a/libc/src/__support/threads/thread.cpp
+++ b/libc/src/__support/threads/thread.cpp
@@ -154,9 +154,6 @@ 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];
diff --git a/libc/src/__support/threads/thread.h b/libc/src/__support/threads/thread.h
index f7710fde2c70d..7536546c38a2a 100644
--- a/libc/src/__support/threads/thread.h
+++ b/libc/src/__support/threads/thread.h
@@ -109,14 +109,12 @@ struct alignas(STACK_ALIGNMENT) ThreadAttributes {
ThreadReturnValue retval;
ThreadAtExitCallbackMgr *atexit_callback_mgr;
void *platform_data;
- bool dtors_called;
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),
- dtors_called(false) {}
+ atexit_callback_mgr(nullptr), platform_data(nullptr) {}
};
using TSSDtor = void(void *);
diff --git a/libc/test/integration/src/__support/threads/double_exit_test.cpp b/libc/test/integration/src/__support/threads/double_exit_test.cpp
index e4a163644a970..86749d937c10c 100644
--- a/libc/test/integration/src/__support/threads/double_exit_test.cpp
+++ b/libc/test/integration/src/__support/threads/double_exit_test.cpp
@@ -16,8 +16,18 @@ void *__dso_handle = nullptr;
int __cxa_thread_atexit_impl(void (*func)(void *), void *arg, void *dso);
}
+int call_num = 0;
+
+[[gnu::destructor]]
+void check() {
+ // This destructor should be called only once.
+ if (call_num != 1)
+ __builtin_trap();
+}
+
TEST_MAIN() {
__cxa_thread_atexit_impl([](void *) { LIBC_NAMESPACE::exit(0); }, nullptr,
__dso_handle);
- return 0;
+ __cxa_thread_atexit_impl([](void *) { ++call_num; }, nullptr, __dso_handle);
+ LIBC_NAMESPACE::exit(1);
}
>From 517934afbdf8ca1c09946d71504a901e286bc65e Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Tue, 1 Apr 2025 11:29:16 -0400
Subject: [PATCH 4/6] revert
---
libc/src/stdlib/CMakeLists.txt | 1 -
libc/src/stdlib/exit.cpp | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index a434a8a397966..74ae864f72e23 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -631,7 +631,6 @@ add_entrypoint_object(
HDRS
exit.h
DEPENDS
- libc.src.__support.threads.thread
${exit_deps}
)
diff --git a/libc/src/stdlib/exit.cpp b/libc/src/stdlib/exit.cpp
index ac1b7a998d532..5501cb4b22f5e 100644
--- a/libc/src/stdlib/exit.cpp
+++ b/libc/src/stdlib/exit.cpp
@@ -14,7 +14,7 @@
namespace LIBC_NAMESPACE_DECL {
extern "C" void __cxa_finalize(void *);
-extern "C" void __cxa_thread_finalize();
+extern "C" [[gnu::weak]] void __cxa_thread_finalize();
// TODO: use recursive mutex to protect this routine.
[[noreturn]] LLVM_LIBC_FUNCTION(void, exit, (int status)) {
>From eaec6c97ecca9735e2cf51fc7c79ffc9129ff1a2 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Tue, 1 Apr 2025 11:34:46 -0400
Subject: [PATCH 5/6] comment
---
libc/src/stdlib/exit.cpp | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/libc/src/stdlib/exit.cpp b/libc/src/stdlib/exit.cpp
index 5501cb4b22f5e..4a51e3495473b 100644
--- a/libc/src/stdlib/exit.cpp
+++ b/libc/src/stdlib/exit.cpp
@@ -14,6 +14,12 @@
namespace LIBC_NAMESPACE_DECL {
extern "C" void __cxa_finalize(void *);
+
+// exit needs to clean up TLS and call associated destructors.
+// 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.
>From 77793f4d8f599a7720c6bf42f5bab17d557dcb0e Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 2 Apr 2025 09:43:28 -0400
Subject: [PATCH 6/6] comment
---
libc/src/stdlib/exit.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libc/src/stdlib/exit.cpp b/libc/src/stdlib/exit.cpp
index 4a51e3495473b..ef3b8dd246305 100644
--- a/libc/src/stdlib/exit.cpp
+++ b/libc/src/stdlib/exit.cpp
@@ -24,7 +24,8 @@ 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();
+ if (__cxa_thread_finalize)
+ __cxa_thread_finalize();
__cxa_finalize(nullptr);
internal::exit(status);
}
More information about the libc-commits
mailing list