Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 911340 - net-libs/nodejs: potential erroneous dependency on GCC
Summary: net-libs/nodejs: potential erroneous dependency on GCC
Status: IN_PROGRESS
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal
Assignee: William Hubbs
URL:
Whiteboard: Nodejs dep needs cleaning up in due c...
Keywords: PullRequest
Depends on: 943129
Blocks:
  Show dependency tree
 
Reported: 2023-07-27 20:31 UTC by Violet Purcell
Modified: 2025-04-13 15:22 UTC (History)
6 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
patch (nodejs-16.10.0-libcxx-dont-link-libatomic.patch,218 bytes, patch)
2023-07-27 20:31 UTC, Violet Purcell
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Violet Purcell 2023-07-27 20:31:44 UTC
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?
Comment 1 William Hubbs gentoo-dev 2023-07-28 18:39:53 UTC
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
Comment 2 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2023-07-28 18:57:44 UTC
Let me take a look first.
Comment 3 Violet Purcell 2023-08-11 21:31:04 UTC
(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.
Comment 4 Violet Purcell 2023-11-03 17:44:40 UTC
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.
Comment 5 vadorovsky 2024-10-09 13:40:34 UTC
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
Comment 6 vadorovsky 2024-10-09 13:48:26 UTC
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.
Comment 7 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-10-09 13:49:24 UTC
On mobile atm but just want to say: thank you so much for doing the kind of investigation I was looking for!
Comment 8 mojyack 2024-10-11 11:37:42 UTC
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
Comment 9 mojyack 2024-10-11 11:47:43 UTC
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.
Comment 10 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-10-12 03:55:26 UTC
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.
Comment 11 mojyack 2024-10-12 08:29:19 UTC
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.
Comment 12 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-10-16 09:22:41 UTC
(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.
Comment 13 mojyack 2024-10-16 09:38:26 UTC
(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.
Comment 14 mojyack 2024-10-16 09:39:09 UTC
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.
Comment 15 vadorovsky 2024-10-16 13:14:03 UTC
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.
Comment 16 vadorovsky 2024-10-16 13:18:56 UTC
(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.
Comment 17 vadorovsky 2024-10-29 17:51:12 UTC
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.
Comment 18 vadorovsky 2024-10-29 17:54:29 UTC
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.
Comment 19 mojyack 2024-10-30 00:32:34 UTC
> 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.
Comment 20 mojyack 2024-11-05 04:48:49 UTC
See Also: https://github.com/gentoo/gentoo/pull/39168
Comment 21 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2024-11-05 04:55:45 UTC
(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.
Comment 22 mojyack 2024-11-05 07:06:23 UTC
(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.
Comment 23 Larry the Git Cow gentoo-dev 2024-11-09 10:14:16 UTC
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(+)
Comment 24 mojyack 2025-03-11 07:34:20 UTC
It has been several months since the atomic-buildins flag was added.
I think it's time to resume this discussion.
What should be done to merge the upstream pull request(https://github.com/nodejs/node/pull/54306)?
Comment 25 vadorovsky 2025-03-11 09:59:02 UTC
The upstream PR works, but isn't getting any attention and honestly, I can't blame the devs - the whole dance with detecting whether compiler-rt has atomics bloats the configure/gyp script. And doing that in every project packaged in Gentoo and using atomics doesn't seem like a good strategy. We would need to do the same thing in qtwebengine and other projects.

Linking libatomic is convenient and works for developers. We should try to not worsen their experience.

So I have an another idea - we could create something similar to llvm-libgcc, but for atomics - a linker version script which takes atomic symbols from compiler-rt and exports them through a shared library libatomic.so, which would serve as a drop-in replacement.

Perhaps it could start as a separate repo (gentoo, proj2, our personal accounts, whatever). But after getting it to work, we could even try to pitch that idea upstream in LLVM and try to create something like llvm-libatomic.

The first step would be making sure that compiler-rt has all atomic present in GCC libatomic - as far as I remember, there is one or two functions missing. This week is quite busy for me, but I could start working on that next week, if nobody beats me up to that.
Comment 26 mojyack 2025-03-12 02:26:37 UTC
Thank you for explaining the current situation. I'm glad you're working on it.

> So I have an another idea - we could create something similar to llvm-libgcc, but for atomics - a linker version script which takes atomic symbols from compiler-rt and exports them through a shared library libatomic.so, which would serve as a drop-in replacement.
That sounds good.
But I doubt that we can (or even we should) provide binary compatibility with libatomic.so, which seems a bit difficult.
libatomic.so (from gentoo amd64 openrc stage3 image) has 91 __atomic_* symbols while compiler-rt has only 55.

How about just aiming for source compatibility for now?
For example, installing empty libatomic.a with `llvm-runtimes/compiler-rt[atomic-builtins]`.
Comment 27 vadorovsky 2025-03-12 08:40:25 UTC
(In reply to mojyack from comment #26)
> Thank you for explaining the current situation. I'm glad you're working on
> it.
> 
> > So I have an another idea - we could create something similar to llvm-libgcc, but for atomics - a linker version script which takes atomic symbols from compiler-rt and exports them through a shared library libatomic.so, which would serve as a drop-in replacement.
> That sounds good.
> But I doubt that we can (or even we should) provide binary compatibility
> with libatomic.so, which seems a bit difficult.
> libatomic.so (from gentoo amd64 openrc stage3 image) has 91 __atomic_*
> symbols while compiler-rt has only 55.

Thanks for checking! Not sure what to others think (Michal Gorny, Sam), but perhaps we could start with these 55 symbols and mention that we don't guarantee the full binary compatibility yet? That way, llvm-libatomic (or whatever name we give to that project) would be a library with atomics, but with differences. Similarily to glibc and musl, which are different libc implementations, but aren't binary compatible.

However, I would still aim for having the full coverage of symbols eventually, but that could be a long term project.

What do you think?

> How about just aiming for source compatibility for now?
> For example, installing empty libatomic.a with
> `llvm-runtimes/compiler-rt[atomic-builtins]`.

That sounds like a hack I would be comfortable with shipping in a private overlay (I will actually do it now in mine, to not have to path nodejs all the time), but not as something I would contribute to the main overlay. Just my opinion.
Comment 28 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2025-03-12 08:42:43 UTC
An empty libatomic may actually be okay. An implementation can't really assume that libatomic, if it exists, will contain anyway. As long as Clang won't emit things that are missing, of course (which is where we started ;)).
Comment 29 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2025-03-12 08:57:29 UTC
(In reply to Sam James from comment #28)
> An empty libatomic may actually be okay. An implementation can't really
> assume that libatomic, if it exists, will contain anyway. As long as Clang
> won't emit things that are missing, of course (which is where we started ;)).

(So, in a word, if symbols may be needed and this would simply placate nodejs: then no.)
Comment 30 mojyack 2025-03-12 09:37:31 UTC
> Thanks for checking! Not sure what to others think (Michal Gorny, Sam), but perhaps we could start with these 55 symbols and mention that we don't guarantee the full binary compatibility yet? That way, llvm-libatomic (or whatever name we give to that project) would be a library with atomics, but with differences. Similarily to glibc and musl, which are different libc implementations, but aren't binary compatible.
> 
> However, I would still aim for having the full coverage of symbols eventually, but that could be a long term project.
> 
> What do you think?

There is no doubt that this is the most correct solution. It would be great if it were achieved.
Leaving that aside, it is questionable how useful it actually is.
In fact, I have never had any trouble with downloaded binaries that depend on libatomic.so.
In most cases, the built-in atomic instructions will suffice and it is a corner case where libatomic is needed I think.

In short, I feel that the amount of effort required to implement llvm-libatomic may not be commensurate with the amount of problems that can be solved.
Comment 31 mojyack 2025-03-12 10:07:12 UTC
(In reply to Sam James from comment #28)
> An empty libatomic may actually be okay. An implementation can't really
> assume that libatomic, if it exists, will contain anyway. As long as Clang
> won't emit things that are missing, of course (which is where we started ;)).

I agree with this.
Except for when libstdc++ is specified, it is inconceivable that clang would use such symbols.
It would be a problem if nodejs used __atomic_* functions directly,
but this is a clearly platform-dependent implementation and they would do not do that.
Comment 32 vadorovsky 2025-03-12 13:11:43 UTC
(In reply to mojyack from comment #31)
> (In reply to Sam James from comment #28)
> > An empty libatomic may actually be okay. An implementation can't really
> > assume that libatomic, if it exists, will contain anyway. As long as Clang
> > won't emit things that are missing, of course (which is where we started ;)).
> 
> I agree with this.
> Except for when libstdc++ is specified, it is inconceivable that clang would
> use such symbols.
> It would be a problem if nodejs used __atomic_* functions directly,
> but this is a clearly platform-dependent implementation and they would do
> not do that.

Alright, let's try the empty libatomic then. I think I'll close my PR in nodejs and wait for your PR, @mojyack. :)
Comment 33 Larry the Git Cow gentoo-dev 2025-04-13 15:22:09 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=846b5d12f21afe7f6f06c9c829a453625ad14142

commit 846b5d12f21afe7f6f06c9c829a453625ad14142
Author:     mojyack <mojyack@gmail.com>
AuthorDate: 2025-03-13 01:53:05 +0000
Commit:     Michał Górny <mgorny@gentoo.org>
CommitDate: 2025-04-13 15:22:00 +0000

    llvm-runtimes/libatomic-stub: new package, add 0
    
    Clang(LLVM) implements atomic functions in compiler-rt, while GCC
    provides a dedicated libatomic for it.
    Some apps such as nodejs erroneously depend on GCC through libatomic.
    LLVM's atomic builtins and libatomic are source-compatible so such
    packages should be also buildable with LLVM.
    It would be hard to fix all those packages, so let's deal with it
    on compiler-rt side by installing a stub libatomic.a.
    
    Bug: https://bugs.gentoo.org/911340
    Signed-off-by: mojyack <mojyack@gmail.com>
    Closes: https://github.com/gentoo/gentoo/pull/41045
    Signed-off-by: Michał Górny <mgorny@gentoo.org>

 .../libatomic-stub/libatomic-stub-0.ebuild         | 25 ++++++++++++++++++++++
 llvm-runtimes/libatomic-stub/metadata.xml          | 15 +++++++++++++
 2 files changed, 40 insertions(+)