Skip to content

Commit 7fe9597

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #105123 - BlackHoleFox:fixing-the-macos-deployment, r=oli-obk
Fix passing MACOSX_DEPLOYMENT_TARGET to the linker I messed up in #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 #104570 (for real this time)
2 parents 785b47d + 56592d3 commit 7fe9597

File tree

7 files changed

+76
-32
lines changed

7 files changed

+76
-32
lines changed

‎compiler/rustc_target/src/spec/aarch64_apple_darwin.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::apple_base::{macos_link_env_remove, macos_llvm_target, opts, Arch};
1+
use super::apple_base::{macos_llvm_target, opts, Arch};
22
use crate::spec::{FramePointer, SanitizerSet, Target, TargetOptions};
33

44
pub fn target() -> Target {
@@ -10,8 +10,6 @@ pub fn target() -> Target {
1010
// FIXME: The leak sanitizer currently fails the tests, see #88132.
1111
base.supported_sanitizers = SanitizerSet::ADDRESS | SanitizerSet::CFI | SanitizerSet::THREAD;
1212

13-
base.link_env_remove.to_mut().extend(macos_link_env_remove());
14-
1513
Target {
1614
// Clang automatically chooses a more specific target based on
1715
// MACOSX_DEPLOYMENT_TARGET. To enable cross-language LTO to work

‎compiler/rustc_target/src/spec/apple/tests.rs

+17-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::spec::{
2-
aarch64_apple_ios_sim, aarch64_apple_watchos_sim, x86_64_apple_ios, x86_64_apple_tvos,
3-
x86_64_apple_watchos_sim,
2+
aarch64_apple_darwin, aarch64_apple_ios_sim, aarch64_apple_watchos_sim, i686_apple_darwin,
3+
x86_64_apple_darwin, x86_64_apple_ios, x86_64_apple_tvos, x86_64_apple_watchos_sim,
44
};
55

66
#[test]
@@ -18,3 +18,18 @@ fn simulator_targets_set_abi() {
1818
assert_eq!(target.abi, "sim")
1919
}
2020
}
21+
22+
#[test]
23+
fn macos_link_environment_unmodified() {
24+
let all_macos_targets = [
25+
aarch64_apple_darwin::target(),
26+
i686_apple_darwin::target(),
27+
x86_64_apple_darwin::target(),
28+
];
29+
30+
for target in all_macos_targets {
31+
// macOS targets should only remove information for cross-compiling, but never
32+
// for the host.
33+
assert_eq!(target.link_env_remove, crate::spec::cvs!["IPHONEOS_DEPLOYMENT_TARGET"]);
34+
}
35+
}

‎compiler/rustc_target/src/spec/apple_base.rs

+31-23
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,6 @@ impl Arch {
7272
Arm64_sim => "apple-a12",
7373
}
7474
}
75-
76-
fn link_env_remove(self) -> StaticCow<[StaticCow<str>]> {
77-
match self {
78-
Armv7 | Armv7k | Armv7s | Arm64 | Arm64_32 | I386 | I686 | X86_64 | X86_64_sim
79-
| Arm64_sim => {
80-
cvs!["MACOSX_DEPLOYMENT_TARGET"]
81-
}
82-
X86_64_macabi | Arm64_macabi => cvs!["IPHONEOS_DEPLOYMENT_TARGET"],
83-
}
84-
}
8575
}
8676

8777
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 {
140130
abi: abi.into(),
141131
os: os.into(),
142132
cpu: arch.target_cpu().into(),
143-
link_env_remove: arch.link_env_remove(),
133+
link_env_remove: link_env_remove(arch, os),
144134
vendor: "apple".into(),
145135
linker_flavor: LinkerFlavor::Darwin(Cc::Yes, Lld::No),
146136
// macOS has -dead_strip, which doesn't rely on function_sections
@@ -211,20 +201,38 @@ pub fn macos_llvm_target(arch: Arch) -> String {
211201
format!("{}-apple-macosx{}.{}.0", arch.target_name(), major, minor)
212202
}
213203

214-
pub fn macos_link_env_remove() -> Vec<StaticCow<str>> {
215-
let mut env_remove = Vec::with_capacity(2);
216-
// Remove the `SDKROOT` environment variable if it's clearly set for the wrong platform, which
217-
// may occur when we're linking a custom build script while targeting iOS for example.
218-
if let Ok(sdkroot) = env::var("SDKROOT") {
219-
if sdkroot.contains("iPhoneOS.platform") || sdkroot.contains("iPhoneSimulator.platform") {
220-
env_remove.push("SDKROOT".into())
204+
fn link_env_remove(arch: Arch, os: &'static str) -> StaticCow<[StaticCow<str>]> {
205+
// Apple platforms only officially support macOS as a host for any compilation.
206+
//
207+
// If building for macOS, we go ahead and remove any erronous environment state
208+
// that's only applicable to cross-OS compilation. Always leave anything for the
209+
// host OS alone though.
210+
if os == "macos" {
211+
let mut env_remove = Vec::with_capacity(2);
212+
// Remove the `SDKROOT` environment variable if it's clearly set for the wrong platform, which
213+
// may occur when we're linking a custom build script while targeting iOS for example.
214+
if let Ok(sdkroot) = env::var("SDKROOT") {
215+
if sdkroot.contains("iPhoneOS.platform") || sdkroot.contains("iPhoneSimulator.platform")
216+
{
217+
env_remove.push("SDKROOT".into())
218+
}
219+
}
220+
// Additionally, `IPHONEOS_DEPLOYMENT_TARGET` must not be set when using the Xcode linker at
221+
// "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld",
222+
// although this is apparently ignored when using the linker at "/usr/bin/ld".
223+
env_remove.push("IPHONEOS_DEPLOYMENT_TARGET".into());
224+
env_remove.into()
225+
} else {
226+
// Otherwise if cross-compiling for a different OS/SDK, remove any part
227+
// of the linking environment that's wrong and reversed.
228+
match arch {
229+
Armv7 | Armv7k | Armv7s | Arm64 | Arm64_32 | I386 | I686 | X86_64 | X86_64_sim
230+
| Arm64_sim => {
231+
cvs!["MACOSX_DEPLOYMENT_TARGET"]
232+
}
233+
X86_64_macabi | Arm64_macabi => cvs!["IPHONEOS_DEPLOYMENT_TARGET"],
221234
}
222235
}
223-
// Additionally, `IPHONEOS_DEPLOYMENT_TARGET` must not be set when using the Xcode linker at
224-
// "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld",
225-
// although this is apparently ignored when using the linker at "/usr/bin/ld".
226-
env_remove.push("IPHONEOS_DEPLOYMENT_TARGET".into());
227-
env_remove
228236
}
229237

230238
fn ios_deployment_target() -> (u32, u32) {

‎compiler/rustc_target/src/spec/i686_apple_darwin.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::apple_base::{macos_link_env_remove, macos_llvm_target, opts, Arch};
1+
use super::apple_base::{macos_llvm_target, opts, Arch};
22
use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, StackProbeType, Target, TargetOptions};
33

44
pub fn target() -> Target {
@@ -7,7 +7,6 @@ pub fn target() -> Target {
77
let mut base = opts("macos", arch);
88
base.max_atomic_width = Some(64);
99
base.add_pre_link_args(LinkerFlavor::Darwin(Cc::Yes, Lld::No), &["-m32"]);
10-
base.link_env_remove.to_mut().extend(macos_link_env_remove());
1110
base.stack_probes = StackProbeType::X86;
1211
base.frame_pointer = FramePointer::Always;
1312

‎compiler/rustc_target/src/spec/x86_64_apple_darwin.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::apple_base::{macos_link_env_remove, macos_llvm_target, opts, Arch};
1+
use super::apple_base::{macos_llvm_target, opts, Arch};
22
use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, SanitizerSet};
33
use crate::spec::{StackProbeType, Target, TargetOptions};
44

@@ -8,7 +8,6 @@ pub fn target() -> Target {
88
base.max_atomic_width = Some(128); // core2 supports cmpxchg16b
99
base.frame_pointer = FramePointer::Always;
1010
base.add_pre_link_args(LinkerFlavor::Darwin(Cc::Yes, Lld::No), &["-m64"]);
11-
base.link_env_remove.to_mut().extend(macos_link_env_remove());
1211
base.stack_probes = StackProbeType::X86;
1312
base.supported_sanitizers =
1413
SanitizerSet::ADDRESS | SanitizerSet::CFI | SanitizerSet::LEAK | SanitizerSet::THREAD;

‎src/test/run-make/macos-deployment-target/Makefile

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

‎src/test/run-make/macos-deployment-target/with_deployment_target.rs

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#![crate_type = "cdylib"]
2+
3+
#[allow(dead_code)]
4+
fn something_and_nothing() {}

0 commit comments

Comments
 (0)