Always use LLD on darwin (#380)

It sounds like a few folks have tried the `-fuse-ld=lld` option and it
works for them, so let's try to roll it out more broadly.

Fixes: #212
Fixes #324

Author: dzbarsky

toolchain/cc_toolchain_config.bzl

@@ -193,8 +193,13 @@ def cc_toolchain_config(
     link_flags = [
         "--target=" + target_system_name,
         "-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")
+
     stdlib = compiler_configuration["stdlib"]
     if stdlib != "none":
         link_flags.extend([
@@ -207,45 +212,33 @@ def cc_toolchain_config(
     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
+    if is_darwin_exec_and_target:
         link_flags.extend([
             "-headerpad_max_install_names",
             "-fobjc-link-runtime",
         ])
 
         # Use the bundled libtool (llvm-libtool-darwin).
         use_libtool = True
-
-        # 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",
-        ])
     elif target_arch in ["wasm32", "wasm64"]:
         # lld is invoked as wasm-ld for WebAssembly targets.
-        use_lld = True
         use_libtool = False
     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
 
+    # 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 = ["-static"] if is_darwin_exec_and_target else []
+
     # Flags related to C++ standard.
     # The linker has no way of knowing if there are C++ objects; so we
     # always link C++ libraries.
@@ -259,21 +252,7 @@ def cc_toolchain_config(
             "-std=" + cxx_standard,
             "-stdlib=libc++",
         ]
-        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
@@ -291,6 +270,20 @@ def cc_toolchain_config(
                 "-Bstatic",
                 "-lunwind",
             ]
+        else:
+            # 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",
+            ]
 
     elif stdlib == "libc++":
         cxx_flags = [
@@ -368,7 +361,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",
@@ -382,9 +375,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:

toolchain/osx_cc_wrapper.sh.tpl

@@ -131,8 +131,9 @@ function sanitize_option() {
   local -r opt=$1
   if [[ ${opt} == */cc_wrapper.sh ]]; then
     printf "%s" "${toolchain_path_prefix}bin/clang"
-  elif [[ ${opt} == "-fuse-ld=ld64.lld" ]]; then
-    echo "--ld-path=${toolchain_path_prefix_abs}bin/ld64.lld"
+  elif [[ ${opt} == "--ld-path=ld.lld" || ${opt} == "--ld-path=ld64.lld" ]]; then
+    local ld_name=${opt#--ld-path=}
+    echo "--ld-path=${toolchain_path_prefix_abs}bin/${ld_name}"
   elif [[ ${opt} =~ ^-fsanitize-(ignore|black)list=[^/] ]] && [[ ${script_dir} == /* ]]; then
     # shellcheck disable=SC2206
     parts=(${opt/=/ }) # Split flag name and value into array.
@@ -149,9 +150,6 @@ for ((i = 0; i <= $#; i++)); do
     tmpfile=$(mktemp)
     CLEANUP_FILES+=("${tmpfile}")
     while IFS= read -r opt; do
-      if [[ ${opt} == "-fuse-ld=ld64.lld" ]]; then
-        echo "-fuse-ld=lld" >>"${tmpfile}"
-      fi
       opt="$(
         set -e
         sanitize_option "${opt}"
@@ -169,26 +167,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[@]}"