Apple: Always pass SDK root when linking with cc, and pass it via SDKROOT env var

compiler/rustc_codegen_ssa/messages.ftl

@@ -401,6 +401,9 @@ codegen_ssa_version_script_write_failure = failed to write version script: {$err
 
 codegen_ssa_visual_studio_not_installed = you may need to install Visual Studio build tools with the "C++ build tools" workload
 
+codegen_ssa_xcrun_about =
+    the SDK is needed by the linker to know where to find symbols in system libraries and for embedding the SDK version in the final object file
+
 codegen_ssa_xcrun_command_line_tools_insufficient =
     when compiling for iOS, tvOS, visionOS or watchOS, you need a full installation of Xcode
 

compiler/rustc_codegen_ssa/src/back/apple.rs

@@ -160,6 +160,10 @@ pub(super) fn add_version_to_llvm_target(
 pub(super) fn get_sdk_root(sess: &Session) -> Option<PathBuf> {
     let sdk_name = sdk_name(&sess.target);
 
+    // Attempt to invoke `xcrun` to find the SDK.
+    //
+    // Note that when cross-compiling from e.g. Linux, the `xcrun` binary may sometimes be provided
+    // as a shim by a cross-compilation helper tool. It usually isn't, but we still try nonetheless.
     match xcrun_show_sdk_path(sdk_name, sess.verbose_internals()) {
         Ok((path, stderr)) => {
             // Emit extra stderr, such as if `-verbose` was passed, or if `xcrun` emitted a warning.
@@ -169,7 +173,19 @@ pub(super) fn get_sdk_root(sess: &Session) -> Option<PathBuf> {
             Some(path)
         }
         Err(err) => {
-            let mut diag = sess.dcx().create_err(err);
+            // Failure to find the SDK is not a hard error, since the user might have specified it
+            // in a manner unknown to us (moreso if cross-compiling):
+            // - A compiler driver like `zig cc` which links using an internally bundled SDK.
+            // - Extra linker arguments (`-Clink-arg=-syslibroot`).
+            // - A custom linker or custom compiler driver.
+            //
+            // Though we still warn, since such cases are uncommon, and it is very hard to debug if
+            // you do not know the details.
+            //
+            // FIXME(madsmtm): Make this a lint, to allow deny warnings to work.
+            // (Or fix <https://github.com/rust-lang/rust/issues/21204>).
+            let mut diag = sess.dcx().create_warn(err);
+            diag.note(fluent::codegen_ssa_xcrun_about);
 
             // Recognize common error cases, and give more Rust-specific error messages for those.
             if let Some(developer_dir) = xcode_select_developer_dir() {
@@ -209,6 +225,8 @@ fn xcrun_show_sdk_path(
     sdk_name: &'static str,
     verbose: bool,
 ) -> Result<(PathBuf, String), XcrunError> {
+    // Intentionally invoke the `xcrun` in PATH, since e.g. nixpkgs provide an `xcrun` shim, so we
+    // don't want to require `/usr/bin/xcrun`.
     let mut cmd = Command::new("xcrun");
     if verbose {
         cmd.arg("--verbose");
@@ -280,7 +298,7 @@ fn stdout_to_path(mut stdout: Vec<u8>) -> PathBuf {
     }
     #[cfg(unix)]
     let path = <OsString as std::os::unix::ffi::OsStringExt>::from_vec(stdout);
-    #[cfg(not(unix))] // Unimportant, this is only used on macOS
-    let path = OsString::from(String::from_utf8(stdout).unwrap());
+    #[cfg(not(unix))] // Not so important, this is mostly used on macOS
+    let path = OsString::from(String::from_utf8(stdout).expect("stdout must be UTF-8"));
     PathBuf::from(path)
 }

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -3194,39 +3194,60 @@ fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavo
 }
 
 fn add_apple_sdk(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) -> Option<PathBuf> {
-    let os = &sess.target.os;
-    if sess.target.vendor != "apple"
-        || !matches!(os.as_ref(), "ios" | "tvos" | "watchos" | "visionos" | "macos")
-        || !matches!(flavor, LinkerFlavor::Darwin(..))
-    {
+    if !sess.target.is_like_darwin {
         return None;
     }
-
-    if os == "macos" && !matches!(flavor, LinkerFlavor::Darwin(Cc::No, _)) {
+    let LinkerFlavor::Darwin(cc, _) = flavor else {
         return None;
-    }
-
-    let sdk_root = sess.time("get_apple_sdk_root", || get_apple_sdk_root(sess))?;
+    };
 
-    match flavor {
-        LinkerFlavor::Darwin(Cc::Yes, _) => {
-            // Use `-isysroot` instead of `--sysroot`, as only the former
-            // makes Clang treat it as a platform SDK.
-            //
-            // This is admittedly a bit strange, as on most targets
-            // `-isysroot` only applies to include header files, but on Apple
-            // targets this also applies to libraries and frameworks.
-            cmd.cc_arg("-isysroot");
-            cmd.cc_arg(&sdk_root);
-        }
-        LinkerFlavor::Darwin(Cc::No, _) => {
-            cmd.link_arg("-syslibroot");
-            cmd.link_arg(&sdk_root);
-        }
-        _ => unreachable!(),
+    // The default compiler driver on macOS is at `/usr/bin/cc`. This is a trampoline binary that
+    // effectively invokes `xcrun cc` internally to look up both the compiler binary and the SDK
+    // root from the current Xcode installation. When cross-compiling, when `rustc` is invoked
+    // inside Xcode, or when invoking the linker directly, this default logic is unsuitable, so
+    // instead we invoke `xcrun` manually.
+    //
+    // (Note that this doesn't mean we get a duplicate lookup here - passing `SDKROOT` below will
+    // cause the trampoline binary to skip looking up the SDK itself).
+    let sdkroot = sess.time("get_apple_sdk_root", || get_apple_sdk_root(sess))?;
+
+    if cc == Cc::Yes {
+        // There are a few options to pass the SDK root when linking with a C/C++ compiler:
+        // - The `--sysroot` flag.
+        // - The `-isysroot` flag.
+        // - The `SDKROOT` environment variable.
+        //
+        // `--sysroot` isn't actually enough to get Clang to treat it as a platform SDK, you need
+        // to specify `-isysroot`. This is admittedly a bit strange, as on most targets `-isysroot`
+        // only applies to include header files, but on Apple targets it also applies to libraries
+        // and frameworks.
+        //
+        // This leaves the choice between `-isysroot` and `SDKROOT`. Both are supported by Clang and
+        // GCC, though they may not be supported by all compiler drivers. We choose `SDKROOT`,
+        // primarily because that is the same interface that is used when invoking the tool under
+        // `xcrun -sdk macosx $tool`.
+        //
+        // In that sense, if a given compiler driver does not support `SDKROOT`, the blame is fairly
+        // clearly in the tool in question, since they also don't support being run under `xcrun`.
+        //
+        // Additionally, `SDKROOT` is an environment variable and thus optional. It also has lower
+        // precedence than `-isysroot`, so a custom compiler driver that does not support it and
+        // instead figures out the SDK on their own can easily do so by using `-isysroot`.
+        //
+        // (This in particular affects Clang built with the `DEFAULT_SYSROOT` CMake flag, such as
+        // the one provided by some versions of Homebrew's `llvm` package. Those will end up
+        // ignoring the value we set here, and instead use their built-in sysroot).
+        cmd.cmd().env("SDKROOT", &sdkroot);
+    } else {
+        // When invoking the linker directly, we use the `-syslibroot` parameter. `SDKROOT` is not
+        // read by the linker, so it's really the only option.
+        //
+        // This is also what Clang does.
+        cmd.link_arg("-syslibroot");
+        cmd.link_arg(&sdkroot);
     }
 
-    Some(sdk_root)
+    Some(sdkroot)
 }
 
 fn get_apple_sdk_root(sess: &Session) -> Option<PathBuf> {
@@ -3255,7 +3276,13 @@ fn get_apple_sdk_root(sess: &Session) -> Option<PathBuf> {
             }
             "macosx"
                 if sdkroot.contains("iPhoneOS.platform")
-                    || sdkroot.contains("iPhoneSimulator.platform") => {}
+                    || sdkroot.contains("iPhoneSimulator.platform")
+                    || sdkroot.contains("AppleTVOS.platform")
+                    || sdkroot.contains("AppleTVSimulator.platform")
+                    || sdkroot.contains("WatchOS.platform")
+                    || sdkroot.contains("WatchSimulator.platform")
+                    || sdkroot.contains("XROS.platform")
+                    || sdkroot.contains("XRSimulator.platform") => {}
             "watchos"
                 if sdkroot.contains("WatchSimulator.platform")
                     || sdkroot.contains("MacOSX.platform") => {}

compiler/rustc_target/src/spec/base/apple/mod.rs

@@ -1,5 +1,4 @@
 use std::borrow::Cow;
-use std::env;
 use std::fmt::{Display, from_fn};
 use std::num::ParseIntError;
 use std::str::FromStr;
@@ -209,29 +208,10 @@ fn link_env_remove(os: &'static str) -> StaticCow<[StaticCow<str>]> {
     // that's only applicable to cross-OS compilation. Always leave anything for the
     // host OS alone though.
     if os == "macos" {
-        let mut env_remove = Vec::with_capacity(2);
-        // Remove the `SDKROOT` environment variable if it's clearly set for the wrong platform, which
-        // may occur when we're linking a custom build script while targeting iOS for example.
-        if let Ok(sdkroot) = env::var("SDKROOT") {
-            if sdkroot.contains("iPhoneOS.platform")
-                || sdkroot.contains("iPhoneSimulator.platform")
-                || sdkroot.contains("AppleTVOS.platform")
-                || sdkroot.contains("AppleTVSimulator.platform")
-                || sdkroot.contains("WatchOS.platform")
-                || sdkroot.contains("WatchSimulator.platform")
-                || sdkroot.contains("XROS.platform")
-                || sdkroot.contains("XRSimulator.platform")
-            {
-                env_remove.push("SDKROOT".into())
-            }
-        }
-        // Additionally, `IPHONEOS_DEPLOYMENT_TARGET` must not be set when using the Xcode linker at
+        // `IPHONEOS_DEPLOYMENT_TARGET` must not be set when using the Xcode linker at
         // "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld",
         // although this is apparently ignored when using the linker at "/usr/bin/ld".
-        env_remove.push("IPHONEOS_DEPLOYMENT_TARGET".into());
-        env_remove.push("TVOS_DEPLOYMENT_TARGET".into());
-        env_remove.push("XROS_DEPLOYMENT_TARGET".into());
-        env_remove.into()
+        cvs!["IPHONEOS_DEPLOYMENT_TARGET", "TVOS_DEPLOYMENT_TARGET", "XROS_DEPLOYMENT_TARGET"]
     } else {
         // Otherwise if cross-compiling for a different OS/SDK (including Mac Catalyst), remove any part
         // of the linking environment that's wrong and reversed.

src/doc/unstable-book/src/compiler-environment-variables/SDKROOT.md

@@ -1,6 +1,6 @@
 # `SDKROOT`
 
 This environment variable is used on Apple targets.
-It is passed through to the linker (currently either as `-isysroot` or `-syslibroot`).
+It is passed through to the linker (currently either directly or via the `-syslibroot` flag).
 
 Note that this variable is not always respected. When the SDKROOT is clearly wrong (e.g. when the platform of the SDK does not match the `--target` used by rustc), this is ignored and rustc does its own detection.

tests/run-make/link-under-xcode/foo.rs

@@ -0,0 +1 @@
+fn main() {}

tests/run-make/link-under-xcode/rmake.rs

@@ -0,0 +1,32 @@
+//! Test that linking works under an environment similar to what Xcode sets up.
+//!
+//! Regression test for https://github.com/rust-lang/rust/issues/80817.
+
+//@ only-apple
+
+use run_make_support::{cmd, rustc, target};
+
+fn main() {
+    // Fetch toolchain `/usr/bin` directory. Usually:
+    // /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
+    let clang_bin = cmd("xcrun").arg("--find").arg("clang").run().stdout_utf8();
+    let toolchain_bin = clang_bin.trim().strip_suffix("/clang").unwrap();
+
+    // Put toolchain directory at the front of PATH.
+    let path = format!("{}:{}", toolchain_bin, std::env::var("PATH").unwrap());
+
+    // Check that compiling and linking still works.
+    //
+    // Removing `SDKROOT` is necessary for the test to excercise what we want, since bootstrap runs
+    // under `/usr/bin/python3`, which will set SDKROOT for us.
+    rustc().target(target()).env_remove("SDKROOT").env("PATH", &path).input("foo.rs").run();
+
+    // Also check linking directly with the system linker.
+    rustc()
+        .target(target())
+        .env_remove("SDKROOT")
+        .env("PATH", &path)
+        .input("foo.rs")
+        .arg("-Clinker-flavor=ld")
+        .run();
+}