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.

Sign up for GitHub

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

Jump to bottom

Update cc crate for bootstrap to v1.0.97 #122504

Merged
merged 1 commit into from
May 7, 2024
Merged

Update cc crate for bootstrap to v1.0.97 #122504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/bootstrap/Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ dependencies = [

[[package]]
name = "cc"
version = "1.0.73"
version = "1.0.97"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2fff2a6927b3bb87f9595d67196a70493f627687a71d87a0d692242c33f58c11"
checksum = "099a5357d84c4c61eb35fc8eafa9a79a902c2f76911e5747ced4e032edd8d9b4"

[[package]]
name = "cfg-if"
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ test = false
# Most of the time updating these dependencies requires modifications to the
# bootstrap codebase(e.g., https://github.com/rust-lang/rust/issues/124565);
# otherwise, some targets will fail. That's why these dependencies are explicitly pinned.
cc = "=1.0.73"
cc = "=1.0.97"
cmake = "=0.1.48"

build_helper = { path = "../tools/build_helper" }
Expand Down
59 changes: 47 additions & 12 deletions src/bootstrap/src/core/build_steps/llvm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use std::sync::OnceLock;
use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
use crate::core::config::{Config, TargetSelection};
use crate::utils::channel;
use crate::utils::helpers::{self, exe, get_clang_cl_resource_dir, output, t, up_to_date};
use crate::utils::helpers::{
self, exe, get_clang_cl_resource_dir, output, t, unhashed_basename, up_to_date,
};
use crate::{generate_smart_stamp_hash, CLang, GitRepo, Kind};

use build_helper::ci::CiEnv;
Expand Down Expand Up @@ -506,7 +508,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 @@ -596,6 +598,7 @@ fn configure_cmake(
use_compiler_launcher: bool,
mut ldflags: LdFlags,
extra_compiler_flags: &[&str],
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 @@ -729,7 +732,17 @@ fn configure_cmake(
}

cfg.build_arg("-j").build_arg(builder.jobs().to_string());
let mut cflags: OsString = builder.cflags(target, GitRepo::Llvm, CLang::C).join(" ").into();
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();
if let Some(ref s) = builder.config.llvm_cflags {
cflags.push(" ");
cflags.push(s);
Expand All @@ -742,7 +755,17 @@ fn configure_cmake(
cflags.push(&format!(" {flag}"));
}
cfg.define("CMAKE_C_FLAGS", cflags);
let mut cxxflags: OsString = builder.cflags(target, GitRepo::Llvm, CLang::Cxx).join(" ").into();
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();
if let Some(ref s) = builder.config.llvm_cxxflags {
cxxflags.push(" ");
cxxflags.push(s);
Expand Down Expand Up @@ -921,7 +944,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 @@ -1022,13 +1045,20 @@ impl Step for Sanitizers {
let use_compiler_launcher = !self.target.contains("apple-darwin");
let extra_compiler_flags: &[&str] =
if self.target.contains("apple") { &["-fembed-bitcode=off"] } else { &[] };
// 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(),
extra_compiler_flags,
suppressed_compiler_flag_prefixes,
);

t!(fs::create_dir_all(&out_dir));
Expand Down Expand Up @@ -1190,7 +1220,7 @@ impl Step for CrtBeginEnd {

let crtbegin_src = builder.src.join("src/llvm-project/compiler-rt/lib/builtins/crtbegin.c");
let crtend_src = builder.src.join("src/llvm-project/compiler-rt/lib/builtins/crtend.c");
if up_to_date(&crtbegin_src, &out_dir.join("crtbegin.o"))
if up_to_date(&crtbegin_src, &out_dir.join("crtbeginS.o"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, we were also copying "crtbeginS". I wonder how it was working before, or wasn't working at all? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were copying both crtbegin.o and crtend.o. But for some reason, we were checking crtbegin.o (without S) and crtendS.o (with S). So, it was working fine before, but it seemed odd, and I wondered if there was a good reason for it or just a careless inconsistency.

&& up_to_date(&crtend_src, &out_dir.join("crtendS.o"))
{
return out_dir;
Expand Down Expand Up @@ -1222,10 +1252,15 @@ impl Step for CrtBeginEnd {
.define("CRT_HAS_INITFINI_ARRAY", None)
.define("EH_USE_FRAME_REGISTRY", None);

cfg.compile("crt");
let objs = cfg.compile_intermediates();
assert_eq!(objs.len(), 2);
for obj in objs {
let base_name = unhashed_basename(&obj);
assert!(base_name == "crtbegin" || base_name == "crtend");
t!(fs::copy(&obj, out_dir.join(format!("{}S.o", base_name))));
t!(fs::rename(&obj, out_dir.join(format!("{}.o", base_name))));
}

t!(fs::copy(out_dir.join("crtbegin.o"), out_dir.join("crtbeginS.o")));
t!(fs::copy(out_dir.join("crtend.o"), out_dir.join("crtendS.o")));
out_dir
}
}
Expand Down Expand Up @@ -1372,9 +1407,9 @@ impl Step for Libunwind {
for entry in fs::read_dir(&out_dir).unwrap() {
let file = entry.unwrap().path().canonicalize().unwrap();
if file.is_file() && file.extension() == Some(OsStr::new("o")) {
// file name starts with "Unwind-EHABI", "Unwind-seh" or "libunwind"
let file_name = file.file_name().unwrap().to_str().expect("UTF-8 file name");
if cpp_sources.iter().any(|f| file_name.starts_with(&f[..f.len() - 4])) {
// Object file name without the hash prefix is "Unwind-EHABI", "Unwind-seh" or "libunwind".
let base_name = unhashed_basename(&file);
if cpp_sources.iter().any(|f| *base_name == f[..f.len() - 4]) {
cc_cfg.object(&file);
count += 1;
}
Expand Down
Loading
Loading