From 62314be705aee343870f3bc36cb5eb7e89ef6269 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 21 May 2023 09:40:00 -0500 Subject: [PATCH 1/2] Load only the header for 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. --- compiler/rustc_metadata/src/locator.rs | 17 +++++----- compiler/rustc_metadata/src/rmeta/decoder.rs | 33 +++++++++++-------- .../src/rmeta/decoder/cstore_impl.rs | 6 ++-- compiler/rustc_metadata/src/rmeta/encoder.rs | 9 +++-- compiler/rustc_metadata/src/rmeta/mod.rs | 31 ++++++++++++++--- 5 files changed, 63 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index ceb348f34690e..a89d7b464e2e9 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -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); diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index cc4e60cf6ac58..982a33d6d030a 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -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> 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: decoding {}", id); cdata @@ -564,7 +565,7 @@ impl<'a, 'tcx> Decodable> 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::::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 { diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 7425963d30ff8..3d8991d99b529 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -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 { diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index f067bca4b0b39..8d67ba1a129a4 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -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, diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 97e67fcf8fdd0..ce2fe70a8b272 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -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, } -/// 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` 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, panic_in_drop_strategy: PanicStrategy, @@ -465,6 +485,7 @@ trivially_parameterized_over_tcx! { RawDefId, TraitImpls, IncoherentImpls, + CrateHeader, CrateRoot, CrateDep, AttrFlags, From 60e95e76d059e3bbeb19d819914df3bcf7684a82 Mon Sep 17 00:00:00 2001 From: jyn Date: Fri, 26 May 2023 11:59:25 -0500 Subject: [PATCH 2/2] Update proc-macro-api for the new rustc metadata format --- src/tools/rust-analyzer/crates/proc-macro-api/src/version.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/rust-analyzer/crates/proc-macro-api/src/version.rs b/src/tools/rust-analyzer/crates/proc-macro-api/src/version.rs index cf637ec359a2d..13f67a0128a7e 100644 --- a/src/tools/rust-analyzer/crates/proc-macro-api/src/version.rs +++ b/src/tools/rust-analyzer/crates/proc-macro-api/src/version.rs @@ -122,7 +122,7 @@ pub fn read_version(dylib_path: &AbsPath) -> io::Result { // 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]