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 rust-lang#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 rust-lang#80817 (by always
passing the SDK root, even when linking with cc on macOS).
  • Loading branch information
@madsmtm
madsmtm committed Oct 9, 2024
1 parent 8f42605 commit e6479fc
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 49 deletions.
67 changes: 40 additions & 27 deletions compiler/rustc_codegen_ssa/src/back/link.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3101,47 +3101,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_osx {
return None;
}

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

let sdk_name = rustc_target::spec::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 (and there are no other options).
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 Down
25 changes: 3 additions & 22 deletions compiler/rustc_target/src/spec/base/apple/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::borrow::Cow;
use std::fmt;
use std::num::ParseIntError;
use std::str::FromStr;
use std::{env, fmt};

use serde::de;

Expand Down Expand Up @@ -279,29 +279,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

0 comments on commit e6479fc

Please sign in to comment.