[cpullvm] Force a consistent TLS model for all library builds using picolibc#357
Merged
Jonathon Penix (jonathonpenix) merged 1 commit intoMay 26, 2026
Conversation
Jonathon Penix (jonathonpenix)
requested review from
Ana Pazos (apazos) and
pzhengqc
Contributor
Author
|
CC'ing Shreeyash Pandey (@bojle) since I can't add you as a reviewer directly (I think you need to join the Qualcomm Github org before we can?) |
Contributor
|
Our musl-embedded is not thread safe. We can ignore it. |
| @@ -19,8 +20,7 @@ | |||
| "ENABLE_CXX_LIBS": "ON", | |||
| "ENABLE_LIBC_TESTS": "ON", | |||
| "ENABLE_COMPILER_RT_TESTS": "OFF", | |||
| "ENABLE_LIBCXX_TESTS": "OFF", | |||
| "TLS_MODEL": "initial-exec" | |||
| "ENABLE_LIBCXX_TESTS": "OFF" | |||
| }, | |||
| "musl-embedded": { | |||
Contributor
There was a problem hiding this comment.
let's move musl-embedded to a separate config where no TLS model is defined.
Contributor
|
Looks good to me, I'll try rebasing this patch and testing the libcxxabi patch with it |
Jonathon Penix (jonathonpenix)
changed the title
Contributor
|
Thanks Jonathon Penix (@jonathonpenix) , we are fine with excluding the deprecated musl-embedded. |
…icolibc Previously, a TLS model was only ever explicitly set for picolibc--other projects (libc++, etc.) would use whatever the compiler selected. This shouldn't necessarily be a problem, but in practice can cause issues when global-dynamic is selected and is implemented via calls to __tls_get_addr (ex: in our 32bit Arm and 32/64bit RISC-V PIC configs). Given we don't provide an implementation and we don't expect people to use a dynamic linker that would implement this, people will generally get undefined reference errors. See the discussion in qualcomm#341. Note that this seems to primarily be an issue with picolibc as it uses "normal" TLS mechanisms [1]. musl-embedded doesn't seem to (see ex: __errno_location) and consequently I don't see any obvious TLS usage in any of our libraries built with musl-embedded as the libc. To try and address this, consistently specify a TLS model across all of the libraries we are building when using picolibc. For musl-embedded, given that a) it is generally not expected to be thread safe b) it doesn't seem to use the "usual" TLS mechanisms from the toolchain as mentioned above and c) historically we haven't had any special handling downstream for this, musl-embedded configs are intentionally excluded here. Since we're special casing the flag on the libc type now, at the same time add basic error checking so we aren't silently dropping this if it is specified via JSON. [1] https://github.com/picolibc/picolibc/blob/main/doc/tls.md Signed-off-by: Jonathon Penix <jpenix@qti.qualcomm.com>
Jonathon Penix (jonathonpenix)
force-pushed
the
pr/tls_model
branch
from
46601ea to
0c26dc6
Compare
Ana Pazos (apazos)
approved these changes
May 26, 2026
Jonathon Penix (jonathonpenix)
merged commit 085a7ca
into
qualcomm:qualcomm-software
9 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, a TLS model was only ever explicitly set for picolibc--other projects (libc++, etc.) would use whatever the compiler selected. This shouldn't necessarily be a problem, but in practice can cause issues when global-dynamic is selected and is implemented via calls to
__tls_get_addr(ex: in our 32bit Arm and 32/64bit RISC-V PIC configs). Given we don't provide an implementation and we don't expect people to use a dynamic linker that would implement this, people will generally get undefined reference errors. See the discussion in #341.Note that this seems to primarily be an issue with picolibc as it uses "normal" TLS mechanisms [1]. musl-embedded doesn't seem to (see ex:
__errno_location) and consequently I don't see any obvious TLS usage in any of our libraries built with musl-embedded as the libc.To try and address this, consistently specify a TLS model across all of the libraries we are building when using picolibc.
For musl-embedded, given that a) it is generally not expected to be thread safe b) it doesn't seem to use the "usual" TLS mechanisms from the toolchain as mentioned above and c) historically we haven't had any special handling downstream for this, musl-embedded configs are intentionally excluded here.
Since we're special casing the flag on the libc type now, at the same time add basic error checking so we aren't silently dropping this if it is specified via JSON.
[1] https://github.com/picolibc/picolibc/blob/main/doc/tls.md