mathieu/granite-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Rollup merge of #105123 - BlackHoleFox:fixing-the-macos-deployment, r=oli-obk

Browse source 
Fix passing MACOSX_DEPLOYMENT_TARGET to the linker

I messed up in https://github.com/rust-lang/rust/pull/103929 when merging the two base files together and as a result, started ignoring `MACOSX_DEPLOYMENT_TARGET` at the linker level. This ended up being the cause of nighty builds not running on older macOS versions.

My original hope with the previous PR was that CI would have caught something like that but there were only tests checking the compiler target definitions in codegen tests. Because of how badly this sucks to break, I put together a new test via `run-make` that actually confirms the deployment target set makes it to the linker instead of just LLVM.

Closes https://github.com/rust-lang/rust/issues/104570 (for real this time)
This commit is contained in:
Matthias Krüger 2022-12-04 11:38:51 +01:00 • committed by GitHub
commit 7fe9597775
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 76 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
compiler/rustc_target/src/spec/aarch64_apple_darwin.rs
View file

@ -1,4 +1,4 @@
use super::apple_base::{macos_link_env_remove, macos_llvm_target, opts, Arch};
use super::apple_base::{macos_llvm_target, opts, Arch};
use crate::spec::{FramePointer, SanitizerSet, Target, TargetOptions};
pub fn target() -> Target {
@ -10,8 +10,6 @@ pub fn target() -> Target {
// FIXME: The leak sanitizer currently fails the tests, see #88132.
base.supported_sanitizers = SanitizerSet::ADDRESS | SanitizerSet::CFI | SanitizerSet::THREAD;
base.link_env_remove.to_mut().extend(macos_link_env_remove());
Target {
// Clang automatically chooses a more specific target based on
// MACOSX_DEPLOYMENT_TARGET. To enable cross-language LTO to work

19
compiler/rustc_target/src/spec/apple/tests.rs
View file

@ -1,6 +1,6 @@
use crate::spec::{
aarch64_apple_ios_sim, aarch64_apple_watchos_sim, x86_64_apple_ios, x86_64_apple_tvos,
x86_64_apple_watchos_sim,
aarch64_apple_darwin, aarch64_apple_ios_sim, aarch64_apple_watchos_sim, i686_apple_darwin,
x86_64_apple_darwin, x86_64_apple_ios, x86_64_apple_tvos, x86_64_apple_watchos_sim,
};
#[test]
@ -18,3 +18,18 @@ fn simulator_targets_set_abi() {
assert_eq!(target.abi, "sim")
}
}
#[test]
fn macos_link_environment_unmodified() {
let all_macos_targets = [
aarch64_apple_darwin::target(),
i686_apple_darwin::target(),
x86_64_apple_darwin::target(),
];
for target in all_macos_targets {
// macOS targets should only remove information for cross-compiling, but never
// for the host.
assert_eq!(target.link_env_remove, crate::spec::cvs!["IPHONEOS_DEPLOYMENT_TARGET"]);
}
}

54
compiler/rustc_target/src/spec/apple_base.rs
View file

@ -72,16 +72,6 @@ impl Arch {
Arm64_sim => "apple-a12",
}
}
fn link_env_remove(self) -> StaticCow<[StaticCow<str>]> {
match self {
Armv7 | Armv7k | Armv7s | Arm64 | Arm64_32 | I386 | I686 | X86_64 | X86_64_sim
| Arm64_sim => {
cvs!["MACOSX_DEPLOYMENT_TARGET"]
}
X86_64_macabi | Arm64_macabi => cvs!["IPHONEOS_DEPLOYMENT_TARGET"],
}
}
}
fn pre_link_args(os: &'static str, arch: Arch, abi: &'static str) -> LinkArgs {
@ -140,7 +130,7 @@ pub fn opts(os: &'static str, arch: Arch) -> TargetOptions {
abi: abi.into(),
os: os.into(),
cpu: arch.target_cpu().into(),
link_env_remove: arch.link_env_remove(),
link_env_remove: link_env_remove(arch, os),
vendor: "apple".into(),
linker_flavor: LinkerFlavor::Darwin(Cc::Yes, Lld::No),
// macOS has -dead_strip, which doesn't rely on function_sections
@ -211,20 +201,38 @@ pub fn macos_llvm_target(arch: Arch) -> String {
format!("{}-apple-macosx{}.{}.0", arch.target_name(), major, minor)
}
pub fn macos_link_env_remove() -> Vec<StaticCow<str>> {
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") {
env_remove.push("SDKROOT".into())
fn link_env_remove(arch: Arch, os: &'static str) -> StaticCow<[StaticCow<str>]> {
// Apple platforms only officially support macOS as a host for any compilation.
//
// If building for macOS, we go ahead and remove any erronous environment state
// 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")
{
env_remove.push("SDKROOT".into())
}
}
// Additionally, `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.into()
} else {
// Otherwise if cross-compiling for a different OS/SDK, remove any part
// of the linking environment that's wrong and reversed.
match arch {
Armv7 | Armv7k | Armv7s | Arm64 | Arm64_32 | I386 | I686 | X86_64 | X86_64_sim
| Arm64_sim => {
cvs!["MACOSX_DEPLOYMENT_TARGET"]
}
X86_64_macabi | Arm64_macabi => cvs!["IPHONEOS_DEPLOYMENT_TARGET"],
}
}
// Additionally, `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
}
fn ios_deployment_target() -> (u32, u32) {

3
compiler/rustc_target/src/spec/i686_apple_darwin.rs
View file

@ -1,4 +1,4 @@
use super::apple_base::{macos_link_env_remove, macos_llvm_target, opts, Arch};
use super::apple_base::{macos_llvm_target, opts, Arch};
use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, StackProbeType, Target, TargetOptions};
pub fn target() -> Target {
@ -7,7 +7,6 @@ pub fn target() -> Target {
let mut base = opts("macos", arch);
base.max_atomic_width = Some(64);
base.add_pre_link_args(LinkerFlavor::Darwin(Cc::Yes, Lld::No), &["-m32"]);
base.link_env_remove.to_mut().extend(macos_link_env_remove());
base.stack_probes = StackProbeType::X86;
base.frame_pointer = FramePointer::Always;

3
compiler/rustc_target/src/spec/x86_64_apple_darwin.rs
View file

@ -1,4 +1,4 @@
use super::apple_base::{macos_link_env_remove, macos_llvm_target, opts, Arch};
use super::apple_base::{macos_llvm_target, opts, Arch};
use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, SanitizerSet};
use crate::spec::{StackProbeType, Target, TargetOptions};
@ -8,7 +8,6 @@ pub fn target() -> Target {
base.max_atomic_width = Some(128); // core2 supports cmpxchg16b
base.frame_pointer = FramePointer::Always;
base.add_pre_link_args(LinkerFlavor::Darwin(Cc::Yes, Lld::No), &["-m64"]);
base.link_env_remove.to_mut().extend(macos_link_env_remove());
base.stack_probes = StackProbeType::X86;
base.supported_sanitizers =
SanitizerSet::ADDRESS | SanitizerSet::CFI | SanitizerSet::LEAK | SanitizerSet::THREAD;

21
src/test/run-make/macos-deployment-target/Makefile Normal file
View file

@ -0,0 +1,21 @@
# only-macos
#
# Check that a set deployment target actually makes it to the linker.
# This is important since its a compatibility hazard. The linker will
# generate load commands differently based on what minimum OS it can assume.
include ../../run-make-fulldeps/tools.mk
ifeq ($(strip $(shell uname -m)),arm64)
GREP_PATTERN = "minos 11.0"
else
GREP_PATTERN = "version 10.9"
endif
OUT_FILE=$(TMPDIR)/with_deployment_target.dylib
all:
env MACOSX_DEPLOYMENT_TARGET=10.9 $(RUSTC) with_deployment_target.rs -o $(OUT_FILE)
# XXX: The check is for either the x86_64 minimum OR the aarch64 minimum (M1 starts at macOS 11).
# They also use different load commands, so we let that change with each too. The aarch64 check
# isn't as robust as the x86 one, but testing both seems unneeded.
vtool -show-build $(OUT_FILE) | $(CGREP) -e $(GREP_PATTERN)

4
src/test/run-make/macos-deployment-target/with_deployment_target.rs Normal file
View file

@ -0,0 +1,4 @@
#![crate_type = "cdylib"]
#[allow(dead_code)]
fn something_and_nothing() {}