From 0bebedd7993117c56fb5b7301c39fdecf9798787 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 29 Sep 2024 14:45:08 +0200 Subject: Document a bit more how the SDK version actually works --- compiler/rustc_codegen_ssa/src/back/link.rs | 48 ++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 7 deletions(-) (limited to 'compiler/rustc_codegen_ssa/src') diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 80e8111516e..8a023fac956 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/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.: + // + // + // - By system frameworks to change certain behaviour. For example, the default value of + // `-[NSView wantsBestResolutionOpenGLSurface]` is `YES` when the SDK version is >= 10.15. + // + // + // 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. + // 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 -- cgit 1.4.1-3-g733a5 From 6b06ceb2fd118a7c6ee62675777a1d503348ef0c Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 29 Sep 2024 13:58:49 +0200 Subject: Do not specify an SDK version in object files This is unnecessary, since it ends up being overwritten when linking anyhow, and it feels wrong to embed some arbitrary SDK version in here. --- compiler/rustc_codegen_ssa/src/back/metadata.rs | 10 +++++++--- compiler/rustc_target/src/spec/base/apple/mod.rs | 17 ----------------- compiler/rustc_target/src/spec/mod.rs | 2 +- tests/run-make/apple-sdk-version/rmake.rs | 9 +++++++++ 4 files changed, 17 insertions(+), 21 deletions(-) (limited to 'compiler/rustc_codegen_ssa/src') diff --git a/compiler/rustc_codegen_ssa/src/back/metadata.rs b/compiler/rustc_codegen_ssa/src/back/metadata.rs index 06433484ea3..8857fda1e97 100644 --- a/compiler/rustc_codegen_ssa/src/back/metadata.rs +++ b/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 } diff --git a/compiler/rustc_target/src/spec/base/apple/mod.rs b/compiler/rustc_target/src/spec/base/apple/mod.rs index 81b5a936d35..73763cf034c 100644 --- a/compiler/rustc_target/src/spec/base/apple/mod.rs +++ b/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 { Some(match (&*target.os, &*target.abi) { ("macos", _) => object::macho::PLATFORM_MACOS, diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index f327c1fd179..70ef21120e5 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/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; diff --git a/tests/run-make/apple-sdk-version/rmake.rs b/tests/run-make/apple-sdk-version/rmake.rs index a16c84ddc59..6463ec00403 100644 --- a/tests/run-make/apple-sdk-version/rmake.rs +++ b/tests/run-make/apple-sdk-version/rmake.rs @@ -50,6 +50,15 @@ fn main() { // 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, -- cgit 1.4.1-3-g733a5