Skip to content

Commit

Permalink
Auto merge of rust-lang#129369 - madsmtm:apple-cc-linker-pass-target,…
Browse files Browse the repository at this point in the history
… r=jieyouxu

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 rust-lang#129432 for more motivation behind this change.

r? compiler

CC `@BlackHoleFox`
  • Loading branch information
bors committed Sep 12, 2024
2 parents 1f51450 + dd35398 commit 7c7372b
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 16 deletions.
30 changes: 29 additions & 1 deletion compiler/rustc_target/src/spec/base/apple/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_target/src/spec/targets/i686_apple_darwin.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_target/src/spec/targets/x86_64_apple_darwin.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
10 changes: 4 additions & 6 deletions tests/run-make/apple-deployment-target/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down

0 comments on commit 7c7372b

Please sign in to comment.