build: link libatomic on gcc or clang on linux

[marsam]

See: #28231 #28232
libatomic is a gcc library. Clang default is to be built to use it on Linux.
cc: @devsnek @addaleax

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[Trott]

@nodejs/node-gyp

[bnoordhuis]

node-gyp only uses common.gypi so this change should be harmless in that respect.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/24273/

[sam-github]

I'm having trouble understanding what problem this fixes. This is adding -latomic for all platforms when gcc is used, why? It is also removing -latomic on Macs when clang is used, but wasn't one of the bugs in the linked reports explicitly about needing -latomic with clang on Mac?

[devsnek]

technically clang always needs libatomic, but node only uses it on mac and linux for the v8 sigsegv handlers.

I don't believe it's ever needed on GCC, but I could be wrong.

[marsam]

This is adding -latomic for all platforms when gcc is used, why?

I assumed this was implicitly required by the previous state. I've replace it with:

OS=="linux" and llvm_version!=0

to only add it on cllang+linux

but wasn't one of the bugs in the linked reports explicitly about needing -latomic with clang on Mac?

no, the #28231 mention clang+linux, it fails on clang+darwin NixOS/nixpkgs#64253 (comment)

technically clang always needs libatomic

not quite, libatomic is a gcc library. Clang by default is to be built to use it on Linux, and to use compiler-rt on darwin

[devsnek]

this will fail on mac+clang now

[marsam]

@devsnek I've tested on clang+darwin https://hatebin.com/vrlbbcwsia and gcc+linux (both with nix)
can you tell me which versions are you using to build it? and which is the output error?

[devsnek]

@marsam on Apple LLVM version 10.0.0 (clang-1000.10.25.5) (what macos ships with) the build will fail if -latomic is not passed.

example error:

out/Release/obj.target/libnode/src/node.o: In function `node::RegisterSignalHandler(int, void (*)(int, siginfo_t*, void*), bool)':
node.cc:(.text+0x17b4): undefined reference to `__atomic_is_lock_free'

[marsam]

AFAIK __atomic_is_lock_free on darwin should be provided by clang compiler-rt. What occurs me it that you might be using libgcc_s, from clang docs:

Clang does not currently automatically link against libatomic when using libgcc_s. You may need to manually add -latomic to support this configuration when using non-native atomic operations (if you see link errors referring to _atomic* functions).

I think adding -latomic in darwin regarless, is a bad choice, because only solves for that specific case. For instance krita disables libatomic on darwin, so does chapel-lang. I haven't found a project which by default links libatomic on darwin unless it explicitly requires gcc.

[devsnek]

I don't really know what any of the stuff you linked means, but you have an alternative to -latomic that doesn't break macos+clang, please include it in this pr.

[BridgeAR]

Ping @marsam

[marsam]

@BridgeAR sorry for the delay, I'm not sure how to solve @devsnek's issue.
Without this patch, nodejs is basically broken on darwin, because darwin does not have libatomic.

I'm not familiar with gyp, and I could patch it downstream, hence I will not be working on this PR any further. Feel free to close it, thanks.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/24566/

[marsam]

Closed in favor of #30099