Apple: Do not specify an SDK version in rlib object files

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -2959,11 +2959,12 @@ pub(crate) fn are_upstream_rust_objects_already_included(sess: &Session) -> bool
     }
 }
 
-/// We need to communicate four things to the linker on Apple/Darwin targets:
+/// We need to communicate five things to the linker on Apple/Darwin targets:
 /// - The architecture.
 /// - The operating system (and that it's an Apple platform).
-/// - The deployment target.
 /// - The environment / ABI.
+/// - The deployment target.
+/// - The SDK version.
 fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
     if !sess.target.is_like_osx {
         return;
@@ -3039,7 +3040,38 @@ fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavo
         let (major, minor, patch) = current_apple_deployment_target(&sess.target);
         let min_version = format!("{major}.{minor}.{patch}");
 
-        // Lie about the SDK version, we don't know it here
+        // The SDK version is used at runtime when compiling with a newer SDK / version of Xcode:
+        // - By dyld to give extra warnings and errors, see e.g.:
+        //   <https://github.com/apple-oss-distributions/dyld/blob/dyld-1165.3/common/MachOFile.cpp#L3029>
+        //   <https://github.com/apple-oss-distributions/dyld/blob/dyld-1165.3/common/MachOFile.cpp#L3738-L3857>
+        // - By system frameworks to change certain behaviour. For example, the default value of
+        //   `-[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.
+        //
+        // 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.
+        //
+        // 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.
+        //
+        // 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.
+        //
+        // Currently, we lie about the SDK version, and choose the second option.
+        //
+        // FIXME(madsmtm): Parse the SDK version from the SDK root instead.
+        // <https://github.com/rust-lang/rust/issues/129432>
         let sdk_version = &*min_version;
 
         // From the man page for ld64 (`man ld`):
@@ -3053,11 +3085,13 @@ fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavo
         cmd.link_args(&["-platform_version", platform_name, &*min_version, sdk_version]);
     } else {
         // cc == Cc::Yes
+        //
         // We'd _like_ to use `-target` everywhere, since that can uniquely
-        // communicate all the required details, but that doesn't work on GCC,
-        // and since we don't know whether the `cc` compiler is Clang, GCC, or
-        // something else, we fall back to other options that also work on GCC
-        // when compiling for macOS.
+        // communicate all the required details except for the SDK version
+        // (which is read by Clang itself from the SDKROOT), but that doesn't
+        // work on GCC, and since we don't know whether the `cc` compiler is
+        // Clang, GCC, or something else, we fall back to other options that
+        // also work on GCC when compiling for macOS.
         //
         // Targets other than macOS are ill-supported by GCC (it doesn't even
         // support e.g. `-miphoneos-version-min`), so in those cases we can

compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -402,13 +402,17 @@ fn macho_object_build_version_for_target(target: &Target) -> object::write::Mach
     let platform =
         rustc_target::spec::current_apple_platform(target).expect("unknown Apple target OS");
     let min_os = rustc_target::spec::current_apple_deployment_target(target);
-    let (sdk_major, sdk_minor) =
-        rustc_target::spec::current_apple_sdk_version(platform).expect("unknown Apple target OS");
 
     let mut build_version = object::write::MachOBuildVersion::default();
     build_version.platform = platform;
     build_version.minos = pack_version(min_os);
-    build_version.sdk = pack_version((sdk_major, sdk_minor, 0));
+    // The version here does not _really_ matter, since it is only used at runtime, and we specify
+    // it when linking the final binary, so we will omit the version. This is also what LLVM does,
+    // and the tooling also allows this (and shows the SDK version as `n/a`). Finally, it is the
+    // semantically correct choice, as the SDK has not influenced the binary generated by rustc at
+    // this point in time.
+    build_version.sdk = 0;
+
     build_version
 }
 

compiler/rustc_target/src/spec/base/apple/mod.rs

@@ -158,23 +158,6 @@ pub(crate) fn base(
     (opts, llvm_target(os, arch, abi), arch.target_arch())
 }
 
-pub fn sdk_version(platform: u32) -> Option<(u16, u8)> {
-    // NOTE: These values are from an arbitrary point in time but shouldn't make it into the final
-    // binary since the final link command will have the current SDK version passed to it.
-    match platform {
-        object::macho::PLATFORM_MACOS => Some((13, 1)),
-        object::macho::PLATFORM_IOS
-        | object::macho::PLATFORM_IOSSIMULATOR
-        | object::macho::PLATFORM_TVOS
-        | object::macho::PLATFORM_TVOSSIMULATOR
-        | object::macho::PLATFORM_MACCATALYST => Some((16, 2)),
-        object::macho::PLATFORM_WATCHOS | object::macho::PLATFORM_WATCHOSSIMULATOR => Some((9, 1)),
-        // FIXME: Upgrade to `object-rs` 0.33+ implementation with visionOS platform definition
-        11 | 12 => Some((1, 0)),
-        _ => None,
-    }
-}
-
 pub fn platform(target: &Target) -> Option<u32> {
     Some(match (&*target.os, &*target.abi) {
         ("macos", _) => object::macho::PLATFORM_MACOS,

compiler/rustc_target/src/spec/mod.rs

@@ -61,7 +61,7 @@ pub mod crt_objects;
 mod base;
 pub use base::apple::{
     deployment_target_for_target as current_apple_deployment_target,
-    platform as current_apple_platform, sdk_version as current_apple_sdk_version,
+    platform as current_apple_platform,
 };
 pub use base::avr_gnu::ef_avr_arch;
 

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

@@ -0,0 +1 @@
+fn main() {}

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

@@ -0,0 +1,95 @@
+//! Test codegen when setting SDK version on Apple platforms.
+//!
+//! This is important since its a compatibility hazard. The linker will
+//! generate load commands differently based on what minimum OS it can assume.
+//!
+//! See https://github.com/rust-lang/rust/issues/129432.
+
+//@ only-apple
+
+use run_make_support::{apple_os, cmd, run_in_tmpdir, rustc, target};
+
+/// Run vtool to check the `sdk` field in LC_BUILD_VERSION.
+///
+/// On lower deployment targets, LC_VERSION_MIN_MACOSX, LC_VERSION_MIN_IPHONEOS and similar
+/// are used instead of LC_BUILD_VERSION, but both name the relevant variable `sdk`.
+#[track_caller]
+fn has_sdk_version(file: &str, version: &str) {
+    cmd("vtool")
+        .arg("-show-build")
+        .arg(file)
+        .run()
+        .assert_stdout_contains(format!("sdk {version}"));
+}
+
+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
+    // and the iPhone Simulator version is the same as for the iPhone SDK.
+    let sdk_name = match apple_os() {
+        "macos" => "macosx",
+        "ios" => "iphoneos",
+        "watchos" => "watchos",
+        "tvos" => "appletvos",
+        "visionos" => "xros",
+        _ => unreachable!(),
+    };
+    let current_sdk_version =
+        cmd("xcrun").arg("--show-sdk-version").arg("--sdk").arg(sdk_name).run().stdout_utf8();
+    let current_sdk_version = current_sdk_version.trim();
+
+    // Check the SDK version in the object file produced by the codegen backend.
+    rustc().target(target()).crate_type("lib").emit("obj").input("foo.rs").output("foo.o").run();
+    // Set to 0, which means not set or "n/a".
+    has_sdk_version("foo.o", "n/a");
+
+    // Check the SDK version in the .rmeta file, as set in `create_object_file`.
+    //
+    // This is just to ensure that we don't set some odd version in `create_object_file`,
+    // if the rmeta file is packed in a different way in the future, this can safely be removed.
+    rustc().target(target()).crate_type("rlib").input("foo.rs").output("libfoo.rlib").run();
+    // Extra .rmeta file (which is encoded as an object file).
+    cmd("ar").arg("-x").arg("libfoo.rlib").arg("lib.rmeta").run();
+    has_sdk_version("lib.rmeta", "n/a");
+
+    // Test that version makes it to the linker.
+    for (crate_type, file_ext) in [("bin", ""), ("dylib", ".dylib")] {
+        // Non-simulator watchOS targets don't support dynamic linking,
+        // for simplicity we disable the test on all watchOS targets.
+        if crate_type == "dylib" && apple_os() == "watchos" {
+            continue;
+        }
+
+        // Test with clang
+        let file_name = format!("foo_cc{file_ext}");
+        rustc()
+            .target(target())
+            .crate_type("bin")
+            .arg("-Clinker-flavor=gcc")
+            .input("foo.rs")
+            .output(&file_name)
+            .run();
+        has_sdk_version(&file_name, current_sdk_version);
+
+        // Test with ld64
+        let file_name = format!("foo_ld{file_ext}");
+        rustc()
+            .target(target())
+            .crate_type("bin")
+            .arg("-Clinker-flavor=ld")
+            .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);
+    }
+}