From 460ecd288a34f73f9178acc723a459cbfe77607a Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 16 Mar 2023 10:05:09 +0000 Subject: [PATCH] Eagerly intern and check CrateNum/StableCrateId collisions --- Cargo.lock | 1 + compiler/rustc_metadata/src/creader.rs | 75 ++++++------------- compiler/rustc_metadata/src/rmeta/decoder.rs | 4 - .../src/rmeta/decoder/cstore_impl.rs | 5 +- compiler/rustc_span/Cargo.toml | 1 + compiler/rustc_span/src/def_id.rs | 6 +- tests/run-make-fulldeps/issue-83045/Makefile | 2 +- 7 files changed, 33 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0c0b5d6ab3b57..aa4d79ed26117 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5294,6 +5294,7 @@ name = "rustc_span" version = "0.0.0" dependencies = [ "cfg-if", + "indexmap", "md-5", "rustc_arena", "rustc_data_structures", diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index f870a1db82d9c..538e61c985104 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -6,11 +6,11 @@ use crate::rmeta::{CrateDep, CrateMetadata, CrateNumMap, CrateRoot, MetadataBlob use rustc_ast::expand::allocator::AllocatorKind; use rustc_ast::{self as ast, *}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::svh::Svh; use rustc_data_structures::sync::{MappedReadGuard, MappedWriteGuard, ReadGuard, WriteGuard}; use rustc_expand::base::SyntaxExtension; -use rustc_hir::def_id::{CrateNum, LocalDefId, StableCrateId, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, LocalDefId, StableCrateId, StableCrateIdMap, LOCAL_CRATE}; use rustc_hir::definitions::Definitions; use rustc_index::vec::IndexVec; use rustc_middle::ty::TyCtxt; @@ -46,9 +46,8 @@ pub struct CStore { /// This crate has a `#[alloc_error_handler]` item. has_alloc_error_handler: bool, - /// This map is used to verify we get no hash conflicts between - /// `StableCrateId` values. - pub(crate) stable_crate_ids: FxHashMap, + /// The interned [StableCrateId]s. + pub(crate) stable_crate_ids: StableCrateIdMap, /// Unused externs of the crate unused_externs: Vec, @@ -144,9 +143,21 @@ impl CStore { }) } - fn alloc_new_crate_num(&mut self) -> CrateNum { - self.metas.push(None); - CrateNum::new(self.metas.len() - 1) + fn intern_stable_crate_id(&mut self, root: &CrateRoot) -> Result { + assert_eq!(self.metas.len(), self.stable_crate_ids.len()); + let num = CrateNum::new(self.stable_crate_ids.len()); + if let Some(&existing) = self.stable_crate_ids.get(&root.stable_crate_id()) { + let crate_name0 = root.name(); + if let Some(crate_name1) = self.metas[existing].as_ref().map(|data| data.name()) { + Err(CrateError::StableCrateIdCollision(crate_name0, crate_name1)) + } else { + Err(CrateError::SymbolConflictsCurrent(crate_name0)) + } + } else { + self.metas.push(None); + self.stable_crate_ids.insert(root.stable_crate_id(), num); + Ok(num) + } } pub fn has_crate_data(&self, cnum: CrateNum) -> bool { @@ -247,7 +258,7 @@ impl CStore { } pub fn new(sess: &Session) -> CStore { - let mut stable_crate_ids = FxHashMap::default(); + let mut stable_crate_ids = StableCrateIdMap::default(); stable_crate_ids.insert(sess.local_stable_crate_id(), LOCAL_CRATE); CStore { // We add an empty entry for LOCAL_CRATE (which maps to zero) in @@ -342,42 +353,6 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { None } - fn verify_no_symbol_conflicts(&self, root: &CrateRoot) -> Result<(), CrateError> { - // Check for (potential) conflicts with the local crate - if self.sess.local_stable_crate_id() == root.stable_crate_id() { - return Err(CrateError::SymbolConflictsCurrent(root.name())); - } - - // Check for conflicts with any crate loaded so far - for (_, other) in self.cstore.iter_crate_data() { - // Same stable crate id but different SVH - if other.stable_crate_id() == root.stable_crate_id() && other.hash() != root.hash() { - bug!( - "Previously returned E0523 here. \ - See https://github.com/rust-lang/rust/pull/100599 for additional discussion.\ - root.name() = {}.", - root.name() - ); - } - } - - Ok(()) - } - - fn verify_no_stable_crate_id_hash_conflicts( - &mut self, - root: &CrateRoot, - cnum: CrateNum, - ) -> Result<(), CrateError> { - if let Some(existing) = self.cstore.stable_crate_ids.insert(root.stable_crate_id(), cnum) { - let crate_name0 = root.name(); - let crate_name1 = self.cstore.get_crate_data(existing).name(); - return Err(CrateError::StableCrateIdCollision(crate_name0, crate_name1)); - } - - Ok(()) - } - fn register_crate( &mut self, host_lib: Option, @@ -396,7 +371,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { self.sess.opts.externs.get(name.as_str()).map_or(false, |e| e.is_private_dep); // Claim this crate number and cache it - let cnum = self.cstore.alloc_new_crate_num(); + let cnum = self.cstore.intern_stable_crate_id(&crate_root)?; info!( "register crate `{}` (cnum = {}. private_dep = {})", @@ -432,14 +407,6 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { None }; - // Perform some verification *after* resolve_crate_deps() above is - // known to have been successful. It seems that - in error cases - the - // cstore can be in a temporarily invalid state between cnum allocation - // and dependency resolution and the verification code would produce - // ICEs in that case (see #83045). - self.verify_no_symbol_conflicts(&crate_root)?; - self.verify_no_stable_crate_id_hash_conflicts(&crate_root, cnum)?; - let crate_metadata = CrateMetadata::new( self.sess, &self.cstore, diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 0070e46ffdf02..cabc144077fd5 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1709,10 +1709,6 @@ impl CrateMetadata { self.root.name } - pub(crate) fn stable_crate_id(&self) -> StableCrateId { - self.root.stable_crate_id - } - pub(crate) fn hash(&self) -> Svh { self.root.hash } diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index a98433953367b..e0f1435ccb3ae 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -615,7 +615,10 @@ impl CrateStore for CStore { } fn stable_crate_id_to_crate_num(&self, stable_crate_id: StableCrateId) -> CrateNum { - self.stable_crate_ids[&stable_crate_id] + *self + .stable_crate_ids + .get(&stable_crate_id) + .unwrap_or_else(|| bug!("uninterned StableCrateId: {stable_crate_id:?}")) } /// Returns the `DefKey` for a given `DefId`. This indicates the diff --git a/compiler/rustc_span/Cargo.toml b/compiler/rustc_span/Cargo.toml index ae81d95e27967..98d6e0ab117a3 100644 --- a/compiler/rustc_span/Cargo.toml +++ b/compiler/rustc_span/Cargo.toml @@ -18,3 +18,4 @@ tracing = "0.1" sha1 = "0.10.0" sha2 = "0.10.1" md5 = { package = "md-5", version = "0.10.0" } +indexmap = { version = "1.9.1" } diff --git a/compiler/rustc_span/src/def_id.rs b/compiler/rustc_span/src/def_id.rs index 162c15574b56c..b2c58caff2ec4 100644 --- a/compiler/rustc_span/src/def_id.rs +++ b/compiler/rustc_span/src/def_id.rs @@ -1,13 +1,17 @@ use crate::{HashStableContext, Symbol}; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey}; +use rustc_data_structures::unhash::Unhasher; use rustc_data_structures::AtomicRef; use rustc_index::vec::Idx; use rustc_macros::HashStable_Generic; use rustc_serialize::{Decodable, Decoder, Encodable, Encoder}; use std::borrow::Borrow; use std::fmt; -use std::hash::{Hash, Hasher}; +use std::hash::{BuildHasherDefault, Hash, Hasher}; + +pub type StableCrateIdMap = + indexmap::IndexMap>; rustc_index::newtype_index! { #[custom_encodable] diff --git a/tests/run-make-fulldeps/issue-83045/Makefile b/tests/run-make-fulldeps/issue-83045/Makefile index 34853cb1d31e5..fc180ccfe28e2 100644 --- a/tests/run-make-fulldeps/issue-83045/Makefile +++ b/tests/run-make-fulldeps/issue-83045/Makefile @@ -29,5 +29,5 @@ all: --crate-type=rlib \ --edition=2018 \ c.rs 2>&1 | tee $(TMPDIR)/output.txt || exit 0 - $(CGREP) E0463 < $(TMPDIR)/output.txt + $(CGREP) E0519 < $(TMPDIR)/output.txt $(CGREP) -v "internal compiler error" < $(TMPDIR)/output.txt