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

Fix passing MACOSX_DEPLOYMENT_TARGET to the linker

I messed up in rust-lang#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 rust-lang#104570 (for real this time)

Author: matthiaskrgr

compiler/rustc_target/src/spec/aarch64_apple_darwin.rs

@@ -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

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

@@ -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"]);
+    }
+}

compiler/rustc_target/src/spec/apple_base.rs

@@ -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) {

compiler/rustc_target/src/spec/i686_apple_darwin.rs

@@ -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;
 

compiler/rustc_target/src/spec/x86_64_apple_darwin.rs

@@ -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;

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

@@ -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)

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

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