Wrong condition for deciding when to add -latomic

[ryandesign]

• Version: 12.12.0, 13.0.1
• Platform: macOS 10.13
• Subsystem:

node.gyp uses this code to decide whether to add the -latomic flag:

      ['OS in ("linux", "mac") and llvm_version != "0.0"', {
         'libraries': ['-latomic'],
       }],

This is exactly wrong. You want to add -latomic when not using llvm/clang. llvm_version is supposed to be 0.0 when not using llvm/clang. Therefore what I think you meant to write was 'OS in ("linux", "mac") and llvm_version == "0.0".

However, in fact, llvm_version ended up being 0.0 even when using llvm/clang on recent macOS versions because you're setting llvm_version wrong, or rather, it's wrong to assume that you can get the llvm version. It's set this way in configure.py:

  o['variables']['llvm_version'] = get_llvm_version(CC) if is_clang else '0.0'

def get_llvm_version(cc):
  return get_version_helper(
    cc, r"(^(?:FreeBSD )?clang version|based on LLVM) ([0-9]+\.[0-9]+)")

This will only work with open-source versions of clang, and versions of Apple's Xcode clang prior to Xcode 7. As of Xcode 7, Apple no longer advertises its compiler as being "based on" a particular open source llvm version; Apple's llvm/clang has diverged too much from open source llvm/clang for any such association to be meaningful.

The consequence of the combination of these two errors is that it correctly omits -latomic with Xcode 7 and later, but incorrectly adds -latomic with any open source clang version and probably also with Xcode 6 and earlier.

You can try to get the clang version using the __clang_major__, __clang_minor__ and __clang_patchlevel__ preprocessor defines, which you do in try_check_compiler in configure.py, and you make decisions based on that number elsewhere, but note that Apple's clang uses a different version numbering scheme than open source clang. If there's a particular clang feature you need that you can't check for using the feature-checking macros, you can check if __apple_build_version__ is defined and if so you can compare that number with a known-good Apple build version; if it's not defined, you can compare __clang_major__.__clang_minor__.__clang_patchlevel__ with a known-good open source clang version.

For this situation, where you merely want to add -latomic when not using clang, it seems like you just need a variable based on the __clang__ preprocessor define that indicates whether you're using clang. It doesn't matter here what the specific llvm version is; it just matters whether or not clang is being used.

[devsnek]

this option was added to fix an issue where clang builds on mac and linux were failing because atomic wasn't being linked.

[ryandesign]

Builds on linux with clang were failing (#28231) but builds on Mac were not, and introducing -latomic on Mac caused build failures because there's no such library (#28232 (comment)). So then I don't know what the correct condition is, but it's not the one currently being used.

[devsnek]

I'm not really sure what to suggest. Node currently builds fine on my mac and linux machines, all using clang.

[sam-github]

@ryandesign your description sounds compelling to a not very informed clang/node-gyp outsider (me), but I'm not sure what to suggest either.

Can you provide info on how to reproduce a specific problem? I.e. "install clang X by doing P on OS Z, configure with args CC on nodejs/node version VV, it will fail to build with PASTE"?

[ryandesign]

Building node 12.12.0 fails on macOS if you use open-source clang instead of Apple clang. Here's a log of that happening on OS X 10.10 using open source clang 9. The error is ld: library not found for -latomic. I also reproduced the issue on macOS 10.13 using open source clang 8.

According to #28532:

libatomic is a gcc library. Clang default is to be built to use it on Linux.

Which I guess means that clang by default does not use libatomic on macOS.

I guess the correct condition to test is just OS == "linux" and llvm_version != "0.0". Here's a log of a successful build on OS X 10.10 using open source clang 9 after making that change.

[btsimonh]

affects RPi3 raspbian building too: #30174 ?

[CL-Jeremy]

I'm almost in the exact same situation as @ryandesign: using Homebrew, brewing node 13.1.0 on 10.10.5 using brewed llvm (using llvm@7, itself brewed with the already removed formula [email protected]). I wonder how -latomic passed the homebrew CI with the formula specifying seemingly no linkage to anything GCC related. Trying to investigate what's different on newer macOS's.

[justinkb]

this condition is still bogus, I'm using clang as the system compiler on linux so this incorrectly gets added, ryan's original analysis that it should check for equality instead of inequality with "0.0" seems right to me

what you guys in discussion above missed is that nearly always gcc libatomic libgcc and libstdc++ will still be installed on linux, even if clang is being compiled with, and clang can still link against it just fine (it'll just not do anything with the linked objects)

[jhuntwork]

this condition is still bogus, I'm using clang as the system compiler on linux so this incorrectly gets added, ryan's original analysis that it should check for equality instead of inequality with "0.0" seems right to me

what you guys in discussion above missed is that nearly always gcc libatomic libgcc and libstdc++ will still be installed on linux, even if clang is being compiled with, and clang can still link against it just fine (it'll just not do anything with the linked objects)

Can confirm. On my linux system where there is no gcc and clang is the default compiler, the build adds in -latomic and fails because that library isn't present. Running sed -i 's/-latomic//' node.gyp is enough to work around it. But I suppose what is really needed here is an actual test for libatomic's presence and need, instead of just making assumptions about the OS.

[barracuda156]

@ryandesign is right, -latomic should be used only with GCC builds.