Skip to content

Commit

Permalink
Always attempt to get SDK root, and pass it to Clang via env var
Browse files Browse the repository at this point in the history
The exact reasoning why we do not always pass the SDK root with
`-isysroot` to `cc` when linking on macOS eludes me (the git history
dead ends in #100286), but I suspect it's because we want to support
`cc`s which do not support this option.

So instead, we pass the SDK root via the environment variable SDKROOT.
This way, compiler drivers that support setting the SDK root (such as
Clang and GCC) can use it, while compiler drivers that don't (presumably
because they figure out the SDK root in some other way) can just ignore
it.

This fixes #80817 (by always
passing the SDK root, even when linking with cc on macOS).
  • Loading branch information
madsmtm committed Oct 31, 2024
1 parent bbb59cd commit 5d010db
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 49 deletions.
76 changes: 48 additions & 28 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3126,47 +3126,61 @@ 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_osx {
return None;
}

if os == "macos" && !matches!(flavor, LinkerFlavor::Darwin(Cc::No, _)) {
let LinkerFlavor::Darwin(cc, _) = flavor else {
return None;
}
};

let sdk_name = apple::sdk_name(&sess.target);

let sdk_root = match get_apple_sdk_root(sdk_name) {
let sdkroot = match get_apple_sdk_root(sdk_name) {
Ok(s) => s,
Err(e) => {
sess.dcx().emit_err(e);
// If cross compiling from non-macOS, the user might be using something like `zig cc`.
//
// In that case, we shouldn't error when the SDK is missing, though we still warn.
if cfg!(target_os = "macos") {
sess.dcx().emit_err(e);
} else {
sess.dcx().emit_warn(e);
}
return None;
}
};

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!(),
if cc == Cc::Yes {
// To pass the SDK root to `cc`, we have a few options:
// 1. `--sysroot` flag.
// 2. `-isysroot` flag.
// 3. `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.
//
// Now, while the `-isysroot` flag is pretty well supported (both Clang and GCC implements
// the desired behaviour), it may not be understood by any `cc`'s that the user might want
// to use.
//
// So to better support such use-cases, we pass the SDK root in the standard environment
// variable instead. This is also supported by GCC since 2019:
// <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87243>
//
// This also works better with the trampoline `/usr/bin/cc` which calls `xcrun cc`
// internally, since the presence of `SDKROOT` means it won't have to look up the SDK root
// itself.
cmd.cmd().env("SDKROOT", &sdkroot);
} else {
// For `ld64`, we use the `-syslibroot` parameter (this is what Clang passes, and `SDKROOT`
// is not read by `ld64` so it's really the only option).
cmd.link_arg("-syslibroot");
cmd.link_arg(&sdkroot);
}

Some(sdk_root)
Some(sdkroot)
}

fn get_apple_sdk_root(sdk_name: &'static str) -> Result<PathBuf, errors::AppleSdkError> {
Expand All @@ -3191,7 +3205,13 @@ fn get_apple_sdk_root(sdk_name: &'static str) -> Result<PathBuf, errors::AppleSd
}
"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") => {}
Expand Down
23 changes: 2 additions & 21 deletions compiler/rustc_target/src/spec/base/apple/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,29 +292,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.
Expand Down
1 change: 1 addition & 0 deletions tests/run-make/link-under-xcode/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
32 changes: 32 additions & 0 deletions tests/run-make/link-under-xcode/rmake.rs
Original file line number Diff line number Diff line change
@@ -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 cc_bin = cmd("xcrun").arg("--find").arg("cc").run().stdout_utf8();
let toolchain_bin = cc_bin.trim().strip_suffix("/cc").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 with ld64.
rustc()
.target(target())
.env_remove("SDKROOT")
.env("PATH", &path)
.arg("-Clinker-flavor=ld")
.input("foo.rs")
.run();
}

0 comments on commit 5d010db

Please sign in to comment.