[PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added a comment. In https://reviews.llvm.org/D21803#556857, @EricWF wrote: > @rmaprath I'll merge this if needed. Feel free to commit your patch first. Yeah, @rmaprath I'm happy to rebase this over your patch. > @tavianator Do you need somebody to merge this for you? I assume so, yeah. https://reviews.llvm.org/D21803 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 71934.
tavianator marked an inline comment as done.
tavianator added a comment.
s/indended/intended/
https://reviews.llvm.org/D21803
Files:
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
test/thread_local_destruction_order.pass.cpp
Index: test/thread_local_destruction_order.pass.cpp
===
--- test/thread_local_destruction_order.pass.cpp
+++ test/thread_local_destruction_order.pass.cpp
@@ -0,0 +1,56 @@
+//===-- thread_local_destruction_order.pass.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+#include
+#include
+
+int seq = 0;
+
+class OrderChecker {
+public:
+ explicit OrderChecker(int n) : n_{n} { }
+
+ ~OrderChecker() {
+assert(seq++ == n_);
+ }
+
+private:
+ int n_;
+};
+
+template
+class CreatesThreadLocalInDestructor {
+public:
+ ~CreatesThreadLocalInDestructor() {
+thread_local OrderChecker checker{ID};
+ }
+};
+
+OrderChecker global{7};
+
+void thread_fn() {
+ static OrderChecker fn_static{5};
+ thread_local CreatesThreadLocalInDestructor<2> creates_tl2;
+ thread_local OrderChecker fn_thread_local{1};
+ thread_local CreatesThreadLocalInDestructor<0> creates_tl0;
+}
+
+int main() {
+ static OrderChecker fn_static{6};
+
+ std::thread{thread_fn}.join();
+ assert(seq == 3);
+
+ thread_local OrderChecker fn_thread_local{4};
+ thread_local CreatesThreadLocalInDestructor<3> creates_tl;
+
+ return 0;
+}
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -7,20 +7,133 @@
//
//===--===//
+#include "abort_message.h"
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
-extern "C" {
-#ifdef HAVE___CXA_THREAD_ATEXIT_IMPL
+ using Dtor = void(*)(void*);
-_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(void (*dtor)(void *), void *obj,
-void *dso_symbol) throw() {
- extern int __cxa_thread_atexit_impl(void (*)(void *), void *, void *);
- return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
-}
+ extern "C"
+#ifndef HAVE___CXA_THREAD_ATEXIT_IMPL
+ // A weak symbol is used to detect this function's presence in the C library
+ // at runtime, even if libc++ is built against an older libc
+ __attribute__((__weak__))
+#endif
+ int __cxa_thread_atexit_impl(Dtor, void*,
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator marked 5 inline comments as done.
Comment at: src/cxa_thread_atexit.cpp:70
@@ +69,3 @@
+while (auto head = dtors) {
+ dtors = head->next;
+ head->dtor(head->obj);
EricWF wrote:
> tavianator wrote:
> > EricWF wrote:
> > > tavianator wrote:
> > > > EricWF wrote:
> > > > > There is a bug here. If `head->next == nullptr` and if
> > > > > `head->dtor(head->obj))` creates a TL variable in the destructor then
> > > > > that destructor will not be invoked.
> > > > >
> > > > > Here's an updated test case which catches the bug:
> > > > > https://gist.github.com/EricWF/3bb50d4f28b91aa28d2adefea0e94a0e
> > > > I can't reproduce that failure here, your exact test case passes (even
> > > > with `#undef HAVE___CXA_THREAD_ATEXIT_IMPL` and the weak symbol test
> > > > commented out).
> > > >
> > > > Tracing the implementation logic, it seems correct. If `head->next ==
> > > > nullptr` then this line does `dtors = nullptr`. Then if
> > > > `head->dtor(head->obj)` registers a new `thread_local`,
> > > > `__cxa_thread_atexit()` does `head = malloc(...); ... dtors = head;`.
> > > > Then the next iteration of the loop `while (auto head = dtors) {` picks
> > > > up that new node.
> > > >
> > > > Have I missed something?
> > > I can't reproduce this morning either, I must have been doing something
> > > funny. I'll look at this with a fresh head tomorrow. If I can't find
> > > anything this will be good to go. Thanks for working on this.
> > No problem! I can integrate your updated test case anyway if you want.
> Yeah I would like to see the upgraded test case applied. At least that way
> we're testing the case in question.
>
> So I agree with your above analysis of what happens, and that all destructors
> are correctly called during the first iteration of pthread key destruction.
> My one issues is that we still register a new non-null key which forces
> pthread to run the destructor for the key again. I would like to see this
> fixed.
Yep, done! I guess that was the point of `thread_alive` from your original
patch-to-my-patch, sorry for stripping it out.
https://reviews.llvm.org/D21803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 71508.
tavianator added a comment.
Herald added subscribers: mgorny, beanz.
Integrated @EricWF's expanded test case, and avoid an unneeded
pthread_setspecific() call if the last thread_local's destructor initializes a
new thread_local.
https://reviews.llvm.org/D21803
Files:
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
test/thread_local_destruction_order.pass.cpp
Index: test/thread_local_destruction_order.pass.cpp
===
--- test/thread_local_destruction_order.pass.cpp
+++ test/thread_local_destruction_order.pass.cpp
@@ -0,0 +1,56 @@
+//===-- thread_local_destruction_order.pass.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+#include
+#include
+
+int seq = 0;
+
+class OrderChecker {
+public:
+ explicit OrderChecker(int n) : n_{n} { }
+
+ ~OrderChecker() {
+assert(seq++ == n_);
+ }
+
+private:
+ int n_;
+};
+
+template
+class CreatesThreadLocalInDestructor {
+public:
+ ~CreatesThreadLocalInDestructor() {
+thread_local OrderChecker checker{ID};
+ }
+};
+
+OrderChecker global{7};
+
+void thread_fn() {
+ static OrderChecker fn_static{5};
+ thread_local CreatesThreadLocalInDestructor<2> creates_tl2;
+ thread_local OrderChecker fn_thread_local{1};
+ thread_local CreatesThreadLocalInDestructor<0> creates_tl0;
+}
+
+int main() {
+ static OrderChecker fn_static{6};
+
+ std::thread{thread_fn}.join();
+ assert(seq == 3);
+
+ thread_local OrderChecker fn_thread_local{4};
+ thread_local CreatesThreadLocalInDestructor<3> creates_tl;
+
+ return 0;
+}
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -7,20 +7,133 @@
//
//===--===//
+#include "abort_message.h"
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
-extern "C" {
-#ifdef HAVE___CXA_THREAD_ATEXIT_IMPL
+ using Dtor = void(*)(void*);
-_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(void (*dtor)(void *), void *obj,
-void *dso_symbol) throw() {
- extern int __cxa_thread_atexit_impl(void (*)(void *), void *, void *);
- return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
-}
+ extern "C"
+#ifndef HAVE___CXA_THREAD_ATEXIT_IMPL
+ // A weak symbol is used to detect this function's presence in the C library
+ // at
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added inline comments.
Comment at: src/cxa_thread_atexit.cpp:70
@@ +69,3 @@
+while (auto head = dtors) {
+ dtors = head->next;
+ head->dtor(head->obj);
EricWF wrote:
> tavianator wrote:
> > EricWF wrote:
> > > There is a bug here. If `head->next == nullptr` and if
> > > `head->dtor(head->obj))` creates a TL variable in the destructor then
> > > that destructor will not be invoked.
> > >
> > > Here's an updated test case which catches the bug:
> > > https://gist.github.com/EricWF/3bb50d4f28b91aa28d2adefea0e94a0e
> > I can't reproduce that failure here, your exact test case passes (even with
> > `#undef HAVE___CXA_THREAD_ATEXIT_IMPL` and the weak symbol test commented
> > out).
> >
> > Tracing the implementation logic, it seems correct. If `head->next ==
> > nullptr` then this line does `dtors = nullptr`. Then if
> > `head->dtor(head->obj)` registers a new `thread_local`,
> > `__cxa_thread_atexit()` does `head = malloc(...); ... dtors = head;`. Then
> > the next iteration of the loop `while (auto head = dtors) {` picks up that
> > new node.
> >
> > Have I missed something?
> I can't reproduce this morning either, I must have been doing something
> funny. I'll look at this with a fresh head tomorrow. If I can't find anything
> this will be good to go. Thanks for working on this.
No problem! I can integrate your updated test case anyway if you want.
https://reviews.llvm.org/D21803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added inline comments.
Comment at: src/cxa_thread_atexit.cpp:70
@@ +69,3 @@
+while (auto head = dtors) {
+ dtors = head->next;
+ head->dtor(head->obj);
EricWF wrote:
> There is a bug here. If `head->next == nullptr` and if
> `head->dtor(head->obj))` creates a TL variable in the destructor then that
> destructor will not be invoked.
>
> Here's an updated test case which catches the bug:
> https://gist.github.com/EricWF/3bb50d4f28b91aa28d2adefea0e94a0e
I can't reproduce that failure here, your exact test case passes (even with
`#undef HAVE___CXA_THREAD_ATEXIT_IMPL` and the weak symbol test commented out).
Tracing the implementation logic, it seems correct. If `head->next == nullptr`
then this line does `dtors = nullptr`. Then if `head->dtor(head->obj)`
registers a new `thread_local`, `__cxa_thread_atexit()` does `head =
malloc(...); ... dtors = head;`. Then the next iteration of the loop `while
(auto head = dtors) {` picks up that new node.
Have I missed something?
https://reviews.llvm.org/D21803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added a comment.
In https://reviews.llvm.org/D21803#532309, @EricWF wrote:
> `__thread`
>
> What do you think of this idea?
Makes sense to me, I'll integrate it into the next revision.
Comment at: src/cxa_thread_atexit.cpp:42
@@ +41,3 @@
+ // - thread_local destructors on other threads run on the first iteration
+ // through the pthread_key destructors. std::notify_all_at_thread_exit()
+ // and similar functions must be careful to wait until the second iteration
EricWF wrote:
> Can you clarify what you mean by "other threads"?
>
> How is libc++ supposed to detect and handle this problem?
I meant "non-main threads" ("other" is in relation to the bullet point above),
but I can clarify this, sure.
libc++ could be patched to do something like this:
```
pthread_key_t key1, key2;
void destructor1(void* ptr) {
pthread_setspecific(key2, ptr);
}
void destructor2(void* ptr) {
// Runs in the second iteration through pthread_key destructors,
// therefore after thread_local destructors
}
pthread_key_create(, destructor1);
pthread_key_create(, destructor2);
pthread_setspecific(key1, ptr);
```
(Or it could use a counter/flag and a single pthread_key.)
libstdc++ has the same bug when __cxa_thread_atexit_impl() isn't available, so
I'm not sure that change would really be necessary. If it is, I can write up
the libc++ patch.
https://reviews.llvm.org/D21803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added a comment. In https://reviews.llvm.org/D21803#530681, @bcraig wrote: > In https://reviews.llvm.org/D21803#530678, @tavianator wrote: > > > Ping? > > > Well, I still think it's fine. Maybe a direct message to @mclow.lists or > @EricWF? Is there a way to do that through Phabricator? Or did you mean to email them directly? (Not sure what their emails are but I can probably figure it out from the list history.) https://reviews.llvm.org/D21803 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added a comment. Ping? https://reviews.llvm.org/D21803 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added a comment. Anything else I need to do for this patch? https://reviews.llvm.org/D21803 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 63806.
tavianator added a comment.
Make sure the tail of the list is null.
http://reviews.llvm.org/D21803
Files:
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
test/thread_local_destruction_order.pass.cpp
Index: test/thread_local_destruction_order.pass.cpp
===
--- test/thread_local_destruction_order.pass.cpp
+++ test/thread_local_destruction_order.pass.cpp
@@ -0,0 +1,59 @@
+//===-- thread_local_destruction_order.pass.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+#include
+#include
+
+int seq = 0;
+
+class OrderChecker {
+public:
+ explicit OrderChecker(int n) : n_{n} { }
+
+ ~OrderChecker() {
+assert(seq++ == n_);
+ }
+
+private:
+ int n_;
+};
+
+class CreatesThreadLocalInDestructor {
+public:
+ CreatesThreadLocalInDestructor(int n) : n_{n} { }
+
+ ~CreatesThreadLocalInDestructor() {
+thread_local OrderChecker checker{n_};
+ }
+
+private:
+ int n_;
+};
+
+OrderChecker global{6};
+
+void thread_fn() {
+ static OrderChecker fn_static{4};
+ thread_local OrderChecker fn_thread_local{1};
+ thread_local CreatesThreadLocalInDestructor creates_tl{0};
+}
+
+int main() {
+ static OrderChecker fn_static{5};
+
+ std::thread{thread_fn}.join();
+ assert(seq == 2);
+
+ thread_local OrderChecker fn_thread_local{3};
+ thread_local CreatesThreadLocalInDestructor creates_tl{2};
+
+ return 0;
+}
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -7,20 +7,151 @@
//
//===--===//
+#include "abort_message.h"
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
-extern "C" {
-#ifdef HAVE___CXA_THREAD_ATEXIT_IMPL
+ using Dtor = void(*)(void*);
-_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(void (*dtor)(void *), void *obj,
-void *dso_symbol) throw() {
- extern int __cxa_thread_atexit_impl(void (*)(void *), void *, void *);
- return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
-}
+ extern "C"
+#ifndef HAVE___CXA_THREAD_ATEXIT_IMPL
+ // A weak symbol is used to detect this function's presence in the C library
+ // at runtime, even if libc++ is built against an older libc
+ __attribute__((__weak__))
+#endif
+ int __cxa_thread_atexit_impl(Dtor, void*, void*);
+
+#ifndef
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 63232.
tavianator marked 6 inline comments as done.
tavianator added a comment.
- Bring back HAVE___CXA_THREAD_ATEXIT_IMPL, and avoid the weak symbol/fallback
implementation in that case
- Fix a leak in an error path
- Add a CreatesThreadLocalInDestructor to a non-main thread in the destructor
ordering test
http://reviews.llvm.org/D21803
Files:
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
test/thread_local_destruction_order.pass.cpp
Index: test/thread_local_destruction_order.pass.cpp
===
--- test/thread_local_destruction_order.pass.cpp
+++ test/thread_local_destruction_order.pass.cpp
@@ -0,0 +1,59 @@
+//===-- thread_local_destruction_order.pass.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+#include
+#include
+
+int seq = 0;
+
+class OrderChecker {
+public:
+ explicit OrderChecker(int n) : n_{n} { }
+
+ ~OrderChecker() {
+assert(seq++ == n_);
+ }
+
+private:
+ int n_;
+};
+
+class CreatesThreadLocalInDestructor {
+public:
+ CreatesThreadLocalInDestructor(int n) : n_{n} { }
+
+ ~CreatesThreadLocalInDestructor() {
+thread_local OrderChecker checker{n_};
+ }
+
+private:
+ int n_;
+};
+
+OrderChecker global{6};
+
+void thread_fn() {
+ static OrderChecker fn_static{4};
+ thread_local OrderChecker fn_thread_local{1};
+ thread_local CreatesThreadLocalInDestructor creates_tl{0};
+}
+
+int main() {
+ static OrderChecker fn_static{5};
+
+ std::thread{thread_fn}.join();
+ assert(seq == 2);
+
+ thread_local OrderChecker fn_thread_local{3};
+ thread_local CreatesThreadLocalInDestructor creates_tl{2};
+
+ return 0;
+}
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -7,20 +7,138 @@
//
//===--===//
+#include "abort_message.h"
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
-extern "C" {
-#ifdef HAVE___CXA_THREAD_ATEXIT_IMPL
+ using Dtor = void(*)(void*);
-_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(void (*dtor)(void *), void *obj,
-void *dso_symbol) throw() {
- extern int __cxa_thread_atexit_impl(void (*)(void *), void *, void *);
- return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
-}
+ extern "C"
+#ifndef HAVE___CXA_THREAD_ATEXIT_IMPL
+ // A weak symbol
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added inline comments.
Comment at: src/cxa_thread_atexit.cpp:47
@@ +46,3 @@
+// called during the loop.
+if (pthread_setspecific(dtors, ptr) != 0) {
+ abort_message("pthread_setspecific() failed during thread_local
destruction");
bcraig wrote:
> The loop doesn't read pthread_getspecific anymore. I get the need for the
> setspecific call here for your previous design, but I don't think it's needed
> now.
__cxa_thread_atexit() calls pthread_getspecific(), so it's still needed.
Otherwise it would create a new list instead of adding to the current one, and
the ordering would be wrong.
Comment at: src/cxa_thread_atexit.cpp:99
@@ +98,3 @@
+
+ if (__cxa_thread_atexit_impl) {
+return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
bcraig wrote:
> I'm going to have to agree with @majnemer. I think that the config check for
> LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL should stay in place. If
> cxa_thread_atexit_impl exists, then all of the fallback code can disappear at
> preprocessing time.
>
> We do lose out on the minor benefit of avoiding some libc++ recompiles, but
> we also avoid code bloat.
>
> For what it's worth, I'm willing to keep the weak symbol check in place if
> __cxa_thread_atexit_impl isn't present, I just don't want to pay for the
> fallback when I know I'm not going to use it.
Makes sense, I'll do that.
Comment at: test/thread_local_destruction_order.pass.cpp:48
@@ +47,3 @@
+ thread_local OrderChecker fn_thread_local{0};
+}
+
bcraig wrote:
> Can we have a CreatesThreadLocalInDestructor in the thread_fn as well? That
> way we can test both the main function and a pthread. If I understand your
> code and comments correctly, those go through different code paths.
Yep, meant to do that actually!
Comment at: test/thread_local_destruction_order.pass.cpp:54
@@ +53,3 @@
+ std::thread{thread_fn}.join();
+
+ thread_local OrderChecker fn_thread_local{2};
bcraig wrote:
> In the places where you can, validate that dtors actually are getting called.
> This may be your only place where you can do that.
>
> So something like 'assert(seq == 1)' here.
Sounds good. What I wanted to do was print some output in the destructors, and
check for a certain expected output. But I couldn't figure out how to do that
with lit.
http://reviews.llvm.org/D21803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 62691.
tavianator added a comment.
Added a test case for destructor ordering. Got rid of
pthread_{get,set}specific in a loop.
http://reviews.llvm.org/D21803
Files:
cmake/config-ix.cmake
src/CMakeLists.txt
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
test/thread_local_destruction_order.pass.cpp
Index: test/thread_local_destruction_order.pass.cpp
===
--- test/thread_local_destruction_order.pass.cpp
+++ test/thread_local_destruction_order.pass.cpp
@@ -0,0 +1,59 @@
+//===- cxa_thread_atexit_test.cpp -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+#include
+#include
+
+int seq = 0;
+
+class OrderChecker
+{
+public:
+ explicit OrderChecker(int n) : n_{n} { }
+
+ ~OrderChecker() {
+assert(seq++ == n_);
+ }
+
+private:
+ int n_;
+};
+
+class CreatesThreadLocalInDestructor
+{
+public:
+ CreatesThreadLocalInDestructor(int n) : n_{n} { }
+
+ ~CreatesThreadLocalInDestructor() {
+thread_local OrderChecker checker{n_};
+ }
+
+private:
+ int n_;
+};
+
+OrderChecker global{5};
+
+void thread_fn() {
+ static OrderChecker fn_static{3};
+ thread_local OrderChecker fn_thread_local{0};
+}
+
+int main() {
+ static OrderChecker fn_static{4};
+
+ std::thread{thread_fn}.join();
+
+ thread_local OrderChecker fn_thread_local{2};
+ thread_local CreatesThreadLocalInDestructor creates_tl{1};
+
+ return 0;
+}
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -7,20 +7,126 @@
//
//===--===//
+#include "abort_message.h"
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
+namespace {
+ // This implementation is used if the C library does not provide
+ // __cxa_thread_atexit_impl() for us. It has a number of limitations that are
+ // difficult to impossible to address without ..._impl():
+ //
+ // - dso_symbol is ignored. This means that a shared library may be unloaded
+ // (via dlclose()) before its thread_local destructors have run.
+ //
+ // - thread_local destructors for the main thread are run by the destructor of
+ // a static object. This is later than expected; they should run before the
+ // destructors of any objects with static storage duration.
+ //
+ // - thread_local destructors on other
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added inline comments.
Comment at: src/cxa_thread_atexit.cpp:47
@@ +46,3 @@
+ void run_dtors(void* ptr) {
+if (pthread_setspecific(dtors, ptr) != 0) {
+ abort_message("pthread_setspecific() failed during thread_local
destruction");
bcraig wrote:
> Why are we doing this? I can see it being a little useful when debugging /
> developing, so that you get an early warning that something has gone wrong,
> but it seems like this will always be setting a value to the value it already
> has.
pthread_key destructors run after the key is set to null. I re-set it here
since the loop reads the key.
Comment at: src/cxa_thread_atexit.cpp:54
@@ +53,3 @@
+// __cxa_thread_atexit() is called during the loop.
+while (auto elem = static_cast(pthread_getspecific(dtors))) {
+ if (pthread_setspecific(dtors, elem->next) != 0) {
bcraig wrote:
> Maybe this concern is unfounded, but I'm not overly fond of
> pthread_getspecific and setspecific in a loop. I've always been under the
> impression that those functions are rather slow. Could we add a layer of
> indirection so that we don't need to call getspecific and setspecific so
> often? Basically make the pointer that is directly stored in TLS an
> immutable pointer to pointer.
Sure, I can do that. Would reduce the number of setspecific() calls in
__cxa_thread_atexit too.
http://reviews.llvm.org/D21803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 62394.
tavianator added a comment.
Fix copy-pasta that result in an infinite loop.
http://reviews.llvm.org/D21803
Files:
cmake/config-ix.cmake
src/CMakeLists.txt
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -7,20 +7,137 @@
//
//===--===//
+#include "abort_message.h"
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
+namespace {
+ // This implementation is used if the C library does not provide
+ // __cxa_thread_atexit_impl() for us. It has a number of limitations that are
+ // difficult to impossible to address without ..._impl():
+ //
+ // - dso_symbol is ignored. This means that a shared library may be unloaded
+ // (via dlclose()) before its thread_local destructors have run.
+ //
+ // - thread_local destructors for the main thread are run with __cxa_atexit().
+ // This is later than expected; they should run before the destructors of
+ // any objects with static storage duration.
+ //
+ // - thread_local destructors on other threads run on the first iteration
+ // through the pthread_key destructors. std::notify_all_at_thread_exit()
+ // and similar functions must be careful to wait until the second iteration
+ // to provide their indended ordering guarantees.
+
+ typedef void (*Dtor)(void*);
+
+ struct DtorList {
+Dtor dtor;
+void* obj;
+DtorList* next;
+ };
+
+ pthread_key_t dtors;
+ pthread_once_t dtors_once = PTHREAD_ONCE_INIT;
+ bool dtors_ready = false;
+ bool dtors_atexit = false;
+
+ void run_dtors(void* ptr) {
+if (pthread_setspecific(dtors, ptr) != 0) {
+ abort_message("pthread_setspecific() failed during thread_local destruction");
+}
+
+// Rather than iterate over the list directly, use the pthread_key to get
+// the head of the list every time. This gives the correct ordering if
+// __cxa_thread_atexit() is called during the loop.
+while (auto elem = static_cast(pthread_getspecific(dtors))) {
+ if (pthread_setspecific(dtors, elem->next) != 0) {
+abort_message("pthread_setspecific() failed during thread_local destruction");
+ }
+ elem->dtor(elem->obj);
+ std::free(elem);
+}
+ }
+
+ void run_dtors_atexit(void*) {
+// Signify that we need to re-register this function if any new
+// thread_locals are created.
+dtors_atexit = false;
+
+auto ptr = pthread_getspecific(dtors);
+run_dtors(ptr);
+ }
+
+ // This is the DSO handle for libc++abi.so itself
+ extern "C" void* __dso_handle;
+
+
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 62388.
tavianator added a comment.
Added missing __dso_handle declaration.
http://reviews.llvm.org/D21803
Files:
cmake/config-ix.cmake
src/CMakeLists.txt
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -7,20 +7,137 @@
//
//===--===//
+#include "abort_message.h"
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
+namespace {
+ // This implementation is used if the C library does not provide
+ // __cxa_thread_atexit_impl() for us. It has a number of limitations that are
+ // difficult to impossible to address without ..._impl():
+ //
+ // - dso_symbol is ignored. This means that a shared library may be unloaded
+ // (via dlclose()) before its thread_local destructors have run.
+ //
+ // - thread_local destructors for the main thread are run with __cxa_atexit().
+ // This is later than expected; they should run before the destructors of
+ // any objects with static storage duration.
+ //
+ // - thread_local destructors on other threads run on the first iteration
+ // through the pthread_key destructors. std::notify_all_at_thread_exit()
+ // and similar functions must be careful to wait until the second iteration
+ // to provide their indended ordering guarantees.
+
+ typedef void (*Dtor)(void*);
+
+ struct DtorList {
+Dtor dtor;
+void* obj;
+DtorList* next;
+ };
+
+ pthread_key_t dtors;
+ pthread_once_t dtors_once = PTHREAD_ONCE_INIT;
+ bool dtors_ready = false;
+ bool dtors_atexit = false;
+
+ void run_dtors(void* ptr) {
+if (pthread_setspecific(dtors, ptr) != 0) {
+ abort_message("pthread_setspecific() failed during thread_local destruction");
+}
+
+// Rather than iterate over the list directly, use the pthread_key to get
+// the head of the list every time. This gives the correct ordering if
+// __cxa_thread_atexit() is called during the loop.
+while (auto elem = static_cast(pthread_getspecific(dtors))) {
+ if (pthread_setspecific(dtors, ptr) != 0) {
+abort_message("pthread_setspecific() failed during thread_local destruction");
+ }
+ elem->dtor(elem->obj);
+ std::free(elem);
+}
+ }
+
+ void run_dtors_atexit(void*) {
+// Signify that we need to re-register this function if any new
+// thread_locals are created.
+dtors_atexit = false;
+
+auto ptr = pthread_getspecific(dtors);
+run_dtors(ptr);
+ }
+
+ // This is the DSO handle for libc++abi.so itself
+ extern "C" void* __dso_handle;
+
+ void
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator marked 3 inline comments as done.
tavianator added a comment.
In http://reviews.llvm.org/D21803#470448, @bcraig wrote:
> What that means for this implementation is that I think that
> _cxa_thread_atexit is allowed to be called during run_dtors. If running the
> dtor for a thread local variable 'cat', we encounter a previously unseen
> thread_local 'dog', the compiler will call the ctor, then register the dtor
> with _cxa_thread_atexit. Since it is the most recently constructed thread
> local object, I would expect the 'dog' dtor to be the next dtor to be run.
> You may be able to support this just by moving "elem = elem->next" below the
> dtor invocation.
It wasn't quite that easy (have to re-look at the pthread_key to get newly
added thread_locals), but that's done in the latest patch.
Comment at: src/cxa_thread_atexit.cpp:115
@@ +114,3 @@
+if (!dtors_atexit) {
+ if (__cxa_atexit(run_dtors_atexit, NULL, __dso_handle) != 0) {
+return -1;
See http://stackoverflow.com/q/38130185/502399 for a test case that would
trigger this. This may not be necessary depending on the answer to that
question.
Comment at: src/cxa_thread_atexit.cpp:122
@@ +121,3 @@
+auto head = static_cast(std::malloc(sizeof(DtorList)));
+if (!head) {
+ return -1;
This has changed somewhat in the latest patch, but the gist is similar. If
libc++abi.so is dlclose()d, there had better not be any still-running threads
that expect to execute thread_local destructors (or any other C++ code, for
that matter). In the usual case (libc++abi.so loaded at startup, not by a
later dlopen()), the last run_dtors() call happens as the final thread is
exiting.
http://reviews.llvm.org/D21803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 62386.
tavianator added a comment.
Fixed some corner cases regarding destruction order and very-late-initialized
thread_locals. Explicitly documented the known limitations compared to
__cxa_thread_atexit_impl().
http://reviews.llvm.org/D21803
Files:
cmake/config-ix.cmake
src/CMakeLists.txt
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -7,20 +7,134 @@
//
//===--===//
+#include "abort_message.h"
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
+namespace {
+ // This implementation is used if the C library does not provide
+ // __cxa_thread_atexit_impl() for us. It has a number of limitations that are
+ // difficult to impossible to address without ..._impl():
+ //
+ // - dso_symbol is ignored. This means that a shared library may be unloaded
+ // (via dlclose()) before its thread_local destructors have run.
+ //
+ // - thread_local destructors for the main thread are run with __cxa_atexit().
+ // This is later than expected; they should run before the destructors of
+ // any objects with static storage duration.
+ //
+ // - thread_local destructors on other threads run on the first iteration
+ // through the pthread_key destructors. std::notify_all_at_thread_exit()
+ // and similar functions must be careful to wait until the second iteration
+ // to provide their indended ordering guarantees.
+
+ typedef void (*Dtor)(void*);
+
+ struct DtorList {
+Dtor dtor;
+void* obj;
+DtorList* next;
+ };
+
+ pthread_key_t dtors;
+ pthread_once_t dtors_once = PTHREAD_ONCE_INIT;
+ bool dtors_ready = false;
+ bool dtors_atexit = false;
+
+ void run_dtors(void* ptr) {
+if (pthread_setspecific(dtors, ptr) != 0) {
+ abort_message("pthread_setspecific() failed during thread_local destruction");
+}
+
+// Rather than iterate over the list directly, use the pthread_key to get
+// the head of the list every time. This gives the correct ordering if
+// __cxa_thread_atexit() is called during the loop.
+while (auto elem = static_cast(pthread_getspecific(dtors))) {
+ if (pthread_setspecific(dtors, ptr) != 0) {
+abort_message("pthread_setspecific() failed during thread_local destruction");
+ }
+ elem->dtor(elem->obj);
+ std::free(elem);
+}
+ }
+
+ void run_dtors_atexit(void*) {
+// Signify that we need to re-register this function if any new
+// thread_locals are created.
+dtors_atexit = false;
+
+auto ptr =
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added a comment. In http://reviews.llvm.org/D21803#469988, @bcraig wrote: > You should look at __thread_specific_ptr in libcxx's . It does a lot > of these things in order to satisfy the requirements of > notify_all_at_thread_exit, set_value_at_thread_exit, and > make_ready_at_thread_exit. Had a look at it. One thing that stands out is that notify_all_at_thread_exit() and friends are supposed to be invoked *after* thread_local destructors. But the order that pthread_key destructors run in is unspecified. This could be worked around by waiting for the second iteration through pthread_key destructors before triggering ~__thread_struct_imp(). It looks like libstdc++ has a similar bug if ..._impl() isn't available. > @rmaprath has been doing some work to make the threading runtime library > swappable. I don't recall if his work extended to libcxxabi or not, but I'll > page him anyway. <__threading_support>? Seems to be libc++-specific. There's a few other raw uses of pthreads in libc++abi. > This implementation of __cxa_thread_atexit doesn't interact nicely with > shared libraries. The following sequence of events causes unloaded code to > get invoked. > > - Create thread 42 > - Load library foo from thread 42 > - Call a function with a thread_local object with a dtor. > - Unload library foo. > - Join thread 42 > > glibc does some extra work during __cxa_thread_atexit_impl to bump the > reference count of the shared library so that the user's "unload" doesn't > actually unload the library. Yep. This is about as good as libc++abi can do on its own though. Note that libstdc++ has similar limitations if ..._impl() isn't available. http://reviews.llvm.org/D21803 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator updated this revision to Diff 62218.
tavianator added a comment.
Added error handling for pthread_key uses
http://reviews.llvm.org/D21803
Files:
cmake/config-ix.cmake
src/CMakeLists.txt
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -8,19 +8,85 @@
//===--===//
#include "cxxabi.h"
+#include
+#include
+#include "abort_message.h"
namespace __cxxabiv1 {
-extern "C" {
+namespace {
+ typedef void (*Dtor)(void *);
+
+ struct DtorList {
+Dtor dtor;
+void* obj;
+DtorList* next;
+ };
+
+ void run_dtors(void* ptr) {
+auto elem = static_cast(ptr);
+while (elem) {
+ auto saved = elem;
+ elem = elem->next;
+ saved->dtor(saved->obj);
+ std::free(saved);
+}
+ }
+
+ class DtorListHolder {
+ public:
+DtorListHolder() {
+ if (pthread_key_create(_, run_dtors) != 0) {
+abort_message("cannot create pthread key for __cxa_thread_atexit()");
+ }
+}
+
+~DtorListHolder() {
+ run_dtors(get());
+ pthread_key_delete(key_);
+}
+
+DtorList* get() {
+ return static_cast(pthread_getspecific(key_));
+}
+
+bool set(DtorList* list) {
+ return pthread_setspecific(key_, list) == 0;
+}
+
+ private:
+pthread_key_t key_;
+ };
+} // namespace
-#ifdef HAVE___CXA_THREAD_ATEXIT_IMPL
+extern "C" {
-_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(void (*dtor)(void *), void *obj,
+_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(Dtor dtor, void *obj,
void *dso_symbol) throw() {
- extern int __cxa_thread_atexit_impl(void (*)(void *), void *, void *);
- return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
-}
+ extern int __cxa_thread_atexit_impl(Dtor, void *, void *)
+__attribute__((__weak__));
+
+ if (__cxa_thread_atexit_impl) {
+return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
+ } else {
+static DtorListHolder dtors;
+
+auto head = static_cast(std::malloc(sizeof(DtorList)));
+if (!head) {
+ return -1;
+}
+
+head->dtor = dtor;
+head->obj = obj;
+head->next = dtors.get();
+
+if (!dtors.set(head)) {
+ std::free(head);
+ return -1;
+}
-#endif // HAVE__CXA_THREAD_ATEXIT_IMPL
+return 0;
+ }
+}
} // extern "C"
} // namespace __cxxabiv1
Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -41,10 +41,6 @@
include_directories("${LIBCXXABI_LIBCXX_INCLUDES}")
-if
Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator added inline comments.
Comment at: src/cxa_thread_atexit.cpp:36-47
@@ +35,14 @@
+ public:
+DtorListHolder() {
+ pthread_key_create(_, run_dtors);
+}
+
+~DtorListHolder() {
+ run_dtors(get());
+ pthread_key_delete(key_);
+}
+
+DtorList* get() {
+ return static_cast(pthread_getspecific(key_));
+}
+
majnemer wrote:
> What happens if `pthread_key_create` fails?
Nothing good! I'll add the necessary error handling.
Comment at: src/cxa_thread_atexit.cpp:61-67
@@ -18,6 +60,9 @@
+_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(Dtor dtor, void *obj,
void *dso_symbol) throw() {
- extern int __cxa_thread_atexit_impl(void (*)(void *), void *, void *);
- return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
-}
+ extern int __cxa_thread_atexit_impl(Dtor, void *, void *)
+__attribute__((__weak__));
-#endif // HAVE__CXA_THREAD_ATEXIT_IMPL
+ if (__cxa_thread_atexit_impl) {
+return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
+ } else {
+static DtorListHolder dtors;
majnemer wrote:
> Is there a reason to use the weak symbol on non-Android platforms?
Just simplicity. (There are some fringe benefits, like the ability to build
against pre-2.18 glibc but have it detect ..._impl() support when run with
newer libraries. Or magically supporting musl etc. if they add it, without
having to recompile.)
Would you rather keep the build-time checks for non-Android platforms?
http://reviews.llvm.org/D21803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation
tavianator created this revision.
tavianator added reviewers: danalbert, jroelofs.
tavianator added a subscriber: cfe-commits.
Herald added subscribers: danalbert, tberghammer.
__cxa_thread_atexit_impl() isn't present on all platforms, for example
Android pre-6.0. This patch uses a weak symbol to detect _impl()
support, falling back to a pthread_key_t-based implementation.
http://reviews.llvm.org/D21803
Files:
cmake/config-ix.cmake
src/CMakeLists.txt
src/cxa_thread_atexit.cpp
test/CMakeLists.txt
test/cxa_thread_atexit_test.pass.cpp
test/libcxxabi/test/config.py
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -13,7 +13,6 @@
config.enable_32bit = "@LIBCXXABI_BUILD_32_BITS@"
config.target_info = "@LIBCXXABI_TARGET_INFO@"
config.executor = "@LIBCXXABI_EXECUTOR@"
-config.thread_atexit= "@LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL@"
config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_exceptions= "@LIBCXXABI_ENABLE_EXCEPTIONS@"
Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -37,8 +37,6 @@
super(Configuration, self).configure_features()
if not self.get_lit_bool('enable_exceptions', True):
self.config.available_features.add('libcxxabi-no-exceptions')
-if self.get_lit_bool('thread_atexit', True):
-self.config.available_features.add('thread_atexit')
def configure_compile_flags(self):
self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
Index: test/cxa_thread_atexit_test.pass.cpp
===
--- test/cxa_thread_atexit_test.pass.cpp
+++ test/cxa_thread_atexit_test.pass.cpp
@@ -8,7 +8,6 @@
//===--===//
// REQUIRES: linux
-// REQUIRES: thread_atexit
#include
#include
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -16,7 +16,6 @@
pythonize_bool(LIBCXXABI_ENABLE_THREADS)
pythonize_bool(LIBCXXABI_ENABLE_EXCEPTIONS)
pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER)
-pythonize_bool(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
"TargetInfo to use when setting up test environment.")
set(LIBCXXABI_EXECUTOR "None" CACHE STRING
Index: src/cxa_thread_atexit.cpp
===
--- src/cxa_thread_atexit.cpp
+++ src/cxa_thread_atexit.cpp
@@ -8,19 +8,77 @@
//===--===//
#include "cxxabi.h"
+#include
+#include
namespace __cxxabiv1 {
-extern "C" {
+namespace {
+ typedef void (*Dtor)(void *);
+
+ struct DtorList {
+Dtor dtor;
+void* obj;
+DtorList* next;
+ };
+
+ void run_dtors(void* ptr) {
+auto elem = static_cast(ptr);
+while (elem) {
+ auto saved = elem;
+ elem = elem->next;
+ saved->dtor(saved->obj);
+ std::free(saved);
+}
+ }
+
+ class DtorListHolder {
+ public:
+DtorListHolder() {
+ pthread_key_create(_, run_dtors);
+}
+
+~DtorListHolder() {
+ run_dtors(get());
+ pthread_key_delete(key_);
+}
+
+DtorList* get() {
+ return static_cast(pthread_getspecific(key_));
+}
+
+void set(DtorList* list) {
+ pthread_setspecific(key_, list);
+}
+
+ private:
+pthread_key_t key_;
+ };
+} // namespace
-#ifdef HAVE___CXA_THREAD_ATEXIT_IMPL
+extern "C" {
-_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(void (*dtor)(void *), void *obj,
+_LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(Dtor dtor, void *obj,
void *dso_symbol) throw() {
- extern int __cxa_thread_atexit_impl(void (*)(void *), void *, void *);
- return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
-}
+ extern int __cxa_thread_atexit_impl(Dtor, void *, void *)
+__attribute__((__weak__));
-#endif // HAVE__CXA_THREAD_ATEXIT_IMPL
+ if (__cxa_thread_atexit_impl) {
+return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
+ } else {
+static DtorListHolder dtors;
+
+auto head = static_cast(std::malloc(sizeof(DtorList)));
+if (!head) {
+ return -1;
+}
+
+head->dtor = dtor;
+head->obj = obj;
+head->next = dtors.get();
+dtors.set(head);
+return 0;
+ }
+}
} // extern "C"
} // namespace __cxxabiv1
Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -41,10 +41,6 @@
Re: [PATCH] D12834: add gcc abi_tag support
tavianator added a subscriber: tavianator. Comment at: lib/AST/ItaniumMangle.cpp:1120 @@ -880,2 +1119,3 @@ mangleSourceName(qualifier->getAsIdentifier()); +writeAbiTags(qualifier->getAsNamespaceAlias()); break; This looks bogus, should be `writeAbiTags(qualifier)` right? Repository: rL LLVM http://reviews.llvm.org/D12834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
