Unrolled build for rust-lang#131016

Rollup merge of rust-lang#131016 - madsmtm:no-sdk-version-in-object, r=jieyouxu

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

This was added in rust-lang#114114, but is unnecessary, since it ends up being overwritten when linking anyhow, and it feels wrong to embed some arbitrary SDK version in here. The object files produced by LLVM also do not set this, and the tooling shows `n/a` when it's `0`, so it seems to genuinely be optional in object files.

I've also added a test for the different places the SDK version shows up, and documented a bit more in the code how SDK versions work.

See rust-lang#129432 for the bigger picture.

Tested with (excludes the same few targets as in rust-lang#130435):
```console
./x test tests/run-make/apple-sdk-version --target aarch64-apple-darwin,aarch64-apple-ios,aarch64-apple-ios-macabi,aarch64-apple-ios-sim,aarch64-apple-tvos,aarch64-apple-tvos-sim,aarch64-apple-visionos,aarch64-apple-visionos-sim,aarch64-apple-watchos,aarch64-apple-watchos-sim,arm64_32-apple-watchos,armv7k-apple-watchos,armv7s-apple-ios,x86_64-apple-darwin,x86_64-apple-ios,x86_64-apple-ios-macabi,x86_64-apple-tvos,x86_64-apple-watchos-sim,x86_64h-apple-darwin
IPHONEOS_DEPLOYMENT_TARGET=10.0 ./x test tests/run-make/apple-sdk-version --target=i386-apple-ios
```

CC `@BlackHoleFox,` you [originally commented on these values](rust-lang#114114 (comment)).

`@rustbot` label O-apple

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