[cpullvm] Enable libcxxabi tests#341
Conversation
Shreeyash Pandey (bojle)
changed the title
|
Ana Pazos (@apazos) Pawan Nirpal (@pawan-nirpal-031) Garvit Gupta (@quic-garvgupt) can you take a look at this? |
|
Thanks Shreeyash Pandey (@bojle), as you observed, we should rebase the patch after #340 is merged. Let's also separate your changes by placing the CMake build commands in a different patch. Pranav will handle running tests and generating failure logs, and test failures will not block other host builds. If there are multilib variants missing to run the tests, we can consider adding them and avoid disabling the tests. As far as I know there are no issues with QEMU installation we have in the builders, and failures with Xqci QEMU should be investigated. Please make sure the right QEMU is being invoked. Please reach out to Sudharsan. |
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. To try and address this, consistently specify a TLS model across all of the libraries we are building. 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. However, I don't know if this holds true in general (if ex: libc++ itself will never have "normal" TLS accesses of it's own). And, given that the usecases we expect for our picolibc variants are the same as for our musl-embedded variants (and these expectations in turn match the restrictions imposed by the given TLS model), I'm erring on the side of specifying the TLS model for variants which use musl-embedded as well. [1] https://github.com/picolibc/picolibc/blob/main/doc/tls.md Signed-off-by: Jonathon Penix <jpenix@qti.qualcomm.com>
Shreeyash Pandey (bojle)
force-pushed
the
libcxxabi
branch
from
5bc3e36 to
fdb4117
Compare
|
Shreeyash Pandey (@bojle) please update commit message to include libcxxabi and libunwind |
There was a problem hiding this comment.
Shreeyash Pandey (@bojle) please remove this change; Jonathon is pushing the fix for building C++ libs which fixes this issue.
There was a problem hiding this comment.
Shreeyash Pandey (@bojle), remove this change, no need to xfail the tests. Pawan Nirpal (@pawan-nirpal-031) has a fix for the QEMU issue and he will push it as a separate PR.
There was a problem hiding this comment.
This is related to #354. I don't think there's a patch up for that issue yet. Jonathon Penix (@jonathonpenix) is aware of this.
There was a problem hiding this comment.
Remove the change and sync up with Pawan Nirpal (@pawan-nirpal-031), he pushing the fix for QEMU in separate patch.
There was a problem hiding this comment.
Shreeyash Pandey (@bojle) please remove this change, Pawan Nirpal (@pawan-nirpal-031) has this fix for all C++ tests
|
Right now, this patch is dependent on these other patches: |
…icolibc (#377) 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 Signed-off-by: Jonathon Penix <jpenix@qti.qualcomm.com>
Shreeyash Pandey (bojle)
changed the title
…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>
…icolibc (#357) 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 Signed-off-by: Jonathon Penix <jpenix@qti.qualcomm.com>
Shreeyash Pandey (bojle)
force-pushed
the
libcxxabi
branch
from
7ffa71a to
eabaa1b
Compare
|
Hi Ana Pazos (@apazos) Jonathon Penix (@jonathonpenix), could you please review this? The build failures are the same as (https://github.com/qualcomm/cpullvm-toolchain/actions/runs/27567168746/job/81494042988) and should resolve once that issue is fixed. I have rebased all dependent patches and retested. Currently, cxxabi needs only one xfail (due to missing atomics in non-A nothreads riscv variants) and a minor change in libc++abi.cfg.in, which I can submit in a separate patch if needed. |
Jonathon Penix (jonathonpenix)
left a comment
Jonathon Penix (jonathonpenix)
left a comment
There was a problem hiding this comment.
Can we add armv7m_hard_fpv5_d16_nopic as well? It has both ENABLE_CXX_LIBS and ENABLE_LIBC_TESTS turned on
Besides that, this LGTM. Thanks!
(But I think this still has to wait to merge until the libc++ patches go in?)
Shreeyash Pandey (bojle)
force-pushed
the
libcxxabi
branch
from
bdd8cba to
639349c
Compare
|
Jonathon Penix (@jonathonpenix) CI failures right now are due to cxx tests, these are enabled as cxx and cxxabi are enabled via the same option ENABLE_LIBCXX_TESTS |
Signed-off-by: Shreeyash Pandey <shrpand@qti.qualcomm.com>
Signed-off-by: Shreeyash Pandey <shrpand@qti.qualcomm.com>
Signed-off-by: Shreeyash Pandey <shrpand@qti.qualcomm.com>
Signed-off-by: Shreeyash Pandey <shrpand@qti.qualcomm.com>
Signed-off-by: Shreeyash Pandey <shrpand@qti.qualcomm.com>
Signed-off-by: Shreeyash Pandey <shrpand@qti.qualcomm.com>
Shreeyash Pandey (bojle)
force-pushed
the
libcxxabi
branch
from
787a691 to
e49d87a
Compare
3 participants
Enable ENABLE_LIBCXX_TESTS for all embedded picolibc variants that have CXX libs and libc tests already enabled (ENABLE_CXX_LIBS=ON, ENABLE_LIBC_TESTS=ON). This covers ARM, AArch64 and RISC-V variants and allows running the libcxxabi test suite against them via QEMU semihosting