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.

Sign up for GitHub

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

Always use LLD on darwin #380

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Always use LLD on darwin #380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 28 additions & 35 deletions toolchain/cc_toolchain_config.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,22 +136,23 @@ def cc_toolchain_config(
"--target=" + target_system_name,
"-lm",
"-no-canonical-prefixes",
"-fuse-ld=lld",
]

if exec_os == "darwin":
# These will get expanded by osx_cc_wrapper's `sanitize_option`
link_flags.append("--ld-path=ld.lld" if is_xcompile else "--ld-path=ld64.lld")

# Similar to link_flags, but placed later in the command line such that
# unused symbols are not stripped.
link_libs = []
libunwind_link_flags = []
compiler_rt_link_flags = []

# Flags for ar.
archive_flags = []
is_darwin_exec_and_target = exec_os == "darwin" and not is_xcompile

# Linker flags:
if exec_os == "darwin" and not is_xcompile:
# lld is experimental for Mach-O, so we use the native ld64 linker.
# TODO: How do we cross-compile from Linux to Darwin?
use_lld = False
# Linker and archive flags
if is_darwin_exec_and_target:
link_flags.extend([
"-headerpad_max_install_names",
"-fobjc-link-runtime",
Expand All @@ -163,21 +164,15 @@ def cc_toolchain_config(
# Pre-installed libtool on macOS has -static as default, but llvm-libtool-darwin needs it
# explicitly. cc_common.create_link_variables does not automatically add this either if
# output_file arg to it is None.
archive_flags.extend([
"-static",
])
archive_flags = ["-static"]
else:
# Note that for xcompiling from darwin to linux, the native ld64 is
# not an option because it is not a cross-linker, so lld is the
# only option.
use_lld = True
link_flags.extend([
"-fuse-ld=lld",
"-Wl,--build-id=md5",
"-Wl,--hash-style=gnu",
"-Wl,-z,relro,-z,now",
])
use_libtool = False
archive_flags = []

# Flags related to C++ standard.
# The linker has no way of knowing if there are C++ objects; so we
Expand All @@ -201,21 +196,7 @@ def cc_toolchain_config(
# https://github.com/llvm/llvm-project/commit/0556138624edf48621dd49a463dbe12e7101f17d
cxx_flags.append("-Xclang")
cxx_flags.append("-fno-cxx-modules")
if use_lld:
# For single-platform builds, we can statically link the bundled
# libraries.
link_flags.extend([
"-l:libc++.a",
"-l:libc++abi.a",
])
compiler_rt_link_flags = ["-rtlib=compiler-rt"]
libunwind_link_flags = [
"-l:libunwind.a",
# To support libunwind.
"-lpthread",
"-ldl",
]
else:
if is_darwin_exec_and_target:
# Several system libraries on macOS dynamically link libc++ and
# libc++abi, so static linking them becomes a problem. We need to
# ensure that they are dynamic linked from the system sysroot and
Expand All @@ -233,7 +214,20 @@ def cc_toolchain_config(
"-Bstatic",
"-lunwind",
]

else:
# For single-platform builds, we can statically link the bundled
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment is preexisting but I don't think it's correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it actually might be right because of this bit here:

toolchains_llvm/toolchain/cc_toolchain_config.bzl

Lines 188 to 189 in 6edc43b

if stdlib == "builtin-libc++" and is_xcompile:
stdlib = "stdc++"

i.e. builtin-libc++ implies not is_xcompile, and this arm has exec_os != "darwin" implying linux (exec) -> linux (target)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely no objection to rewording that comment though

# libraries.
link_flags.extend([
"-l:libc++.a",
"-l:libc++abi.a",
])
compiler_rt_link_flags = ["-rtlib=compiler-rt"]
libunwind_link_flags = [
"-l:libunwind.a",
# To support libunwind.
"-lpthread",
"-ldl",
]
elif stdlib == "libc++":
cxx_flags = [
"-std=" + cxx_standard,
Expand Down Expand Up @@ -287,7 +281,7 @@ def cc_toolchain_config(
"dwp": tools_path_prefix + "llvm-dwp",
"gcc": wrapper_bin_prefix + "cc_wrapper.sh",
"gcov": tools_path_prefix + "llvm-profdata",
"ld": tools_path_prefix + "ld.lld" if use_lld else "/usr/bin/ld",
"ld": tools_path_prefix + "ld.lld",
"llvm-cov": tools_path_prefix + "llvm-cov",
"llvm-profdata": tools_path_prefix + "llvm-profdata",
"nm": tools_path_prefix + "llvm-nm",
Expand All @@ -300,9 +294,8 @@ def cc_toolchain_config(
# This was added to `lld` in this patch: http://reviews.llvm.org/D18814
#
# The oldest version of LLVM that we support is 6.0.0 which was released
# after the above patch was merged, so we just set this to `True` when
# `lld` is being used as the linker.
supports_start_end_lib = use_lld
# after the above patch was merged, so we just set this to `True`.
supports_start_end_lib = True

# Replace flags with any user-provided overrides.
if compiler_configuration["compile_flags"] != None:
Expand Down
27 changes: 3 additions & 24 deletions toolchain/osx_cc_wrapper.sh.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ function sanitize_option() {
local -r opt=$1
if [[ ${opt} == */cc_wrapper.sh ]]; then
printf "%s" "${execroot_path}%{toolchain_path_prefix}bin/clang"
elif [[ ${opt} == "-fuse-ld=ld64.lld" ]]; then
elif [[ ${opt} == "--ld-path=ld.lld" ]]; then
echo "--ld-path=${execroot_abs_path}%{toolchain_path_prefix}bin/ld.lld"
elif [[ ${opt} == "--ld-path=ld64.lld" ]]; then
echo "--ld-path=${execroot_abs_path}%{toolchain_path_prefix}bin/ld64.lld"
elif [[ ${opt} =~ ^-fsanitize-(ignore|black)list=[^/] ]]; then
# shellcheck disable=SC2206
Expand All @@ -88,9 +90,6 @@ cmd=()
for ((i = 0; i <= $#; i++)); do
if [[ ${!i} == @* ]]; then
while IFS= read -r opt; do
if [[ ${opt} == "-fuse-ld=ld64.lld" ]]; then
cmd+=("-fuse-ld=lld")
fi
opt="$(
set -e
sanitize_option "${opt}"
Expand All @@ -108,26 +107,6 @@ for ((i = 0; i <= $#; i++)); do
fi
done

# On macOS, we use ld as the linker for single-platform builds (i.e., when not
# cross-compiling). Some applications may remove /usr/bin from PATH before
# calling this script, which would make /usr/bin/ld unreachable. For example,
# rules_rust does not set PATH (unless the user explicitly sets PATH in env
# through attributes) [1] when calling rustc, and rustc does not replace an
# unset PATH with a reasonable default either ([2], [3]), which results in CC
# being called with PATH={sysroot}/{rust_lib}/bin. Note that rules_cc [4] and
# rules_go [5] do ensure that /usr/bin is in PATH.
# [1]: https://github.com/bazelbuild/rules_rust/blob/e589105b4e8181dd1d0d8ccaa0cf3267efb06e86/cargo/cargo_build_script.bzl#L66-L68
# [2]: https://github.com/rust-lang/rust/blob/1c03f0d0ba4fee54b7aa458f4d3ad989d8bf7b34/compiler/rustc_session/src/session.rs#L804-L813
# [3]: https://github.com/rust-lang/rust/blob/1c03f0d0ba4fee54b7aa458f4d3ad989d8bf7b34/compiler/rustc_codegen_ssa/src/back/link.rs#L640-L645
# [4]: https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java;l=529;drc=72caead7b428fd50164079956ec368fc54a9567c
# [5]: https://github.com/bazelbuild/rules_go/blob/63dfd99403076331fef0775d52a8039d502d4115/go/private/context.bzl#L434
# Let's restore /usr/bin to PATH in such cases. Note that /usr/bin is not a
# writeable directory on macOS even with sudo privileges, so it should be safe
# to add it to PATH even when the application wants to use a very strict PATH.
if [[ ":${PATH}:" != *":/usr/bin:"* ]]; then
PATH="${PATH}:/usr/bin"
fi

# Call the C++ compiler.
"${cmd[@]}"

Expand Down
Loading