[libc-commits] [libc] [libc] ensure tls dtors are called in main thread (PR #133641)
Schrodinger ZHU Yifan via libc-commits
libc-commits at lists.llvm.org
Mon Mar 31 07:26:53 PDT 2025
https://github.com/SchrodingerZhu updated https://github.com/llvm/llvm-project/pull/133641
>From c443d6ae3224877ec5b036f354827e1cd8abede3 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/7] [libc] ensure tls dtors are called in main thread
---
libc/src/stdlib/CMakeLists.txt | 1 +
libc/src/stdlib/atexit.cpp | 2 ++
.../src/__support/threads/CMakeLists.txt | 10 ++++++
.../src/__support/threads/main_exit_test.cpp | 31 +++++++++++++++++++
4 files changed, 44 insertions(+)
create mode 100644 libc/test/integration/src/__support/threads/main_exit_test.cpp
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 74ae864f72e23..2b0fb869935ef 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -598,6 +598,7 @@ add_entrypoint_object(
CXX_STANDARD
20 # For constinit
DEPENDS
+ libc.src.__support.threads.thread
.exit_handler
)
diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp
index 799aad136bda5..4289518b0d6cf 100644
--- a/libc/src/stdlib/atexit.cpp
+++ b/libc/src/stdlib/atexit.cpp
@@ -10,6 +10,7 @@
#include "hdr/types/atexithandler_t.h"
#include "src/__support/common.h"
#include "src/__support/macros/config.h"
+#include "src/__support/threads/thread.h"
#include "src/stdlib/exit_handler.h"
namespace LIBC_NAMESPACE_DECL {
@@ -26,6 +27,7 @@ int __cxa_atexit(AtExitCallback *callback, void *payload, void *) {
void __cxa_finalize(void *dso) {
if (!dso) {
+ internal::call_atexit_callbacks(self.attrib);
call_exit_callbacks(atexit_callbacks);
if (teardown_main_tls)
teardown_main_tls();
diff --git a/libc/test/integration/src/__support/threads/CMakeLists.txt b/libc/test/integration/src/__support/threads/CMakeLists.txt
index 5a12d28ada3fd..dae789e157667 100644
--- a/libc/test/integration/src/__support/threads/CMakeLists.txt
+++ b/libc/test/integration/src/__support/threads/CMakeLists.txt
@@ -25,3 +25,13 @@ 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
+)
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..1110be22a0336
--- /dev/null
+++ b/libc/test/integration/src/__support/threads/main_exit_test.cpp
@@ -0,0 +1,31 @@
+//===-- 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 30295d0e2576e9452deb9125a5dabf604930ec28 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i at zhuyi.fan>
Date: Sun, 30 Mar 2025 12:33:58 -0400
Subject: [PATCH 2/7] [libc] add an defense for reentrant exit
---
libc/src/__support/threads/thread.cpp | 3 +++
libc/src/__support/threads/thread.h | 6 +++--
.../src/__support/threads/CMakeLists.txt | 11 +++++++++
.../__support/threads/double_exit_test.cpp | 23 +++++++++++++++++++
.../src/__support/threads/main_exit_test.cpp | 1 -
5 files changed, 41 insertions(+), 3 deletions(-)
create mode 100644 libc/test/integration/src/__support/threads/double_exit_test.cpp
diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp
index 6f6b75be5766d..cee3414718c7b 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];
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/test/integration/src/__support/threads/CMakeLists.txt b/libc/test/integration/src/__support/threads/CMakeLists.txt
index dae789e157667..40e96681b1207 100644
--- a/libc/test/integration/src/__support/threads/CMakeLists.txt
+++ b/libc/test/integration/src/__support/threads/CMakeLists.txt
@@ -35,3 +35,14 @@ add_integration_test(
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
index 1110be22a0336..c90e4e569cfba 100644
--- a/libc/test/integration/src/__support/threads/main_exit_test.cpp
+++ b/libc/test/integration/src/__support/threads/main_exit_test.cpp
@@ -14,7 +14,6 @@ bool called = false;
extern "C" {
[[gnu::weak]]
void *__dso_handle = nullptr;
-
int __cxa_thread_atexit_impl(void (*func)(void *), void *arg, void *dso);
}
>From 3d6301f92eb5d49a52e4d6b744cbc41d471e52a4 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i at zhuyi.fan>
Date: Sun, 30 Mar 2025 14:53:11 -0400
Subject: [PATCH 3/7] mark thread test as hermetic only
---
libc/test/src/__support/threads/linux/CMakeLists.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/libc/test/src/__support/threads/linux/CMakeLists.txt b/libc/test/src/__support/threads/linux/CMakeLists.txt
index 4299a5617b8ff..419aad690b1d9 100644
--- a/libc/test/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/test/src/__support/threads/linux/CMakeLists.txt
@@ -1,5 +1,6 @@
add_libc_test(
raw_mutex_test
+ HERMETIC_TEST_ONLY
SUITE
libc-support-threads-tests
SRCS
>From 79d9ea2b3b7fe5c6c5dff7ebb0ae37ba478cf920 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i at zhuyi.fan>
Date: Sun, 30 Mar 2025 17:42:32 -0400
Subject: [PATCH 4/7] alternative approach
---
libc/src/__support/threads/thread.cpp | 2 ++
libc/src/stdlib/CMakeLists.txt | 1 -
libc/src/stdlib/atexit.cpp | 10 ++++++++--
libc/test/src/__support/threads/linux/CMakeLists.txt | 1 -
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp
index cee3414718c7b..d361c28e71106 100644
--- a/libc/src/__support/threads/thread.cpp
+++ b/libc/src/__support/threads/thread.cpp
@@ -166,6 +166,8 @@ void call_atexit_callbacks(ThreadAttributes *attrib) {
}
}
+void call_atexit_callbacks() { 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 2b0fb869935ef..74ae864f72e23 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -598,7 +598,6 @@ add_entrypoint_object(
CXX_STANDARD
20 # For constinit
DEPENDS
- libc.src.__support.threads.thread
.exit_handler
)
diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp
index 4289518b0d6cf..d6c9502217c00 100644
--- a/libc/src/stdlib/atexit.cpp
+++ b/libc/src/stdlib/atexit.cpp
@@ -10,7 +10,6 @@
#include "hdr/types/atexithandler_t.h"
#include "src/__support/common.h"
#include "src/__support/macros/config.h"
-#include "src/__support/threads/thread.h"
#include "src/stdlib/exit_handler.h"
namespace LIBC_NAMESPACE_DECL {
@@ -19,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 *) {
@@ -27,7 +30,10 @@ int __cxa_atexit(AtExitCallback *callback, void *payload, void *) {
void __cxa_finalize(void *dso) {
if (!dso) {
- internal::call_atexit_callbacks(self.attrib);
+ // cxa callback also need to handle local static destructors.
+ // see
+ // https://refspecs.linuxbase.org/LSB_4.0.0/LSB-Core-generic/LSB-Core-generic/baselib---cxa-finalize.html
+ internal::call_atexit_callbacks();
call_exit_callbacks(atexit_callbacks);
if (teardown_main_tls)
teardown_main_tls();
diff --git a/libc/test/src/__support/threads/linux/CMakeLists.txt b/libc/test/src/__support/threads/linux/CMakeLists.txt
index 419aad690b1d9..4299a5617b8ff 100644
--- a/libc/test/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/test/src/__support/threads/linux/CMakeLists.txt
@@ -1,6 +1,5 @@
add_libc_test(
raw_mutex_test
- HERMETIC_TEST_ONLY
SUITE
libc-support-threads-tests
SRCS
>From c754437f22167754aa454b8ccd1609b68cbfd9d8 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Mon, 31 Mar 2025 08:51:31 -0400
Subject: [PATCH 5/7] align with bionic
---
libc/src/__support/threads/thread.cpp | 2 +-
libc/src/stdlib/atexit.cpp | 4 ----
libc/src/stdlib/exit.cpp | 4 ++++
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp
index d361c28e71106..c7135596622c6 100644
--- a/libc/src/__support/threads/thread.cpp
+++ b/libc/src/__support/threads/thread.cpp
@@ -166,7 +166,7 @@ void call_atexit_callbacks(ThreadAttributes *attrib) {
}
}
-void call_atexit_callbacks() { call_atexit_callbacks(self.attrib); }
+extern "C" void __cxa_thread_finalize() { call_atexit_callbacks(self.attrib); }
} // namespace internal
diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp
index d6c9502217c00..979ed1f29b642 100644
--- a/libc/src/stdlib/atexit.cpp
+++ b/libc/src/stdlib/atexit.cpp
@@ -30,10 +30,6 @@ int __cxa_atexit(AtExitCallback *callback, void *payload, void *) {
void __cxa_finalize(void *dso) {
if (!dso) {
- // cxa callback also need to handle local static destructors.
- // see
- // https://refspecs.linuxbase.org/LSB_4.0.0/LSB-Core-generic/LSB-Core-generic/baselib---cxa-finalize.html
- internal::call_atexit_callbacks();
call_exit_callbacks(atexit_callbacks);
if (teardown_main_tls)
teardown_main_tls();
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);
}
>From 16a3922186a4e989ac7276b11ac58281c6d182bb Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Mon, 31 Mar 2025 10:26:07 -0400
Subject: [PATCH 6/7] Update atexit.cpp
---
libc/src/stdlib/atexit.cpp | 5 -----
1 file changed, 5 deletions(-)
diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp
index 979ed1f29b642..8b1bc669250ef 100644
--- a/libc/src/stdlib/atexit.cpp
+++ b/libc/src/stdlib/atexit.cpp
@@ -18,12 +18,7 @@ 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 *) {
return add_atexit_unit(atexit_callbacks, {callback, payload});
}
>From c8bebd761028c3a348b96982d7eb5c6c42f6a3bd Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Mon, 31 Mar 2025 10:26:42 -0400
Subject: [PATCH 7/7] Update atexit.cpp
---
libc/src/stdlib/atexit.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp
index 8b1bc669250ef..799aad136bda5 100644
--- a/libc/src/stdlib/atexit.cpp
+++ b/libc/src/stdlib/atexit.cpp
@@ -19,6 +19,7 @@ Mutex handler_list_mtx(false, false, false, false);
[[gnu::weak]] extern void teardown_main_tls();
extern "C" {
+
int __cxa_atexit(AtExitCallback *callback, void *payload, void *) {
return add_atexit_unit(atexit_callbacks, {callback, payload});
}
More information about the libc-commits
mailing list