Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always set the deployment target when building std #133092

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 51 additions & 3 deletions src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,55 @@ fn compiler_rt_for_profiler(builder: &Builder<'_>) -> PathBuf {
/// Configure cargo to compile the standard library, adding appropriate env vars
/// and such.
pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, cargo: &mut Cargo) {
if let Some(target) = env::var_os("MACOSX_STD_DEPLOYMENT_TARGET") {
cargo.env("MACOSX_DEPLOYMENT_TARGET", target);
// rustc already ensures that it builds with the minimum deployment
// target, so ideally we shouldn't need to do anything here.
//
// However, `cc` currently defaults to a higher version for backwards
// compatibility, which means that compiler-rt, which is built via
// compiler-builtins' build script, gets built with a higher deployment
// target. This in turn causes warnings while linking, and is generally
// a compatibility hazard.
//
// So, at least until https://github.com/rust-lang/cc-rs/issues/1171, or
// perhaps https://github.com/rust-lang/cargo/issues/13115 is resolved, we
// explicitly set the deployment target environment variables to avoid
// this issue.
//
// This place also serves as an extension point if we ever wanted to raise
// rustc's default deployment target while keeping the prebuilt `std` at
// a lower version, so it's kinda nice to have in any case.
if target.contains("apple") && !builder.config.dry_run() {
// Query rustc for the deployment target.
let mut cmd = command(builder.rustc(cargo.compiler()));
cmd.arg("--target").arg(target.rustc_target_arg());
cmd.arg("--print=deployment-target");
let output = cmd.run_capture_stdout(builder).stdout();

let value = output.split('=').last().unwrap().trim();

// FIXME: Simplify after https://github.com/rust-lang/rust/pull/133041
let env_var = if target.contains("apple-darwin") {
"MACOSX_DEPLOYMENT_TARGET"
} else if target.contains("apple-ios") {
"IPHONEOS_DEPLOYMENT_TARGET"
} else if target.contains("apple-tvos") {
"TVOS_DEPLOYMENT_TARGET"
} else if target.contains("apple-watchos") {
"WATCHOS_DEPLOYMENT_TARGET"
} else if target.contains("apple-visionos") {
"XROS_DEPLOYMENT_TARGET"
} else {
panic!("unknown target OS for apple target");
};

// Unconditionally set the env var (if it was set in the environment
// already, rustc should've picked that up).
cargo.env(env_var, value);

// Allow CI to override the deployment target for `std`.
madsmtm marked this conversation as resolved.
Show resolved Hide resolved
if let Some(target) = env::var_os("MACOSX_STD_DEPLOYMENT_TARGET") {
cargo.env("MACOSX_DEPLOYMENT_TARGET", target);
}
}

// Paths needed by `library/profiler_builtins/build.rs`.
Expand Down Expand Up @@ -1564,7 +1611,8 @@ pub fn compiler_file(
return PathBuf::new();
}
let mut cmd = command(compiler);
cmd.args(builder.cflags(target, GitRepo::Rustc, c));
cmd.args(builder.default_cflags(target, c));
cmd.args(builder.extra_cflags(target, GitRepo::Rustc, c));
cmd.arg(format!("-print-file-name={file}"));
let out = cmd.run_capture_stdout(builder).stdout();
PathBuf::from(out.trim())
Expand Down
48 changes: 8 additions & 40 deletions src/bootstrap/src/core/build_steps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ impl Step for Llvm {
cfg.define("LLVM_VERSION_SUFFIX", suffix);
}

configure_cmake(builder, target, &mut cfg, true, ldflags, &[]);
configure_cmake(builder, target, &mut cfg, true, ldflags);
configure_llvm(builder, target, &mut cfg);

for (key, val) in &builder.config.llvm_build_config {
Expand Down Expand Up @@ -623,7 +623,6 @@ fn configure_cmake(
cfg: &mut cmake::Config,
use_compiler_launcher: bool,
mut ldflags: LdFlags,
suppressed_compiler_flag_prefixes: &[&str],
) {
// Do not print installation messages for up-to-date files.
// LLVM and LLD builds can produce a lot of those and hit CI limits on log size.
Expand Down Expand Up @@ -757,17 +756,8 @@ fn configure_cmake(
}

cfg.build_arg("-j").build_arg(builder.jobs().to_string());
let mut cflags: OsString = builder
.cflags(target, GitRepo::Llvm, CLang::C)
.into_iter()
.filter(|flag| {
!suppressed_compiler_flag_prefixes
.iter()
.any(|suppressed_prefix| flag.starts_with(suppressed_prefix))
})
.collect::<Vec<String>>()
.join(" ")
.into();
let mut cflags: OsString =
builder.extra_cflags(target, GitRepo::Llvm, CLang::C).join(" ").into();
if let Some(ref s) = builder.config.llvm_cflags {
cflags.push(" ");
cflags.push(s);
Expand All @@ -777,17 +767,8 @@ fn configure_cmake(
cflags.push(format!(" --target={target}"));
}
cfg.define("CMAKE_C_FLAGS", cflags);
let mut cxxflags: OsString = builder
.cflags(target, GitRepo::Llvm, CLang::Cxx)
.into_iter()
.filter(|flag| {
!suppressed_compiler_flag_prefixes
.iter()
.any(|suppressed_prefix| flag.starts_with(suppressed_prefix))
})
.collect::<Vec<String>>()
.join(" ")
.into();
let mut cxxflags: OsString =
builder.extra_cflags(target, GitRepo::Llvm, CLang::Cxx).join(" ").into();
if let Some(ref s) = builder.config.llvm_cxxflags {
cxxflags.push(" ");
cxxflags.push(s);
Expand Down Expand Up @@ -950,7 +931,7 @@ impl Step for Enzyme {
// FIXME(ZuseZ4): Find a nicer way to use Enzyme Debug builds
//cfg.profile("Debug");
//cfg.define("CMAKE_BUILD_TYPE", "Debug");
configure_cmake(builder, target, &mut cfg, true, LdFlags::default(), &[]);
configure_cmake(builder, target, &mut cfg, true, LdFlags::default());

// Re-use the same flags as llvm to control the level of debug information
// generated for lld.
Expand Down Expand Up @@ -1065,7 +1046,7 @@ impl Step for Lld {
ldflags.push_all("-Wl,-rpath,'$ORIGIN/../../../'");
}

configure_cmake(builder, target, &mut cfg, true, ldflags, &[]);
configure_cmake(builder, target, &mut cfg, true, ldflags);
configure_llvm(builder, target, &mut cfg);

// Re-use the same flags as llvm to control the level of debug information
Expand Down Expand Up @@ -1164,20 +1145,7 @@ impl Step for Sanitizers {
// Unfortunately sccache currently lacks support to build them successfully.
// Disable compiler launcher on Darwin targets to avoid potential issues.
let use_compiler_launcher = !self.target.contains("apple-darwin");
// Since v1.0.86, the cc crate adds -mmacosx-version-min to the default
// flags on MacOS. A long-standing bug in the CMake rules for compiler-rt
// causes architecture detection to be skipped when this flag is present,
// and compilation fails. https://github.com/llvm/llvm-project/issues/88780
let suppressed_compiler_flag_prefixes: &[&str] =
if self.target.contains("apple-darwin") { &["-mmacosx-version-min="] } else { &[] };
configure_cmake(
builder,
self.target,
&mut cfg,
use_compiler_launcher,
LdFlags::default(),
suppressed_compiler_flag_prefixes,
);
configure_cmake(builder, self.target, &mut cfg, use_compiler_launcher, LdFlags::default());

t!(fs::create_dir_all(&out_dir));
cfg.out_dir(out_dir);
Expand Down
8 changes: 6 additions & 2 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2012,14 +2012,18 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// Only pass correct values for these flags for the `run-make` suite as it
// requires that a C++ compiler was configured which isn't always the case.
if !builder.config.dry_run() && mode == "run-make" {
let mut cflags = builder.default_cflags(target, CLang::C);
cflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::C));
let mut cxxflags = builder.default_cflags(target, CLang::Cxx);
cxxflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx));
cmd.arg("--cc")
.arg(builder.cc(target))
.arg("--cxx")
.arg(builder.cxx(target).unwrap())
.arg("--cflags")
.arg(builder.cflags(target, GitRepo::Rustc, CLang::C).join(" "))
.arg(cflags.join(" "))
.arg("--cxxflags")
.arg(builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "));
.arg(cxxflags.join(" "));
copts_passed = true;
if let Some(ar) = builder.ar(target) {
cmd.arg("--ar").arg(ar);
Expand Down
8 changes: 6 additions & 2 deletions src/bootstrap/src/core/builder/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ impl Cargo {
cargo
}

pub fn compiler(&self) -> Compiler {
self.compiler
}

pub fn into_cmd(self) -> BootstrapCommand {
self.into()
}
Expand Down Expand Up @@ -307,7 +311,7 @@ impl Cargo {
let cc = ccacheify(&builder.cc(target));
self.command.env(format!("CC_{triple_underscored}"), &cc);

let cflags = builder.cflags(target, GitRepo::Rustc, CLang::C).join(" ");
let cflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::C).join(" ");
self.command.env(format!("CFLAGS_{triple_underscored}"), &cflags);

if let Some(ar) = builder.ar(target) {
Expand All @@ -319,7 +323,7 @@ impl Cargo {

if let Ok(cxx) = builder.cxx(target) {
let cxx = ccacheify(&cxx);
let cxxflags = builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" ");
let cxxflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx).join(" ");
self.command
.env(format!("CXX_{triple_underscored}"), &cxx)
.env(format!("CXXFLAGS_{triple_underscored}"), cxxflags);
Expand Down
21 changes: 13 additions & 8 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ impl Mode {
}
}

#[derive(Debug, Hash, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub enum CLang {
C,
Cxx,
Expand Down Expand Up @@ -1187,9 +1188,9 @@ Executed at: {executed_at}"#,
self.cc.borrow()[&target].path().into()
}

/// Returns a list of flags to pass to the C compiler for the target
/// specified.
fn cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec<String> {
/// Returns C flags that `cc-rs` thinks should be enabled for the
/// specified target by default.
fn default_cflags(&self, target: TargetSelection, c: CLang) -> Vec<String> {
if self.config.dry_run() {
return Vec::new();
}
Expand All @@ -1198,14 +1199,18 @@ Executed at: {executed_at}"#,
CLang::Cxx => self.cxx.borrow()[&target].clone(),
};

// Filter out -O and /O (the optimization flags) that we picked up from
// cc-rs because the build scripts will determine that for themselves.
let mut base = base
.args()
// Filter out -O and /O (the optimization flags) that we picked up
// from cc-rs, that's up to the caller to figure out.
base.args()
.iter()
.map(|s| s.to_string_lossy().into_owned())
.filter(|s| !s.starts_with("-O") && !s.starts_with("/O"))
.collect::<Vec<String>>();
.collect::<Vec<String>>()
}

/// Returns extra C flags that `cc-rs` doesn't handle.
fn extra_cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec<String> {
let mut base = Vec::new();

// If we're compiling C++ on macOS then we add a flag indicating that
// we want libc++ (more filled out than libstdc++), ensuring that
Expand Down
6 changes: 4 additions & 2 deletions src/bootstrap/src/utils/cc_detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ pub fn find_target(build: &Build, target: TargetSelection) {
};

build.cc.borrow_mut().insert(target, compiler.clone());
let cflags = build.cflags(target, GitRepo::Rustc, CLang::C);
let mut cflags = build.default_cflags(target, CLang::C);
cflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::C));

// If we use llvm-libunwind, we will need a C++ compiler as well for all targets
// We'll need one anyways if the target triple is also a host triple
Expand All @@ -158,7 +159,8 @@ pub fn find_target(build: &Build, target: TargetSelection) {
build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target)));
build.verbose(|| println!("CFLAGS_{} = {cflags:?}", target.triple));
if let Ok(cxx) = build.cxx(target) {
let cxxflags = build.cflags(target, GitRepo::Rustc, CLang::Cxx);
let mut cxxflags = build.default_cflags(target, CLang::Cxx);
cxxflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::Cxx));
build.verbose(|| println!("CXX_{} = {cxx:?}", target.triple));
build.verbose(|| println!("CXXFLAGS_{} = {cxxflags:?}", target.triple));
}
Expand Down
23 changes: 22 additions & 1 deletion tests/run-make/apple-deployment-target/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@

//@ only-apple

use run_make_support::{apple_os, cmd, run_in_tmpdir, rustc, target};
use std::collections::HashSet;

use run_make_support::{
apple_os, cmd, has_extension, path, regex, run_in_tmpdir, rustc, shallow_find_files, target,
};

/// Run vtool to check the `minos` field in LC_BUILD_VERSION.
///
Expand Down Expand Up @@ -156,4 +160,21 @@ fn main() {
rustc().env_remove(env_var).run();
minos("foo.o", default_version);
});

// Test that all binaries in rlibs produced by `rustc` have the same version.
// Regression test for https://github.com/rust-lang/rust/issues/128419.
let sysroot = rustc().print("sysroot").run().stdout_utf8();
let target_sysroot = path(sysroot.trim()).join("lib/rustlib").join(target()).join("lib");
let rlibs = shallow_find_files(&target_sysroot, |path| has_extension(path, "rlib"));

let output = cmd("otool").arg("-l").args(rlibs).run().stdout_utf8();
let re = regex::Regex::new(r"(minos|version) ([0-9.]*)").unwrap();
let mut versions = HashSet::new();
for (_, [_, version]) in re.captures_iter(&output).map(|c| c.extract()) {
versions.insert(version);
}
// FIXME(madsmtm): See above for aarch64-apple-watchos.
if versions.len() != 1 && target() != "aarch64-apple-watchos" {
panic!("std rlibs contained multiple different deployment target versions: {versions:?}");
}
}
Loading