Skip to content

Conversation

@fanquake
Copy link
Member

I have been down a 🐇 hole. Closes #19359.

When Clang is compiled, a check is run to define HOST_LINK_VERSION as the output of $CMAKE_LINKER -v. Note the this is the version of the linker being used to compile Clang itself.. and this check is only run when compiling Clang for macOS.

In the Clang driver, if HOST_LINK_VERSION has been defined, there is some additional runtime functionality. An -mlinker-version argument, with the value of HOST_LINK_VERSION will be added to the linker arguments, if -mlinker-version has not been passed in by the user.

This is a bit weird, as by default, you are setting -mlinker-version to the version of the linker that was used to build the Clang binary, not the linker which will be used when compiling. The commit which introduced the functionality, llvm/llvm-project@628fcf4, described it as a "hack", that should be replaced. However, that was 10 years ago, and the behaviour is still here.

In the Darwin driver, a check is done for the -mlinker-version argument. If there is no argument, the version will default to 0. Given the above, this should never happen when using Clang for macOS. A series of comparisons are then performed, to check whether the linker version is modern enough to enable certain features, like -demangle.

What this means

macOS

A Clang compiled for macOS, i.e clang+llvm-8.0.0-x86_64-apple-darwin, will have HOST_LINKER_VERSION set to the version of the linker used to compile Clang itself.

At runtime, -mlinker-version=HOST_LINKER_VERSION will be added to the linker args, if -mlinker-version wasn't passed in. In the Darwin driver, additional arguments, like -demangle, will be added to the linker arguments, because HOST_LINKER_VERSION was likely some very modern version of lld or ld64.

Linux (cross compilation in depends)

A Clang compiled for Linux, i.e clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04, which we now use for macOS builds in depends, will behave differently. As it's built for Linux, HOST_LINKER_VERSION was not defined at compile time, and there will be no default behaviour of appending -mlinker-version=HOST_LINKER_VERSION to the linker args. Thus, unless you pass in -mlinker-version yourself, when the version checks are done in the Darwin driver, no modern linker features will be enabled, as the version will have defaulted to 0.

Therefore, it's important that we continue to pass -mlinker-version="our LD64 version" as part of our compilation flags, if we want to have "modern" linker features enabled for our macOS builds.

Summary

Clang 8. Building a macOS binary. Link line with path arguments trimmed.

default behaviour -mlinker-version=100 (-demangle threshold) -mlinker-version=530
macOS Clang -demangle -lto_library ../libLTO.dylib -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 10.15.0 -o a.out ../test-b8b9b3.o -lc++ -lSystem ../libclang_rt.osx.a -demangle -dynamic -arch x86_64 -macosx_version_min 10.15.0 -o a.out ../test-a66966.o -lc++ -lSystem ../libclang_rt.osx.a same as default
Linux Clang -dynamic -arch x86_64 -macosx_version_min 10.12.0 -o a.out ../test-bfce57.o -lc++ -lSystem -demangle -dynamic -arch x86_64 -macosx_version_min 10.12.0 -o a.out ../test-a846a3.o -lc++ -lSystem -demangle -lto_library ../libLTO.dylib -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 10.12.0 -o a.out ../test-de0280.o -lc++ -lSystem

Note: Most links here are pointing to the 8.x branch of LLVM/Clang, as we are using that version in depends.

Note: To add a little more confusion, you wont see -mlinker-version X in your compile flags, you'll see -target-linker-version X.

@laanwj
Copy link
Member

laanwj commented Jun 29, 2020

ACK a8d39b8

I guess this option isn't mentioned in the MacOS build doc as that doesn't deal with cross-compilation?
But, it seems this information would be useful in some documentation. I expect no one to look in that ,mk file unless they've already descended into a another warren of rabbit holes.

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake
Copy link
Member Author

I guess this option isn't mentioned in the MacOS build doc as that doesn't deal with cross-compilation?

Correct. When you're building on macOS, for macOS, Apples Clang etc takes care of wiring things up correctly. As I mentioned in my comment here, I think I'll try and combine a more concise version of these notes into some depends documentation.

@fanquake fanquake merged commit f61019f into bitcoin:master Jul 3, 2020
@fanquake fanquake deleted the explain_mlinker_version_required branch July 3, 2020 09:39
zkbot added a commit to zcash/zcash that referenced this pull request Jul 30, 2020
Modernise macOS cross-compilation toolchain

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#13617
  - Excluding the QT GUI changes.
- bitcoin/bitcoin#17550
- bitcoin/bitcoin#16392
  - Excluding the QT GUI changes.
- bitcoin/bitcoin#18589
- bitcoin/bitcoin#19240
- bitcoin/bitcoin#19407
- bitcoin/bitcoin#17919
  - Only the ancillary changes, not the `FORCE_USE_SYSTEM_CLANG` change.
- bitcoin/bitcoin#19530

After these changes, macOS versions earlier than 10.12 are no longer supported.

To cross-compile for macOS:
- Follow the instructions in `contrib/macdeploy/README.md` to generate
  `Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers.tar.gz`
  (requires an Apple Developer Account)
- Extract it into `depends/SDKs` (creating that folder first if it does not exist)
- `HOST=x86_64-apple-darwin16 ./zcutil/build.sh`
zkbot added a commit to zcash/zcash that referenced this pull request Aug 7, 2020
Modernise macOS cross-compilation toolchain

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#13617
  - Excluding the QT GUI changes.
- bitcoin/bitcoin#17550
- bitcoin/bitcoin#16392
  - Excluding the QT GUI changes.
- bitcoin/bitcoin#18589
- bitcoin/bitcoin#19240
- bitcoin/bitcoin#19407
- bitcoin/bitcoin#17919
  - Only the ancillary changes, not the `FORCE_USE_SYSTEM_CLANG` change.
- bitcoin/bitcoin#19530

After these changes, macOS versions earlier than 10.12 are no longer supported.

To cross-compile for macOS:
- Follow the instructions in `contrib/macdeploy/README.md` to generate
  `Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers.tar.gz`
  (requires an Apple Developer Account)
- Extract it into `depends/SDKs` (creating that folder first if it does not exist)
- `HOST=x86_64-apple-darwin16 ./zcutil/build.sh`
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 2, 2020
Summary:
Backport of core [[bitcoin/bitcoin#19407 | PR19407]].

Depends on D7737.

Test Plan: Read the comment.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D7740
deadalnix pushed a commit to Bitcoin-ABC/secp256k1 that referenced this pull request Oct 3, 2020
Summary:
Backport of core [[bitcoin/bitcoin#19407 | PR19407]].

Depends on D7737.

Test Plan: Read the comment.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D7740
furszy added a commit to PIVX-Project/PIVX that referenced this pull request May 25, 2021
e1b89ac Fix QPainter non-determinism on macOS (Andrew Chow)
831c317 macOS deploy: use the new plistlib API (Jonas Schnelli)
5857aaf doc: Document ALLOW_HOST_PACKAGES dependency option (skmcontrib)
2329e08 build: Fix behavior when ALLOW_HOST_PACKAGES unset (Hennadii Stepanov)
1768870 depends: native_ds_store 1.3.0 (fanquake)
3f9f3e5 depends: pull upstream libdmg-hfsplus changes (fanquake)
f7606dc depends: latest config.guess & config.sub (fanquake)
cc3ae74 depends: bump native_cctools for fixed lto with external clang (Cory Fields)
b26c648 depends: enable lto support for Apple's ld64 (Cory Fields)
50933d7 depends: Add documentation for FORCE_USE_SYSTEM_CLANG make flag (Carl Dong)
ba3ddf2 depends: Reformat make options as definition list (Carl Dong)
3b855a7 depends: Add justifications for macOS clang flags (Carl Dong)
4104de0 depends: specify libc++ header location for darwin (Cory Fields)
cd4335f depends: force a new host id string if FORCE_USE_SYSTEM_CLANG is in use (Cory Fields)
d30e1af depends: Allow building with system clang (Carl Dong)
234828b depends: Decouple toolchain + binutils (Carl Dong)
1dd3a5a doc: explain why passing -mlinker-version is required (fanquake)
5cc0d0f darwin: pass mlinker-version so that clang enables new features (Cory Fields)
813a552 macos: Bump to xcode 11.3.1 and 10.15 SDK (Cory Fields)
ee7085f depends: bump MacOS toolchain (Cory Fields)
e5b092b contrib: macdeploy: Remove historical extraction notes (Carl Dong)
5893caf contrib: macdeploy: Use apple-sdk-tools instead of xar+pbzx (Carl Dong)
9f2d4ba native_cctools: Don't use libc++ from pinned clang (Carl Dong)
0c8d217 Adapt rest of tooling to new SDK naming scheme (Carl Dong)
bdacfa8 contrib: macdeploy: Correctly generate macOS SDK (Carl Dong)
f7eee2c Fix naming of macOS SDK and clarify version (Andrew Chow)
62f9e23 build: use macOS 10.14 SDK (fanquake)
bc2e1af depends: native_cctools 921, ld64 409.12, libtapi 1000.10.8 (fanquake)
a296d87 depends: clang 6.0.1 (fanquake)
8f6c475 build: Set minimum supported macOS to 10.12 (Fuzzbawls)

Pull request description:

  This backports the following upstream PRs to update the macOS cross-compiling tools:

  bitcoin#17550
  bitcoin#16392
  bitcoin#18589
  bitcoin#19240
  bitcoin#19407
  bitcoin#17919
  bitcoin#19530
  bitcoin#17057
  bitcoin#20333
  bitcoin#18051
  bitcoin#19124
  bitcoin#20298
  bitcoin#20447

  The tools being updated are

  ### Clang
  Upgraded from `3.7.1` to `8.0.0`

  ### cctools

  * cctools `877.8` -> `949.0.1`
  * LD64 `253.9` -> `530`
  * TAPI `1000.10.8`

  ### DSStore
  Upgraded from `1.1.2` to `1.3.0` (this removes the biplist dependency)

  This also effectively bumps our minimum supported macOS version to 10.12 (Sierra).

ACKs for top commit:
  furszy:
    tested ACK e1b89ac
  random-zebra:
    utACK e1b89ac

Tree-SHA512: f5cec8db57e07d8855070646b9e1400d48aac1d01e3c2c3b3e134665c6372d6535f3328888bb9a75087f7b3d5231ecb4b509723bfa51bd40770ffe2810c67f65
kwvg added a commit to kwvg/dash that referenced this pull request Aug 26, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 3, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Sep 3, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build: follow up with ld64 versioning for macOS toolchain

3 participants