Auto merge of #111329 - jyn514:metadata-ice, r=bjorn3

Load only the crate header for `locator::crate_matches`

Previously, we used the following info to determine whether to load the crate:
1. The METADATA_HEADER, which includes a METADATA_VERSION constant
2. The embedded rustc version
3. Various metadata in the `CrateRoot`, including the SVH

This worked ok most of the time. Unfortunately, when building locally the rustc version is always
the same because `omit-git-hash` is on by default. That meant that we depended only on 1 and 3, and
we are not very good about bumping METADATA_VERSION (it's currently at 7) so in practice we were
only depending on 3. `CrateRoot` is a very large struct and changes somewhat regularly, so this led
to a steady stream of crashes from trying to load it.

Change the logic to add an intermediate step between 2 and 3: introduce a new `CrateHeader` struct
that contains only the minimum info needed to decide whether the crate should be loaded or not. That
avoids having to load all of `CrateRoot`, which in practice means we should crash much less often.

Note that this works because the SVH should be different between any two dependencies, even if the
compiler has changed, because we use `-Zbinary-dep-depinfo` in bootstrap. See
https://github.com/rust-lang/rust/pull/111329#issuecomment-1538303474 for more details about how the
original crash happened.
This commit is contained in:
bors 2023-05-29 10:40:32 +00:00
commit 99ff5afeb8
6 changed files with 64 additions and 34 deletions

View File

@ -666,31 +666,30 @@ impl<'a> CrateLocator<'a> {
return None;
}
let root = metadata.get_root();
if root.is_proc_macro_crate() != self.is_proc_macro {
let header = metadata.get_header();
if header.is_proc_macro_crate != self.is_proc_macro {
info!(
"Rejecting via proc macro: expected {} got {}",
self.is_proc_macro,
root.is_proc_macro_crate(),
self.is_proc_macro, header.is_proc_macro_crate,
);
return None;
}
if self.exact_paths.is_empty() && self.crate_name != root.name() {
if self.exact_paths.is_empty() && self.crate_name != header.name {
info!("Rejecting via crate name");
return None;
}
if root.triple() != &self.triple {
info!("Rejecting via crate triple: expected {} got {}", self.triple, root.triple());
if header.triple != self.triple {
info!("Rejecting via crate triple: expected {} got {}", self.triple, header.triple);
self.crate_rejections.via_triple.push(CrateMismatch {
path: libpath.to_path_buf(),
got: root.triple().to_string(),
got: header.triple.to_string(),
});
return None;
}
let hash = root.hash();
let hash = header.hash;
if let Some(expected_hash) = self.hash {
if hash != expected_hash {
info!("Rejecting via hash: expected {} got {}", expected_hash, hash);

View File

@ -74,6 +74,7 @@ pub(crate) struct CrateMetadata {
blob: MetadataBlob,
// --- Some data pre-decoded from the metadata blob, usually for performance ---
/// Data about the top-level items in a crate, as well as various crate-level metadata.
root: CrateRoot,
/// Trait impl data.
/// FIXME: Used only from queries and can use query cache,
@ -449,7 +450,7 @@ impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for SyntaxContext {
You need to explicitly pass `(crate_metadata_ref, tcx)` to `decode` instead of just `crate_metadata_ref`.");
};
let cname = cdata.root.name;
let cname = cdata.root.name();
rustc_span::hygiene::decode_syntax_context(decoder, &cdata.hygiene_context, |_, id| {
debug!("SpecializedDecoder<SyntaxContext>: decoding {}", id);
cdata
@ -564,7 +565,7 @@ impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for Span {
let cnum = u32::decode(decoder);
panic!(
"Decoding of crate {:?} tried to access proc-macro dep {:?}",
decoder.cdata().root.name,
decoder.cdata().root.header.name,
cnum
);
}
@ -671,6 +672,16 @@ impl MetadataBlob {
.decode(self)
}
pub(crate) fn get_header(&self) -> CrateHeader {
let slice = &self.blob()[..];
let offset = METADATA_HEADER.len();
let pos_bytes = slice[offset..][..4].try_into().unwrap();
let pos = u32::from_be_bytes(pos_bytes) as usize;
LazyValue::<CrateHeader>::from_position(NonZeroUsize::new(pos).unwrap()).decode(self)
}
pub(crate) fn get_root(&self) -> CrateRoot {
let slice = &self.blob()[..];
let offset = METADATA_HEADER.len();
@ -684,8 +695,8 @@ impl MetadataBlob {
pub(crate) fn list_crate_metadata(&self, out: &mut dyn io::Write) -> io::Result<()> {
let root = self.get_root();
writeln!(out, "Crate info:")?;
writeln!(out, "name {}{}", root.name, root.extra_filename)?;
writeln!(out, "hash {} stable_crate_id {:?}", root.hash, root.stable_crate_id)?;
writeln!(out, "name {}{}", root.name(), root.extra_filename)?;
writeln!(out, "hash {} stable_crate_id {:?}", root.hash(), root.stable_crate_id)?;
writeln!(out, "proc_macro {:?}", root.proc_macro_data.is_some())?;
writeln!(out, "=External Dependencies=")?;
@ -709,21 +720,17 @@ impl CrateRoot {
}
pub(crate) fn name(&self) -> Symbol {
self.name
self.header.name
}
pub(crate) fn hash(&self) -> Svh {
self.hash
self.header.hash
}
pub(crate) fn stable_crate_id(&self) -> StableCrateId {
self.stable_crate_id
}
pub(crate) fn triple(&self) -> &TargetTriple {
&self.triple
}
pub(crate) fn decode_crate_deps<'a>(
&self,
metadata: &'a MetadataBlob,
@ -794,7 +801,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
bug!(
"CrateMetadata::def_kind({:?}): id not found, in crate {:?} with number {}",
item_id,
self.root.name,
self.root.name(),
self.cnum,
)
})
@ -1702,11 +1709,11 @@ impl CrateMetadata {
}
pub(crate) fn name(&self) -> Symbol {
self.root.name
self.root.header.name
}
pub(crate) fn hash(&self) -> Svh {
self.root.hash
self.root.header.hash
}
fn num_def_ids(&self) -> usize {

View File

@ -317,9 +317,9 @@ provide! { tcx, def_id, other, cdata,
}
native_libraries => { cdata.get_native_libraries(tcx.sess).collect() }
foreign_modules => { cdata.get_foreign_modules(tcx.sess).map(|m| (m.def_id, m)).collect() }
crate_hash => { cdata.root.hash }
crate_hash => { cdata.root.header.hash }
crate_host_hash => { cdata.host_hash }
crate_name => { cdata.root.name }
crate_name => { cdata.root.header.name }
extra_filename => { cdata.root.extra_filename.clone() }
@ -581,7 +581,7 @@ impl CrateStore for CStore {
}
fn crate_name(&self, cnum: CrateNum) -> Symbol {
self.get_crate_data(cnum).root.name
self.get_crate_data(cnum).root.header.name
}
fn stable_crate_id(&self, cnum: CrateNum) -> StableCrateId {

View File

@ -662,10 +662,13 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let root = stat!("final", || {
let attrs = tcx.hir().krate_attrs();
self.lazy(CrateRoot {
name: tcx.crate_name(LOCAL_CRATE),
header: CrateHeader {
name: tcx.crate_name(LOCAL_CRATE),
triple: tcx.sess.opts.target_triple.clone(),
hash: tcx.crate_hash(LOCAL_CRATE),
is_proc_macro_crate: proc_macro_data.is_some(),
},
extra_filename: tcx.sess.opts.cg.extra_filename.clone(),
triple: tcx.sess.opts.target_triple.clone(),
hash: tcx.crate_hash(LOCAL_CRATE),
stable_crate_id: tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id(),
required_panic_strategy: tcx.required_panic_strategy(LOCAL_CRATE),
panic_in_drop_strategy: tcx.sess.opts.unstable_opts.panic_in_drop,

View File

@ -56,7 +56,7 @@ pub(crate) fn rustc_version(cfg_version: &'static str) -> String {
/// Metadata encoding version.
/// N.B., increment this if you change the format of metadata such that
/// the rustc version can't be found to compare with `rustc_version()`.
const METADATA_VERSION: u8 = 7;
const METADATA_VERSION: u8 = 8;
/// Metadata header which includes `METADATA_VERSION`.
///
@ -199,7 +199,27 @@ pub(crate) struct ProcMacroData {
macros: LazyArray<DefIndex>,
}
/// Serialized metadata for a crate.
/// Serialized crate metadata.
///
/// This contains just enough information to determine if we should load the `CrateRoot` or not.
/// Prefer [`CrateRoot`] whenever possible to avoid ICEs when using `omit-git-hash` locally.
/// See #76720 for more details.
///
/// If you do modify this struct, also bump the [`METADATA_VERSION`] constant.
#[derive(MetadataEncodable, MetadataDecodable)]
pub(crate) struct CrateHeader {
pub(crate) triple: TargetTriple,
pub(crate) hash: Svh,
pub(crate) name: Symbol,
/// Whether this is the header for a proc-macro crate.
///
/// This is separate from [`ProcMacroData`] to avoid having to update [`METADATA_VERSION`] every
/// time ProcMacroData changes.
pub(crate) is_proc_macro_crate: bool,
}
/// Serialized `.rmeta` data for a crate.
///
/// When compiling a proc-macro crate, we encode many of
/// the `LazyArray<T>` fields as `Lazy::empty()`. This serves two purposes:
///
@ -217,10 +237,10 @@ pub(crate) struct ProcMacroData {
/// to being unused.
#[derive(MetadataEncodable, MetadataDecodable)]
pub(crate) struct CrateRoot {
name: Symbol,
triple: TargetTriple,
/// A header used to detect if this is the right crate to load.
header: CrateHeader,
extra_filename: String,
hash: Svh,
stable_crate_id: StableCrateId,
required_panic_strategy: Option<PanicStrategy>,
panic_in_drop_strategy: PanicStrategy,
@ -465,6 +485,7 @@ trivially_parameterized_over_tcx! {
RawDefId,
TraitImpls,
IncoherentImpls,
CrateHeader,
CrateRoot,
CrateDep,
AttrFlags,

View File

@ -122,7 +122,7 @@ pub fn read_version(dylib_path: &AbsPath) -> io::Result<String> {
// https://github.com/rust-lang/rust/commit/0696e79f2740ad89309269b460579e548a5cd632
let snappy_portion = match version {
5 | 6 => &dot_rustc[8..],
7 => {
7 | 8 => {
let len_bytes = &dot_rustc[8..12];
let data_len = u32::from_be_bytes(len_bytes.try_into().unwrap()) as usize;
&dot_rustc[12..data_len + 12]