Auto merge of #129369 - madsmtm:apple-cc-linker-pass-target, r=jieyouxu

bors · bors · commit 7c7372b6a1b9 · 2024-09-12T06:57:38.000Z
Pass deployment target when linking with CC on Apple targets This PR effectively implements what's also being considered in the `cc` crate [here](rust-lang/cc-rs#1030 (comment)), that is: - When linking macOS targets with CC, pass the `-mmacosx-version-min=.` option to specify the desired deployment target. Also, no longer pass `-m32`/`-m64`, these are redundant since we already pass `-arch`. - When linking with CC on iOS, tvOS, watchOS and visionOS, only pass `-target` (we assume for these targets that CC forwards to Clang). This is required to get the linker to emit the correct `LC_BUILD_VERSION` of the final binary. See #129432 for more motivation behind this change. r? compiler CC `@BlackHoleFox`
diff --git a/compiler/rustc_target/src/spec/base/apple/mod.rs b/compiler/rustc_target/src/spec/base/apple/mod.rs
@@ -168,14 +168,42 @@ fn pre_link_args(os: &'static str, arch: Arch, abi: TargetAbi) -> LinkArgs {
         ["-platform_version".into(), platform_name, min_version, sdk_version].into_iter(),
     );
 
-    if abi != TargetAbi::MacCatalyst {
+    // We need to communicate four things to the C compiler to be able to link:
+    // - The architecture.
+    // - The operating system (and that it's an Apple platform).
+    // - The deployment target.
+    // - The environment / ABI.
+    //
+    // We'd like to use `-target` everywhere, since that can uniquely
+    // communicate all of these, 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 fairly
+    // safely use `-target`. See also the following, where it is made explicit
+    // that the recommendation by LLVM developers is to use `-target`:
+    // <https://github.com/llvm/llvm-project/issues/88271>
+    if os == "macos" {
+        // `-arch` communicates the architecture.
+        //
         // CC forwards the `-arch` to the linker, so we use the same value
         // here intentionally.
         add_link_args(
             &mut args,
             LinkerFlavor::Darwin(Cc::Yes, Lld::No),
             &["-arch", arch.ld_arch()],
         );
+        // The presence of `-mmacosx-version-min` makes CC default to macOS,
+        // and it sets the deployment target.
+        let (major, minor, patch) = deployment_target(os, arch, abi);
+        let opt = format!("-mmacosx-version-min={major}.{minor}.{patch}").into();
+        add_link_args_iter(&mut args, LinkerFlavor::Darwin(Cc::Yes, Lld::No), [opt].into_iter());
+        // macOS has no environment, so with these two, we've told CC all the
+        // desired parameters.
+        //
+        // We avoid `-m32`/`-m64`, as this is already encoded by `-arch`.
     } else {
         add_link_args_iter(
             &mut args,
diff --git a/compiler/rustc_target/src/spec/targets/i686_apple_darwin.rs b/compiler/rustc_target/src/spec/targets/i686_apple_darwin.rs
@@ -1,10 +1,8 @@
 use crate::spec::base::apple::{base, Arch, TargetAbi};
-use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, Target, TargetOptions};
+use crate::spec::{FramePointer, Target, TargetOptions};
 
 pub(crate) fn target() -> Target {
-    let (mut opts, llvm_target, arch) = base("macos", Arch::I686, TargetAbi::Normal);
-    opts.add_pre_link_args(LinkerFlavor::Darwin(Cc::Yes, Lld::No), &["-m32"]);
-
+    let (opts, llvm_target, arch) = base("macos", Arch::I686, TargetAbi::Normal);
     Target {
         llvm_target,
         metadata: crate::spec::TargetMetadata {
diff --git a/compiler/rustc_target/src/spec/targets/x86_64_apple_darwin.rs b/compiler/rustc_target/src/spec/targets/x86_64_apple_darwin.rs
@@ -1,9 +1,8 @@
 use crate::spec::base::apple::{base, Arch, TargetAbi};
-use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, SanitizerSet, Target, TargetOptions};
+use crate::spec::{FramePointer, SanitizerSet, Target, TargetOptions};
 
 pub(crate) fn target() -> Target {
-    let (mut opts, llvm_target, arch) = base("macos", Arch::X86_64, TargetAbi::Normal);
-    opts.add_pre_link_args(LinkerFlavor::Darwin(Cc::Yes, Lld::No), &["-m64"]);
+    let (opts, llvm_target, arch) = base("macos", Arch::X86_64, TargetAbi::Normal);
     Target {
         llvm_target,
         metadata: crate::spec::TargetMetadata {
diff --git a/compiler/rustc_target/src/spec/targets/x86_64h_apple_darwin.rs b/compiler/rustc_target/src/spec/targets/x86_64h_apple_darwin.rs
@@ -1,11 +1,10 @@
 use crate::spec::base::apple::{base, Arch, TargetAbi};
-use crate::spec::{Cc, FramePointer, LinkerFlavor, Lld, SanitizerSet, Target, TargetOptions};
+use crate::spec::{FramePointer, SanitizerSet, Target, TargetOptions};
 
 pub(crate) fn target() -> Target {
     let (mut opts, llvm_target, arch) = base("macos", Arch::X86_64h, TargetAbi::Normal);
     opts.max_atomic_width = Some(128);
     opts.frame_pointer = FramePointer::Always;
-    opts.add_pre_link_args(LinkerFlavor::Darwin(Cc::Yes, Lld::No), &["-m64"]);
     opts.supported_sanitizers =
         SanitizerSet::ADDRESS | SanitizerSet::CFI | SanitizerSet::LEAK | SanitizerSet::THREAD;
 
diff --git a/tests/run-make/apple-deployment-target/rmake.rs b/tests/run-make/apple-deployment-target/rmake.rs
@@ -81,9 +81,8 @@ fn main() {
         rustc().env(env_var, example_version).run();
         minos("libfoo.dylib", example_version);
 
-        // FIXME(madsmtm): Deployment target is not currently passed properly to linker
-        // rustc().env_remove(env_var).run();
-        // minos("libfoo.dylib", default_version);
+        rustc().env_remove(env_var).run();
+        minos("libfoo.dylib", default_version);
 
         // Test with ld64 instead
 
@@ -110,9 +109,8 @@ fn main() {
             rustc().env(env_var, example_version).run();
             minos("foo", example_version);
 
-            // FIXME(madsmtm): Deployment target is not currently passed properly to linker
-            // rustc().env_remove(env_var).run();
-            // minos("foo", default_version);
+            rustc().env_remove(env_var).run();
+            minos("foo", default_version);
         }
 
         // Test with ld64 instead
