Skip to content

Wrong condition for deciding when to add -latomic #30093

Closed
Closed
Wrong condition for deciding when to add -latomic#30093

Description

@ryandesign
ryandesign
opened on Oct 23, 2019
  • 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.

Activity

  1. devsnek commented on Oct 23, 2019

    @devsnek
    devsnek
    on Oct 23, 2019
    Member

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

  2. ryandesign commented on Oct 23, 2019

    @ryandesign
    ryandesign
    on Oct 23, 2019
    ContributorAuthor

    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.

  3. devsnek commented on Oct 23, 2019

    @devsnek
    devsnek
    on Oct 23, 2019
    Member

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

  4. sam-github commented on Oct 23, 2019

    @sam-github
    sam-github
    on Oct 23, 2019
    Contributor

    @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"?

  5. ryandesign commented on Oct 23, 2019

    @ryandesign
    ryandesign
    on Oct 23, 2019
    ContributorAuthor

    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.

  6. btsimonh commented on Oct 30, 2019

    @btsimonh
    btsimonh
    on Oct 30, 2019

    affects RPi3 raspbian building too: #30174 ?

  7. CL-Jeremy commented on Nov 22, 2019

    @CL-Jeremy
    CL-Jeremy
    on Nov 22, 2019

    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 llvm@3.9). 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.

  8. BridgeAR
    closed this as completedin 4bd0d8con Jan 2, 2020
  9. BridgeAR
    added a commit that references this issue on Jan 3, 2020

    build: don't use -latomic on macOS

    9b7cf09
  10. targos
    added a commit that references this issue on Jan 14, 2020

    build: don't use -latomic on macOS

    8ab748e
  11. BethGriggs
    added a commit that references this issue on Feb 6, 2020

    build: don't use -latomic on macOS

    cb89fbc

  12. justinkb commented on May 21, 2021

    @justinkb
    justinkb
    on May 21, 2021

    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)

  13. jhuntwork commented on Jan 5, 2022

    @jhuntwork
    jhuntwork
    on Jan 5, 2022

    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.

  14. barracuda156 commented on Sep 7, 2022

    @barracuda156
    barracuda156
    on Sep 7, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions