Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[libcxxabi] HAS_THREAD_LOCAL is not defined by cmake #78207

Open
12101111 opened this issue Jan 15, 2024 · 3 comments · May be fixed by #79868
Open

[libcxxabi] HAS_THREAD_LOCAL is not defined by cmake #78207

12101111 opened this issue Jan 15, 2024 · 3 comments · May be fixed by #79868
Assignees
@ldionne
Labels
libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@12101111
Copy link

libcxxabi/src/cxa_exception_storage.cpp contain a check for HAS_THREAD_LOCAL, but the flags is set by compiler-rt/lib/fuzzer/CMakeLists.txt. This cmake script is not used in normal build of libcxxabi.

Maybe it should be replaced by __has_feature(cxx_thread_local)?

See also: https://reviews.llvm.org/D155278
@github-actions github-actions bot added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc++abi libc++abi C++ Runtime Library. Not libc++. labels Jan 15, 2024
@ldionne
Copy link
Member

ldionne commented Jan 16, 2024

CC @kalle-llvm @mstorsjo

Do you folks know if thread_local is available everywhere? It's technically required for C++11 conformance, so I'd expect it to be available on all platforms when threads are enabled?

@kalle-llvm
Copy link
Contributor

Do you folks know if thread_local is available everywhere?

@ldionne Can't say about other platforms but it's available on mingw. I've been using libc++ with HAS_THREAD_LOCAL for several months without problems.

@mstorsjo mstorsjo mentioned this issue Jan 27, 2024
Closed
@ldionne
Copy link
Member

ldionne commented Jan 29, 2024

Hmm, this is related to https://reviews.llvm.org/D138461. I'd be tempted to do the naive thing and try landing https://reviews.llvm.org/D138461 again.

ldionne added a commit to ldionne/llvm-project that referenced this issue Jan 29, 2024
@ldionne @smeenai
[libc++abi] Always use thread_local for cxa_exception_storage
5a8eefe 
This was previously guarded by HAS_THREAD_LOCAL, which was never set by
CMake and had to be specified manually. Android has been setting this to
solve android/ndk#1200 [1], but every compiler
and platform libc++abi supports should have thread_local by now, so we
can just get rid of the fallback implementation and simplify things
significantly (including removing the now unused fallback calloc).

This is a re-application of https://reviews.llvm.org/D138461.
Fixes llvm#78207.

[1]: https://android-review.googlesource.com/c/toolchain/llvm-project/+/1285596

Co-Authored-By: Shoaib Meenai <smeenai@fb.com>
@ldionne ldionne linked a pull request Jan 29, 2024 that will close this issue
Open
@ldionne ldionne self-assigned this Jan 29, 2024
ldionne added a commit to ldionne/llvm-project that referenced this issue Jan 30, 2024
@ldionne @smeenai
[libc++abi] Always use thread_local for cxa_exception_storage
b203586 
This was previously guarded by HAS_THREAD_LOCAL, which was never set by
CMake and had to be specified manually. Android has been setting this to
solve android/ndk#1200 [1], but every compiler
and platform libc++abi supports should have thread_local by now, so we
can just get rid of the fallback implementation and simplify things
significantly (including removing the now unused fallback calloc).

This is a re-application of https://reviews.llvm.org/D138461.
Fixes llvm#78207.

[1]: https://android-review.googlesource.com/c/toolchain/llvm-project/+/1285596

Co-Authored-By: Shoaib Meenai <smeenai@fb.com>
ldionne added a commit to ldionne/llvm-project that referenced this issue Feb 5, 2024
@ldionne @smeenai
[libc++abi] Always use thread_local for cxa_exception_storage
a764f11 
This was previously guarded by HAS_THREAD_LOCAL, which was never set by
CMake and had to be specified manually. Android has been setting this to
solve android/ndk#1200 [1], but every compiler
and platform libc++abi supports should have thread_local by now, so we
can just get rid of the fallback implementation and simplify things
significantly (including removing the now unused fallback calloc).

This is a re-application of https://reviews.llvm.org/D138461.
Fixes llvm#78207.

[1]: https://android-review.googlesource.com/c/toolchain/llvm-project/+/1285596

Co-Authored-By: Shoaib Meenai <smeenai@fb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ldionne ldionne

Labels
libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[libc++abi] Always use thread_local for cxa_exception_storage ldionne/llvm-project
3 participants
@ldionne @12101111 @kalle-llvm