Pass SDK version to ld64 when linking

This is an important property of the binary that we were previously
setting incorrectly, and is a big step towards making binaries linked
with cc and ld64 equal.

Author: madsmtm

compiler/rustc_codegen_ssa/messages.ftl

@@ -45,6 +45,11 @@ codegen_ssa_apple_sdk_error_missing_developer_dir =
     - `{$sdkroot}`
     - `{$sdkroot_bare}`
 
+codegen_ssa_apple_sdk_error_missing_mac_catalyst_version =
+    failed to find {$version} in SDK version map key `macOS_iOSMac`.
+
+    This is probably a bug in your SDK.
+
 codegen_ssa_apple_sdk_error_missing_sdk_settings =
     failed finding `SDKSettings.json` or `SDKSettings.plist` in `{$sdkroot}`.
 
@@ -62,6 +67,9 @@ codegen_ssa_apple_sdk_error_missing_xcode_select =
 
     Consider using `sudo xcode-select --switch path/to/Xcode.app` or `sudo xcode-select --reset` to select a valid path.
 
+codegen_ssa_apple_sdk_error_must_have_when_using_ld64 =
+    must know the SDK when linking with ld64
+
 codegen_ssa_apple_sdk_error_not_sdk_path =
     failed parsing SDK at `{$path}`.
 

compiler/rustc_codegen_ssa/src/apple.rs

@@ -281,7 +281,6 @@ pub(crate) struct SDKSettings {
     canonical_name: String,
 
     #[serde(rename = "Version")]
-    #[allow(dead_code)]
     version: AppleOSVersion,
 
     #[serde(rename = "MaximumDeploymentTarget")]
@@ -293,6 +292,13 @@ pub(crate) struct SDKSettings {
     /// Optional to support `SDKSettings.json` converted from an older `SDKSettings.plist`.
     #[serde(rename = "SupportedTargets")]
     supported_targets: Option<BTreeMap<String, SupportedTargets>>,
+
+    /// Optional to support `SDKSettings.json` converted from an older `SDKSettings.plist`.
+    ///
+    /// Could in general be useful in the future for building "zippered" binaries, see:
+    /// <https://github.com/rust-lang/rust/issues/131216>
+    #[serde(rename = "VersionMap")]
+    version_map: Option<BTreeMap<String, BTreeMap<AppleOSVersion, AppleOSVersion>>>,
 }
 
 impl SDKSettings {
@@ -326,6 +332,7 @@ impl SDKSettings {
             },
             default_properties: DefaultProperties::default(),
             supported_targets: None,
+            version_map: None,
         })
     }
 
@@ -371,6 +378,35 @@ impl SDKSettings {
             .unwrap_or(Path::new("/System/iOSSupport"))
     }
 
+    /// The version of the SDK.
+    ///
+    /// This is needed by the linker.
+    ///
+    /// settings["Version"] or settings["VersionMap"]["macOS_iOSMac"][settings["Version"]].
+    pub(crate) fn sdk_version(&self, target: &Target, sdkroot: &Path) -> Result<AppleOSVersion, AppleSdkError> {
+        if target.abi == "macabi" {
+            let map = self
+                .version_map
+                .as_ref()
+                .ok_or(AppleSdkError::SdkDoesNotSupportOS {
+                    sdkroot: sdkroot.to_owned(),
+                    os: target.os.clone(),
+                    abi: target.abi.clone(),
+                })?
+                .get("macOS_iOSMac")
+                .ok_or(AppleSdkError::SdkDoesNotSupportOS {
+                    sdkroot: sdkroot.to_owned(),
+                    os: target.os.clone(),
+                    abi: target.abi.clone(),
+                })?;
+            map.get(&self.version).cloned().ok_or(AppleSdkError::MissingMacCatalystVersion {
+                version: self.version.pretty().to_string(),
+            })
+        } else {
+            Ok(self.version)
+        }
+    }
+
     fn supports_target(&self, target: &Target, sdkroot: &Path) -> Result<(), AppleSdkError> {
         let arch = ld64_arch(target);
 

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -2410,10 +2410,10 @@ fn add_order_independent_options(
     // Take care of the flavors and CLI options requesting the `lld` linker.
     add_lld_args(cmd, sess, flavor, self_contained_components);
 
-    add_apple_link_args(cmd, sess, flavor);
-
     let apple_sdk_data = add_apple_sdk(cmd, sess, crate_type, flavor);
 
+    add_apple_link_args(cmd, sess, flavor, &apple_sdk_data);
+
     add_link_script(cmd, sess, tmpdir, crate_type);
 
     if sess.target.os == "fuchsia"
@@ -2970,7 +2970,12 @@ pub(crate) fn are_upstream_rust_objects_already_included(sess: &Session) -> bool
 /// - The environment / ABI.
 /// - The deployment target.
 /// - The SDK version.
-fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
+fn add_apple_link_args(
+    cmd: &mut dyn Linker,
+    sess: &Session,
+    flavor: LinkerFlavor,
+    settings: &Option<(PathBuf, SDKSettings)>,
+) {
     if !sess.target.is_like_osx {
         return;
     }
@@ -3030,31 +3035,41 @@ fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavo
         //   `-[NSView wantsBestResolutionOpenGLSurface]` is `YES` when the SDK version is >= 10.15.
         //   <https://developer.apple.com/documentation/appkit/nsview/1414938-wantsbestresolutionopenglsurface?language=objc>
         //
-        // We do not currently know the actual SDK version though, so we have a few options:
-        // 1. Use the minimum version supported by rustc.
-        // 2. Use the same as the deployment target.
-        // 3. Use an arbitary recent version.
-        // 4. Omit the version.
+        // So it is important that we pass the correct version here.
         //
-        // The first option is too low / too conservative, and means that users will not get the
-        // same behaviour from a binary compiled with rustc as with one compiled by clang.
         //
-        // The second option is similarly conservative, and also wrong since if the user specified a
-        // higher deployment target than the SDK they're compiling/linking with, the runtime might
-        // make invalid assumptions about the capabilities of the binary.
+        // For posterity, insufficient alternatives are listed below:
         //
-        // The third option requires that `rustc` is periodically kept up to date with Apple's SDK
-        // version, and is also wrong for similar reasons as above.
+        // 1. Use the minimum version supported by rustc.
+        //    Too low / too conservative, and means that users will not get the same behaviour from
+        //    a binary compiled with rustc as with one compiled by clang.
         //
-        // The fourth option is bad because while `ld`, `otool`, `vtool` and such understand it to
-        // mean "absent" or `n/a`, dyld doesn't actually understand it, and will end up interpreting
-        // it as 0.0, which is again too low/conservative.
+        // 2. Use the same as the deployment target.
+        //    Similarly conservative, and also wrong since if the user specified a higher deployment
+        //    target than the SDK they're compiling/linking with, the runtime might make invalid
+        //    assumptions about the capabilities of the binary.
         //
-        // Currently, we lie about the SDK version, and choose the second option.
+        // 3. Use an arbitary recent version.
+        //    Requires that `rustc` is periodically kept up to date with Apple's SDK version, and is
+        //    also wrong for similar reasons as above.
         //
-        // FIXME(madsmtm): Parse the SDK version from the SDK root instead.
-        // <https://github.com/rust-lang/rust/issues/129432>
-        let sdk_version = &*min_version;
+        // 4. Omit the version.
+        //    Bad because while `ld`, `otool`, `vtool` and such understand it to mean "absent" or
+        //    `n/a`, dyld doesn't actually understand it, and will end up interpreting it as 0.0,
+        //    which is again too low/conservative.
+        let AppleOSVersion { major, minor, patch } = if let Some((sdkroot, settings)) = settings {
+            settings.sdk_version(&sess.target, &sdkroot).unwrap_or_else(|err| {
+                sess.dcx().emit_err(err);
+                AppleOSVersion::MAX
+            })
+        } else {
+            // If the SDK wasn't read properly, we may have errored already, but we may also only
+            // have given a warning to support `zig cc` on non-macOS hosts. `ld64` requires the SDK
+            // version though, so we must actually error here.
+            sess.dcx().emit_err(errors::AppleSdkError::MustHaveWhenUsingLd64);
+            AppleOSVersion::MAX
+        };
+        let sdk_version = format!("{major}.{minor}.{patch}");
 
         // From the man page for ld64 (`man ld`):
         // > This is set to indicate the platform, oldest supported version of
@@ -3064,7 +3079,7 @@ fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavo
         // Like with `-arch`, the linker can figure out the platform versions
         // itself from the binaries being linked, but to be safe, we specify
         // the desired versions here explicitly.
-        cmd.link_args(&["-platform_version", platform_name, &*min_version, sdk_version]);
+        cmd.link_args(&["-platform_version", platform_name, &*min_version, &*sdk_version]);
     } else {
         // cc == Cc::Yes
         //

compiler/rustc_codegen_ssa/src/errors.rs

@@ -565,6 +565,9 @@ pub(crate) enum AppleSdkError {
     #[diag(codegen_ssa_apple_sdk_error_missing_developer_dir)]
     MissingDeveloperDir { dir: PathBuf, sdkroot: PathBuf, sdkroot_bare: PathBuf },
 
+    #[diag(codegen_ssa_apple_sdk_error_missing_mac_catalyst_version)]
+    MissingMacCatalystVersion { version: String },
+
     #[diag(codegen_ssa_apple_sdk_error_missing_sdk_settings)]
     MissingSDKSettings { sdkroot: PathBuf },
 
@@ -574,6 +577,9 @@ pub(crate) enum AppleSdkError {
     #[diag(codegen_ssa_apple_sdk_error_missing_xcode_select)]
     MissingXcodeSelect { dir: PathBuf, sdkroot: PathBuf, sdkroot_bare: PathBuf },
 
+    #[diag(codegen_ssa_apple_sdk_error_must_have_when_using_ld64)]
+    MustHaveWhenUsingLd64,
+
     #[diag(codegen_ssa_apple_sdk_error_not_sdk_path)]
     NotSdkPath { sdkroot: PathBuf },
 

tests/run-make/apple-sdk-version/rmake.rs

@@ -23,12 +23,6 @@ fn has_sdk_version(file: &str, version: &str) {
 }
 
 fn main() {
-    // Fetch rustc's inferred deployment target.
-    let current_deployment_target =
-        rustc().target(target()).print("deployment-target").run().stdout_utf8();
-    let current_deployment_target =
-        current_deployment_target.strip_prefix("deployment_target=").unwrap().trim();
-
     // Fetch current SDK version via. xcrun.
     //
     // Assumes a standard Xcode distribution, where e.g. the macOS SDK's Mac Catalyst
@@ -87,9 +81,6 @@ fn main() {
             .input("foo.rs")
             .output(&file_name)
             .run();
-        // FIXME(madsmtm): This uses the current deployment target
-        // instead of the current SDK version like Clang does.
-        // https://github.com/rust-lang/rust/issues/129432
-        has_sdk_version(&file_name, current_deployment_target);
+        has_sdk_version(&file_name, current_sdk_version);
     }
 }