build: Allow to use atomic builtins from compiler-rt instead of libatomic

[vadorovsky]

compiler-rt, when it's built with the COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN option disabled, provides all atomic builtins and serves as a full replacement for libatomic.

By default, that option is enabled[0], meaning that default builds of compiler-rt, followed by the most of Linux distributions (Fedora, openSUSE, Ubuntu), do not provide all necessary atomics.

However, FreeBSD[1] and Gentoo (with LLVM-based profiles)[2] disable it, therefore providing compiler-rt with atomic builtins.

That difference can be detected by checking the symbol table of compiler-rt:

nm $(clang -print-libgcc-file-name) | grep __atomic

No matching symbols indicate lack of atomic builtins and necessity of linking libatomic. Matching symbols indicate a possibility to not link libatomic.

Given that difference, provide the --use-compiler-rt-atomics option in configure.py. When enabled, the configure script checks whether compiler-rt provides atomics and, if yes, does not link libatomic. By default, without that option enabled, a build with clang links libatomic.

For more context, see the discussion on Gentoo bugzilla[3].

[0] https://github.com/llvm/llvm-project/blob/llvmorg-19.1.6/compiler-rt/lib/builtins/CMakeLists.txt#L227-L229
[1] https://cgit.freebsd.org/src/commit/?id=7b67d47c70cca47f65fbbc9d8607b7516c2a82ee
[2] gentoo/gentoo@63b4ae7
[3] https://bugs.gentoo.org/911340

[nodejs-github-bot]

Review requested:

• @nodejs/gyp
• @nodejs/v8-update

[richardlau]

This has broken the GitHub actions Linux workflows, which use clang.

Error is 2024-08-11T07:08:51.3573153Z node.cc:(.text+0x2bed): undefined reference to __atomic_is_lock_free'`

[vadorovsky]

I see. I can't reproduce this error locally, perhaps it's either:

• too old clang on Github runners
• compiler-rt not being installed
• compiler-rt not being picked up for some reason

I will get back to you once I figure out.

[mojyack]

Any progress on this PR?
I have been using this patch for months with my llvm+musl gentoo system on both x86_64 and aarch64 and have had no problems.
Hope this issue will be resolved soon.
Let me know if there is anything I can do to help.

[targos]

It seems unlikely. We found recently that newer V8 versions require it for __atomic_compare_exchange

[thesamesam]

I do keep trying to explain this to people on our end. You can't be sure that the compiler won't emit a libcall that compiler-rt doesn't supply (it's not clear to me why Clang does this, but whatever) -- or that, with GCC, needs libatomic, hence what nodejs is doing right now is fine (or fine enough). Working on one person's machine doesn't mean it should be removed.

We removed it a few times and had reports for some, but not all, users of the Clang case, and the GCC case is obvious. So yes, the status quo is correct (or more correct than dropping it).

[targos]

What could be accepted more easily is a new configure flag to disable it.

[mojyack]

microsoft/mimalloc#898
I worked on a similar problem with mimalloc a while ago.
mimalloc was also unconditionally adding -latomic, but by using add_link_library, it changed to only use it in environments where libatomic is available.
I think a similar solution could be applied to nodejs, what do you think?

[mojyack]

It seems unlikely. We found recently that newer V8 versions require it for __atomic_compare_exchange

I could build latest nodejs(98788da) with llvm-18.1.8 successfully.
Did you mean the upstream V8, not the one included in nodejs?

[mojyack]

Anyway, LLVM has implemented __atomic_compare_exchange years ago.
I haven't been able to verify that the latest V8 can be built with LLVM (building V8 in a non-glibc environment is a pain!). But as long as the latest nodejs can be built without problems, I think it's good to support natively such an environment.

[mojyack]

Does gyp have ability to find optional dependencies like CMake's check_linker_flag?
If so, it can be used to eliminate ugly flag addition based on the architecture in tools/v8_gypfiles/v8.gyp
If not, at least the behaviour should be configurable as @targos says.

[vadorovsky]

First of all, I'm sorry for the late follow-up.

@targos

It seems unlikely. We found recently that newer V8 versions require it for __atomic_compare_exchange

Just for clarity __atomic_compare_exchange is provided by compiler-rt here https://github.com/llvm/llvm-project/blob/llvmorg-18.1.8/compiler-rt/lib/builtins/atomic.c#L41

The reason why the CI is red and what I overlooked when submitting the PR is the fact that clang on the most of Linux distros is going to pick up libgcc_s as a runtime library, unless you suppress that with --rtlib=compiler-rt ldflag.

What would have a chance of working would be not requiring -latomic after checking that compiler-rt is used as rtlib, that would be probably way too messy to do in gyp and there is a chance I'm still overlooking something for certain configurations, so...

What could be accepted more easily is a new configure flag to disable it.

...I'm happy to hear that and I totally agree that would be the best approach! I will do it in this PR.

Would you be also open to having a CI job checking the clang+compiler-rt configuration (which would disable latomic), just to make sure things don't regress? I could try to use the docker.io/gentoo/stage3:musl-llvm image to add a job similar to this one:

node/.github/workflows/test-linux.yml

Lines 35 to 56 in accb239

	jobs:
	test-linux:
	if: github.event.pull_request.draft == false
	runs-on: ubuntu-24.04
	steps:
	- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
	with:
	persist-credentials: false
	- name: Set up Python ${{ env.PYTHON_VERSION }}
	uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
	with:
	python-version: ${{ env.PYTHON_VERSION }}
	- name: Set up sccache
	uses: mozilla-actions/sccache-action@9e326ebed976843c9932b3aa0e021c6f50310eb4 # v0.0.6
	with:
	version: v0.8.1
	- name: Environment Information
	run: npx envinfo
	- name: Build
	run: make build-ci -j4 V=1 CONFIG_FLAGS="--error-on-warn"
	- name: Test
	run: make run-ci -j4 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9"

@thesamesam

We removed it a few times and had reports for some, but not all, users of the Clang case, and the GCC case is obvious. So yes, the status quo is correct (or more correct than dropping it).

Are you referring to https://bugs.gentoo.org/870919?

[thesamesam]

There's https://bugs.gentoo.org/869992#c4 and so on too. All of the reports are with Clang versions later than the one which added support. I think someone who cares about such setups needs to investigate it, but the explanation being offered isn't sufficient at least.

[thesamesam]

OK, coming back to this, I think it'd work to have such a check if it's possible in gyp.

[mojyack]

I assume the problem reported on Gentoo is caused by the user's environment setup, not nodejs.
The idea of adding the configure option seems fine. Is there any obstacle?

[thesamesam]

I don't think that concurs with @vadorovsky's analysis at https://bugs.gentoo.org/911340#c5.

[vadorovsky]

@targos @thesamesam First of all, sorry for very late follow up!

I went ahead with adding a configure option, which also performs a check whether compiler-rt really supports atomics. I also added more context and linked to the Gentoo bug, where we were figuring out how and under what circumstances compiler-rt doesn't need libatomic and came up with the solution proposed her.

I'm still thinking how to test that in CI. I was considering using the docker.io/gentoo/stage3:musl-llvm container image. If you agree with that solution, I can do that.