Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support LLD linker for Darwin #286

Merged
merged 3 commits into from
Mar 14, 2024

Conversation

siddharthab
Copy link
Contributor

@siddharthab siddharthab commented Mar 12, 2024

Users can use with --linkopt=-fuse-ld=ld64.lld flag.

Eventually, we should make this the default. But only after we hear from some users that it works for their projects. This PR will make it easy for them to test.

@DavidZbarsky-at
Copy link

DavidZbarsky-at commented Mar 13, 2024

I gave this a quick spin, it might need more work for darwin_arm? (With llvm 17.0.6)

ERROR: /Users/david.zbarsky/h/source/hyperbase/server_shared/lightweight_gc_stats/BUILD.bazel:7:16: Linking server_shared/lightweight_gc_stats/libindex.node.so failed: (Exit 1): cc_wrapper.sh failed: error executing CppLink command (from target //server_shared/lightweight_gc_stats:libindex.node.so) external/llvm_toolchain/bin/cc_wrapper.sh @bazel-out/darwin_arm64-fastbuild/bin/server_shared/lightweight_gc_stats/libindex.node.so-2.params

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
ld64.lld: error: must specify -platform_version
ld64.lld: error: missing or unsupported -arch arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)


(env-18.16.0) david.zbarsky@JF6FQ9PXP9 hyperbase % cat bazel-out/darwin_arm64-fastbuild/bin/server_shared/lightweight_gc_stats/libindex.node.so-2.params
-shared
-o
bazel-out/darwin_arm64-fastbuild/bin/server_shared/lightweight_gc_stats/libindex.node.so
bazel-out/darwin_arm64-fastbuild/bin/server_shared/lightweight_gc_stats/_objs/libindex.node.so/lightweight_gc_stats.o
-Wl,-S
-mmacos-version-min=14.2
-no-canonical-prefixes
-fobjc-link-runtime
--target=aarch64-apple-macosx
-lm
-no-canonical-prefixes
-fuse-ld=ld64.lld
-headerpad_max_install_names
-fobjc-link-runtime
-Lexternal/sysroot_darwin_universal//usr/lib
-lc++
-lc++abi
-undefined
dynamic_lookup
--sysroot=external/sysroot_darwin_universal/

@siddharthab
Copy link
Contributor Author

It might.

I did develop this on an M2 Mini, but I tested it only on the simple tests in this repo, without a custom sysroot. The params look like this for me:

% cat bazel-out/darwin_arm64-fastbuild/bin/stdlib_test-2.params
-o
bazel-out/darwin_arm64-fastbuild/bin/stdlib_test
bazel-out/darwin_arm64-fastbuild/bin/_objs/stdlib_test/stdlib_test.o
bazel-out/darwin_arm64-fastbuild/bin/libstdlib.a
-Wl,-S
-mmacosx-version-min=10.11
-no-canonical-prefixes
-fobjc-link-runtime
--target=aarch64-apple-macosx
-lm
-no-canonical-prefixes
-fuse-ld=ld64.lld
-headerpad_max_install_names
-fobjc-link-runtime
-L/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib
-lc++
-lc++abi
-Lexternal/toolchains_llvm~~llvm~llvm_toolchain_llvm/lib
-Wl,-t
--sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk

@siddharthab
Copy link
Contributor Author

I found my mistake. At least the repo tests should be passing now.

@siddharthab siddharthab marked this pull request as ready for review March 13, 2024 08:34
@siddharthab
Copy link
Contributor Author

I will wait for at least some people to say this change is working for them. This may not make it into the next release.

@siddharthab siddharthab added the help wanted Extra attention is needed label Mar 13, 2024
@DavidZbarsky-at
Copy link

@siddharthab I suspect this might be a llvm/lld version difference. Looks like the tests run on 16.x

@siddharthab
Copy link
Contributor Author

On my local machine, I checked with LLVM 17 as well.

@aran
Copy link

aran commented Mar 14, 2024

I will wait for at least some people to say this change is working for them. This may not make it into the next release.

Anecdata, working fine on Bazel 7.1 on an M2 mac with up-to-date rules_rust, rules_go, llvm.

@siddharthab
Copy link
Contributor Author

I think that for @DavidZbarsky-at , it is either the -shared or the universal sysroot that is causing issues. Based on the error message, it is likely to be the sysroot.

I am thinking of landing this behind a feature flag so people can more easily try it within their projects. Most likely, it might just mean adding a few more flags to the invocation which can be iterated on as more people try.

@siddharthab siddharthab changed the title Use LLD linker for Darwin Support LLD linker for Darwin Mar 14, 2024
@siddharthab
Copy link
Contributor Author

Made it optional, and added to the PR description instructions on how to use for a project.

@siddharthab siddharthab merged commit 6bca3e2 into bazel-contrib:master Mar 14, 2024
35 checks passed
helly25 added a commit to helly25/mbo that referenced this pull request Mar 26, 2024
* 2024.03.12: bazel-contrib/toolchains_llvm#286: Support LLD linker for Darwin
helly25 added a commit to helly25/mbo that referenced this pull request Mar 26, 2024
* 2024.03.12: bazel-contrib/toolchains_llvm#286: Support LLD linker for Darwin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants