Skip to content

Commit

Permalink
Auto merge of #129369 - madsmtm:apple-cc-linker-pass-target, r=jieyouxu
Browse files Browse the repository at this point in the history
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`
  • 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.