Created attachment 866451 [details, diff] patch Currently, net-libs/nodejs hard depends on libgcc with this reasoning in the ebuild: # nodejs unconditionally links to libatomic #869992 # specifically it requires __atomic_is_lock_free which # is not yet implemented by sys-libs/compiler-rt (see # https://reviews.llvm.org/D85044?id=287068), therefore # we depend on gcc and force using libgcc as the support lib However, it seems that it was implemented in this commit: https://reviews.llvm.org/rG00530dee5d1295dc20ebafdd9a8a79662f41513e There is a note about the linked PR, saying "This function is also added in D85044, but that review also adds support for using lock-free atomics in more cases, whereas this is a minimal change that just adds __atomic_is_lock_free() for the implementation of atomic.c." However, nodejs builds and appears to function fine (all tests pass) with the unconditional link to libatomic removed (via the attached patch). Is this something that would be worthwhile raising upstream?
Nodejs 16.x is almost to end of life, but yes, if this is still an issue with nodejs 18 and 20, please file a bug upstream and link it to this bug. Thanks much. William
Let me take a look first.
(In reply to William Hubbs from comment #1) > Nodejs 16.x is almost to end of life, but yes, if this is still an issue > with nodejs 18 and 20, please file a bug upstream and link it to this > bug. > > Thanks much. > > William The patch was just named nodejs-16.10.0, it works with (and is still an issue in) latest version though. Sorry for the confusion.
Also see Chimera Linux's packaging of nodejs: https://github.com/chimera-linux/cports/tree/master/contrib/nodejs. They use nearly the same patch and support many architectures, so it should be a good reference until someone can properly test w/ gentoo.
Sam, moyjack, I guess that's where we want to continue the discussion related to https://github.com/nodejs/node/pull/54306 To double check that enabling compiler-rt is enough for getting atomics to work, I tried writing the following small programs: $ cat atomic.c #include <stdio.h> #include <stdatomic.h> int main() { atomic_int atomicInt = 0; atomic_llong atomicLongLong = 0; _Atomic double atomicDouble = 0.0; // Check if atomic operations are lock-free if (atomic_is_lock_free(&atomicInt)) { printf("atomic<int> is lock-free.\n"); } else { printf("atomic<int> is not lock-free.\n"); } if (atomic_is_lock_free(&atomicLongLong)) { printf("atomic<long long> is lock-free.\n"); } else { printf("atomic<long long> is not lock-free.\n"); } if (atomic_is_lock_free(&atomicDouble)) { printf("atomic<double> is lock-free.\n"); } else { printf("atomic<double> is not lock-free.\n"); } return 0; } $ cat atomic.cpp #include <iostream> #include <atomic> int main() { std::atomic<int> atomicInt(0); std::atomic<long long> atomicLongLong(0); std::atomic<double> atomicDouble(0.0); // Check if the atomic operations are lock-free if (atomic_is_lock_free(&atomicInt)) { std::cout << "atomic<int> is lock-free.\n"; } else { std::cout << "atomic<int> is not lock-free.\n"; } if (atomic_is_lock_free(&atomicLongLong)) { std::cout << "atomic<long long> is lock-free.\n"; } else { std::cout << "atomic<long long> is not lock-free.\n"; } if (atomic_is_lock_free(&atomicDouble)) { std::cout << "atomic<double> is lock-free.\n"; } else { std::cout << "atomic<double> is not lock-free.\n"; } return 0; } And I also re-used your program from Sam's comment https://bugs.gentoo.org/870919#c14 $ cat atomic.cxx #include <stdio.h> #include <stdatomic.h> _Atomic struct A { int a[100]; } a; _Atomic struct B { int x, y; } b; int main(void) { printf("_Atomic struct A is lock free? %s\n", atomic_is_lock_free(&a) ? "true" : "false"); printf("_Atomic struct B is lock free? %s\n", atomic_is_lock_free(&b) ? "true" : "false"); } The verdict is that atomic.c and atomic.cpp work without -latomic, but atomic.cxx (Sam's program) doesn't. I tried both my Gentoo musl-llvm host: $ clang atomic.c $ ./a.out atomic<int> is lock-free. atomic<long long> is lock-free. atomic<double> is lock-free. $ clang++ atomic.cpp $ ./a.out atomic<int> is lock-free. atomic<long long> is lock-free. atomic<double> is lock-free. $ clang++ atomic.cxx ld.lld: error: undefined symbol: __atomic_is_lock_free >>> referenced by atomic.cxx >>> /tmp/atomic-bc70d0.o:(main) clang++: error: linker command failed with exit code 1 (use -v to see invocation) And I tried also in Ubuntu container, using LLVM 18 from apt.llvm.org, explicitly specifying libc++ and compiler-rt: $ podman run --rm -v .:/src -it ubuntu:22.04 root@1112fc36aae2:/src# apt update [...] root@1112fc36aae2:/src# apt install lsb-release wget software-properties-common gnupg [...] root@1112fc36aae2:/src# bash -c "$(wget -O - https://apt.llvm.org/llvm.sh)" [...] root@1112fc36aae2:/src# apt install libc++-18-dev [...] root@1112fc36aae2:/src# clang -fuse-ld=lld --unwindlib=libunwind --rtlib=compiler-rt atomic.c root@1112fc36aae2:/src# ./a.out atomic<int> is lock-free. atomic<long long> is lock-free. atomic<double> is lock-free. root@1112fc36aae2:/src# clang++ -fuse-ld=lld --stdlib=libc++ --rtlib=compiler-rt atomic.cpp root@1112fc36aae2:/src# ./a.out atomic<int> is lock-free. atomic<long long> is lock-free. atomic<double> is lock-free. root@1112fc36aae2:/src# clang++ -fuse-ld=lld --stdlib=libc++ --rtlib=compiler-rt atomic.cxx ld.lld: error: undefined symbol: __atomic_is_lock_free >>> referenced by atomic.cxx >>> /tmp/atomic-5ee783.o:(main) clang++: error: linker command failed with exit code 1 (use -v to see invocation) In case of atomic.cpp on Ubuntu, it's important to use both libc++ and compiler-rt. Combination of stdlibc++ and compiler-rt doesn't work: root@1112fc36aae2:/src# clang++ -fuse-ld=lld --rtlib=compiler-rt atomic.cpp ld.lld: error: undefined symbol: __atomic_is_lock_free >>> referenced by atomic.cpp >>> /tmp/atomic-0ca37e.o:(std::__atomic_base<int>::is_lock_free() const) >>> referenced by atomic.cpp >>> /tmp/atomic-0ca37e.o:(std::__atomic_base<long long>::is_lock_free() const) >>> referenced by atomic.cpp >>> /tmp/atomic-0ca37e.o:(std::atomic<double>::is_lock_free() const) clang++: error: linker command failed with exit code 1 (use -v to see invocation) OK, so the conclusion is that with libc++ and compiler-rt: - atomic.c works - atomic.cpp works - atomic.cxx doesn't work Why? I need to dig deeper, but looks like libc++ and compiler-rt support atomics only for certain (primitive enough) types. I also tried rewriting Sam's example in more "pure" C++: root@1112fc36aae2:/src# cat atomic2.cxx #include <iostream> #include <atomic> struct A { int a[100]; }; struct B { int x, y; }; // Define atomic types for struct A and struct B std::atomic<A> atomicA; std::atomic<B> atomicB; int main() { std::cout << "std::atomic<A> is lock free? " << (std::atomic_is_lock_free(&atomicA) ? "true" : "false") << "\n"; std::cout << "std::atomic<B> is lock free? " << (std::atomic_is_lock_free(&atomicB) ? "true" : "false") << "\n"; return 0; } But it also doesn't work, neither on Gentoo: $ clang++ atomic2.cxx ld.lld: error: undefined symbol: __atomic_is_lock_free >>> referenced by atomic2.cxx >>> /tmp/atomic2-11a183.o:(std::__1::__atomic_base<A, false>::is_lock_free[abi:se180100]() const volatile) clang++: error: linker command failed with exit code 1 (use -v to see invocation) nor on Ubuntu: root@1112fc36aae2:/src# clang++ -fuse-ld=lld --stdlib=libc++ --rtlib=compiler-rt atomic2.cxx ld.lld: error: undefined symbol: __atomic_is_lock_free >>> referenced by atomic2.cxx >>> /tmp/atomic2-047ef4.o:(std::__1::__atomic_base<A, false>::is_lock_free[abi:ne180100]() const volatile) clang++: error: linker command failed with exit code 1 (use -v to see invocation) What about classes? They don't work either. $ cat atomic3.cxx #include <iostream> #include <atomic> class A { public: int a[100]; }; class B { public: int x, y; }; int main() { A a_instance; B b_instance; std::atomic<A> atomicA_ptr{a_instance}; std::atomic<B> atomicB_ptr{b_instance}; std::cout << "std::atomic<A*> is lock free? " << (std::atomic_is_lock_free(&atomicA_ptr) ? "true" : "false") << "\n"; std::cout << "std::atomic<B*> is lock free? " << (std::atomic_is_lock_free(&atomicB_ptr) ? "true" : "false") << "\n"; return 0; } $ clang++ atomic3.cxx ld.lld: error: undefined symbol: __atomic_is_lock_free >>> referenced by atomic3.cxx >>> /tmp/atomic3-10c5ea.o:(std::__1::__atomic_base<A, false>::is_lock_free[abi:se180100]() const volatile) clang++: error: linker command failed with exit code 1 (use -v to see invocation) What works however are atomics on class pointers: $ cat atomic4.cxx #include <iostream> #include <atomic> class A { public: int a[100]; }; class B { public: int x, y; }; int main() { A a_instance; B b_instance; std::atomic<A*> atomicA_ptr{&a_instance}; std::atomic<B*> atomicB_ptr{&b_instance}; std::cout << "std::atomic<A*> is lock free? " << (std::atomic_is_lock_free(&atomicA_ptr) ? "true" : "false") << "\n"; std::cout << "std::atomic<B*> is lock free? " << (std::atomic_is_lock_free(&atomicB_ptr) ? "true" : "false") << "\n"; return 0; } $ clang++ atomic4.cxx $ ./a.out std::atomic<A*> is lock free? true std::atomic<B*> is lock free? true
How does that fit into nodejs context? I grepped through all their `atomic<T>` declarations. They're done only on primitives and pointers. That's why I'm still inclined to conclude that building nodejs with clang + libc++ + compiler-rt without GCC works. However, the fact that simply using some atomic<T> on a more complex type can just trigger the linker error is worrisome for me. I'm honestly not sure what to do about this. The only sane options for me are: - Introducing the GCC-free build option in gyp, when libc++ and compiler-rt are enabled, but also add a CI job testing that case, so we make sure that upstream can't introduce any regressions. - We move on and live with the fact that we need GCC. We keep the patch in third-party overlays for people who feel confident. Either way, I think it would be nice to create some issue in LLVM for covering more types / achieving compatibility with latomic.
On mobile atm but just want to say: thank you so much for doing the kind of investigation I was looking for!
I found that llvm has a libatomic equivalent but default disabled. Just build llvm with -DCOMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF. With this option, the /lib/clang/18/lib/linux/libclang_rt.builtins-x86_64.a should contain atomic.c.o, which will allow the atomic.cxx to compile and run. https://github.com/llvm/llvm-project/blob/303c8d20601d810c177f6646f771c1eb3f29ab8c/compiler-rt/lib/builtins/CMakeLists.txt#L249
I would like someone else to verify this. Run `echo 'MYCMAKEARGS="-DCOMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF"' > /etc/portage/env/sys-libs/compiler-rt` and rebuild compiler-rt.
I have a vague recollection of LLVM upstream considering it a hack which they want to get rid of but I might be misremembering. I don't have a source to hand.
Hmm,I could not find such a source. Do you have any idea as to where you saw it? Hopefully this is not an obstacle to resolving the issue.
(In reply to mojyack from comment #11) > Hmm,I could not find such a source. > Do you have any idea as to where you saw it? > Hopefully this is not an obstacle to resolving the issue. I was saying it as a note for myself or in case someone else could find it, not a blocker of course if I can't figure out where I saw it. I'm also not sure that COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN solves the issues from https://bugs.gentoo.org/911340#c5, even if we should probably do it anyway.
(In reply to Sam James from comment #12) > (In reply to mojyack from comment #11) > > Hmm,I could not find such a source. > > Do you have any idea as to where you saw it? > > Hopefully this is not an obstacle to resolving the issue. > > I was saying it as a note for myself or in case someone else could find it, > not a blocker of course if I can't figure out where I saw it. yeah I see > > I'm also not sure that COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN solves the issues > from https://bugs.gentoo.org/911340#c5, even if we should probably do it > anyway. At least I was able to solve the #c5 problem with COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN.
As mentioned in #c6, current nodejs only uses atomic<T>, which works without libatomic. I therefore support the first suggestion in #c6. I also think it would be good to add a USE flag to compiler-rt which enables the libatomic builtin and rely on it to prevent future breakages.
Just FYI - I'm extremely busy until Friday, but I will spend some time this weekend on confirming whether COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF also solves #c5 problems for me. And on anything else which can move this effort forward. What would be nice to confirm is whether there is any deterministic way of checking whether the given LLVM toolchain was built with COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF, apart from compiling little programs and facing the error. If there is some way, we could propose adding it nodejs' gyp build system along with a check for libc++ and compiler-rt. If there is no way, then I would be inclined to go with the configure flag idea that targos suggested.
(In reply to mojyack from comment #14) > As mentioned in #c6, current nodejs only uses atomic<T>, which works without > libatomic. > I therefore support the first suggestion in #c6. After giving it some thought, I'm still unsure if I would like to pursue with the autodetection in gyp if there is no deterministic way of figuring out if atomic on non-primitives are supported (COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF). Sure, as of today, nodejs uses atomics only on pointers, but I wouldn't like to limit upstream developers in way they use atomics, especially if they are just going to see the weird linker error as a result. > I also think it would be good to add a USE flag to compiler-rt which enables > the libatomic builtin and rely on it to prevent future breakages. 100%. And I think we should enable that new USE flag globally in llvm profiles.
Sorry for the late follow up here. @mojyack: COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF indeed fixes the issue and makes all my little example programs to build successfully. Do you want to make a PR with a change in compiler-rt ebuilds to add the USE flag for it? It's your finding, so I don't want to steal your potential contribution. About determining whether compiler-rt has atomic builtins, it's quite easy and can be done with: ``` nm $(clang -print-libgcc-file-name) | grep _atomic ``` Where `clang -print-libgcc-file-name` returns a path to the runtime library: ``` $ clang -print-libgcc-file-name /usr/lib/llvm/18/bin/../../../../lib/clang/18/lib/linux/libclang_rt.builtins-x86_64.a ``` It's also able to adapt to the `-rtlib` flag. On my musl-llvm system where I have llvm-libgcc installed, this works: ``` $ clang -print-libgcc-file-name -rtlib=libgcc /lib/libgcc.a ``` GCC also supports the `-print-libgcc-file-name`: ``` $ gcc -print-libgcc-file-name /usr/lib/gcc/x86_64-pc-linux-gnu/13/libgcc.a ``` For compiler-rt with excluded atomic builtins, it's not showing anything. After applying COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF, it shows: ``` $ nm $(clang -print-libgcc-file-name) | grep atomic /lib64/clang/18/lib/linux/libclang_rt.builtins-x86_64.a:apple_versioning.c.o: no symbols /lib64/clang/18/lib/linux/libclang_rt.builtins-x86_64.a:os_version_check.c.o: no symbols /lib64/clang/18/lib/linux/libclang_rt.builtins-x86_64.a:trampoline_setup.c.o: no symbols atomic.c.o: 0000000000000290 T __atomic_compare_exchange 00000000000008e0 T __atomic_compare_exchange_1 00000000000009e0 T __atomic_compare_exchange_16 0000000000000920 T __atomic_compare_exchange_2 0000000000000960 T __atomic_compare_exchange_4 00000000000009a0 T __atomic_compare_exchange_8 00000000000004f0 T __atomic_exchange 00000000000007e0 T __atomic_exchange_1 0000000000000860 T __atomic_exchange_16 0000000000000800 T __atomic_exchange_2 0000000000000820 T __atomic_exchange_4 0000000000000840 T __atomic_exchange_8 0000000000000a30 T __atomic_fetch_add_1 0000000000000ab0 T __atomic_fetch_add_16 0000000000000a50 T __atomic_fetch_add_2 0000000000000a70 T __atomic_fetch_add_4 0000000000000a90 T __atomic_fetch_add_8 0000000000000cf0 T __atomic_fetch_and_1 /lib64/clang/18/lib/linux/libclang_rt.builtins-x86_64.a0000000000000ef0 T __atomic_fetch_and_16 :0000000000000d60 T __atomic_fetch_and_2 extendhftf2.c.o0000000000000dd0 T __atomic_fetch_and_4 : 0000000000000e40 T __atomic_fetch_and_8 no symbols 0000000000001560 T __atomic_fetch_nand_1 00000000000017f0 T __atomic_fetch_nand_16 0000000000001610 T __atomic_fetch_nand_2 00000000000016c0 T __atomic_fetch_nand_4 0000000000001730 T __atomic_fetch_nand_8 0000000000000fc0 T __atomic_fetch_or_1 00000000000011c0 T __atomic_fetch_or_16 0000000000001030 T __atomic_fetch_or_2 00000000000010a0 T __atomic_fetch_or_4 0000000000001110 T __atomic_fetch_or_8 0000000000000b80 T __atomic_fetch_sub_1 0000000000000c20 T __atomic_fetch_sub_16 0000000000000ba0 T __atomic_fetch_sub_2 0000000000000bd0 T __atomic_fetch_sub_4 0000000000000bf0 T __atomic_fetch_sub_8 0000000000001290 T __atomic_fetch_xor_1 0000000000001490 T __atomic_fetch_xor_16 0000000000001300 T __atomic_fetch_xor_2 0000000000001370 T __atomic_fetch_xor_4 00000000000013e0 T __atomic_fetch_xor_8 0000000000000000 T __atomic_is_lock_free 0000000000000040 T __atomic_load 00000000000006c0 T __atomic_load_1 0000000000000700 T __atomic_load_16 00000000000006d0 T __atomic_load_2 /lib64/clang/18/lib/linux/libclang_rt.builtins-x86_64.a00000000000006e0 T __atomic_load_4 :00000000000006f0 T __atomic_load_8 trunctfhf2.c.o0000000000000130 T __atomic_store : 0000000000000720 T __atomic_store_1 no symbols 0000000000000790 T __atomic_store_16 0000000000000740 T __atomic_store_2 0000000000000760 T __atomic_store_4 0000000000000770 T __atomic_store_8 ``` So to sum it up, I think it would be safe to introduce ``` nm $("$CC" -print-libgcc-file-name) | grep __atomic ``` as a check in gyp to determine whether libatomic should be linked or not. That seems like the most bulletproof and generic way to check if the given runtime library supports atomics. Before pinging NodeJS maintainers and doing any more changes in my pull request, I think we need to: 1. Add COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF to compiler-rt ebuild. 2. Wait until docker.io/gentoo/stage3:musl-llvm catches that change. Then I could add a CI test, using that container image. And in general, I need to wrap my head around how I'm going to present all that info to NodeJS maintainers in a convincing way.
On a side note, I think we could think of using the `nm $("$CC" -print-libgcc-file-name) | grep __atomic` trick also in the `append-atomic-flags()` function in flag-o-matic.eclass. I guess that would potentially unblock some other ebuilds which have hard dependency on GCC.
> Do you want to make a PR with a change in compiler-rt ebuilds to add the USE flag for it? It's your finding, so I don't want to steal your potential contribution. Thank you for your consideration. Ok, I'll make PR in a few days.
See Also: https://github.com/gentoo/gentoo/pull/39168
(In reply to mojyack from comment #20) > See Also: https://github.com/gentoo/gentoo/pull/39168 The bot will tag it if you do the reassign thing it mentions.
(In reply to Sam James from comment #21) > (In reply to mojyack from comment #20) > > See Also: https://github.com/gentoo/gentoo/pull/39168 > > The bot will tag it if you do the reassign thing it mentions. Wow I didn't know how to do that. Thanks.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=63b4ae7aaa6e520706e1237b649d8fe29f5aba83 commit 63b4ae7aaa6e520706e1237b649d8fe29f5aba83 Author: mojyack <mojyack@gmail.com> AuthorDate: 2024-11-09 04:27:01 +0000 Commit: Michał Górny <mgorny@gentoo.org> CommitDate: 2024-11-09 10:14:13 +0000 sys-libs/compiler-rt: enable atomic builtin Several packages depend on GCC's libatomic to perform atomic operations. For example, this compiles without -latomic: ```c _Atomic struct { int v[1]; } a; atomic_store(&a, a); ``` But this fails with error "undefined reference to __atomic_store_16" without -latomic: ```c _Atomic struct { int v[4]; } b; atomic_store(&b, b); ``` LLVM does not have a libatomic.so, but an atomic builtin to support such operations. However, it is disabled by default to allow use of system libatomic while using LLVM as a compiler. Pure LLVM environments without GCC installed require this builtin. This commit adds `-DCOMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF` cmake option to enable the builtin. Bug: https://bugs.gentoo.org/911340 Signed-off-by: mojyack <mojyack@gmail.com> Closes: https://github.com/gentoo/gentoo/pull/39168 Signed-off-by: Michał Górny <mgorny@gentoo.org> sys-libs/compiler-rt/compiler-rt-15.0.7-r2.ebuild | 150 +++++++++++++++++ sys-libs/compiler-rt/compiler-rt-16.0.6-r6.ebuild | 178 +++++++++++++++++++++ sys-libs/compiler-rt/compiler-rt-17.0.6-r2.ebuild | 178 +++++++++++++++++++++ sys-libs/compiler-rt/compiler-rt-18.1.8-r2.ebuild | 173 ++++++++++++++++++++ sys-libs/compiler-rt/compiler-rt-19.1.3-r1.ebuild | 175 ++++++++++++++++++++ .../compiler-rt/compiler-rt-20.0.0.9999.ebuild | 1 + 6 files changed, 855 insertions(+)