Skip to content

Commit

Permalink
Let a portion of DefPathHash uniquely identify the DefPath's crate.
Browse files Browse the repository at this point in the history
This allows to directly map from a DefPathHash to the crate it
originates from, without constructing side tables to do that mapping.

It also allows to reliably and cheaply check for DefPathHash collisions.
  • Loading branch information
michaelwoerister committed Feb 2, 2021
1 parent a3ed564 commit 22d489b
Show file tree
Hide file tree
Showing 12 changed files with 181 additions and 27 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_ast/src/lib.rs
Expand Up @@ -41,7 +41,6 @@ pub mod util {

pub mod ast;
pub mod attr;
pub mod crate_disambiguator;
pub mod entry;
pub mod expand;
pub mod mut_visit;
Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_data_structures/src/fingerprint.rs
Expand Up @@ -7,19 +7,30 @@ use std::hash::{Hash, Hasher};
use std::mem::{self, MaybeUninit};

#[derive(Eq, PartialEq, Ord, PartialOrd, Debug, Clone, Copy)]
#[repr(C)]
pub struct Fingerprint(u64, u64);

impl Fingerprint {
pub const ZERO: Fingerprint = Fingerprint(0, 0);

#[inline]
pub fn new(_0: u64, _1: u64) -> Fingerprint {
Fingerprint(_0, _1)
}

#[inline]
pub fn from_smaller_hash(hash: u64) -> Fingerprint {
Fingerprint(hash, hash)
}

#[inline]
pub fn to_smaller_hash(&self) -> u64 {
self.0
// Even though both halves of the fingerprint are expected to be good
// quality hash values, let's still combine the two values because the
// Fingerprints in DefPathHash have the StableCrateId portion which is
// the same for all DefPathHashes from the same crate. Combining the
// two halfs makes sure we get a good quality hash in such cases too.
self.0.wrapping_mul(3).wrapping_add(self.1)
}

#[inline]
Expand Down Expand Up @@ -93,7 +104,7 @@ impl FingerprintHasher for crate::unhash::Unhasher {
#[inline]
fn write_fingerprint(&mut self, fingerprint: &Fingerprint) {
// `Unhasher` only wants a single `u64`
self.write_u64(fingerprint.0);
self.write_u64(fingerprint.0.wrapping_add(fingerprint.1));
}
}

Expand Down
61 changes: 41 additions & 20 deletions compiler/rustc_hir/src/definitions.rs
Expand Up @@ -5,13 +5,16 @@
//! expressions) that are mostly just leftovers.

pub use crate::def_id::DefPathHash;
use crate::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
use crate::def_id::{
CrateNum, DefId, DefIndex, LocalDefId, StableCrateId, CRATE_DEF_INDEX, LOCAL_CRATE,
};
use crate::hir;

use rustc_ast::crate_disambiguator::CrateDisambiguator;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::StableHasher;
use rustc_data_structures::unhash::UnhashMap;
use rustc_index::vec::IndexVec;
use rustc_span::crate_disambiguator::CrateDisambiguator;
use rustc_span::hygiene::ExpnId;
use rustc_span::symbol::{kw, sym, Symbol};

Expand All @@ -27,6 +30,7 @@ use tracing::debug;
pub struct DefPathTable {
index_to_key: IndexVec<DefIndex, DefKey>,
def_path_hashes: IndexVec<DefIndex, DefPathHash>,
def_path_hash_to_index: UnhashMap<DefPathHash, DefIndex>,
}

impl DefPathTable {
Expand All @@ -39,6 +43,25 @@ impl DefPathTable {
};
self.def_path_hashes.push(def_path_hash);
debug_assert!(self.def_path_hashes.len() == self.index_to_key.len());

// Check for hash collisions of DefPathHashes. These should be
// exceedingly rare.
if let Some(existing) = self.def_path_hash_to_index.insert(def_path_hash, index) {
let def_path1 = DefPath::make(LOCAL_CRATE, existing, |idx| self.def_key(idx));
let def_path2 = DefPath::make(LOCAL_CRATE, index, |idx| self.def_key(idx));

// Continuing with colliding DefPathHashes can lead to correctness
// issues. We must abort compilation.
panic!("Found DefPathHash collsion between {:?} and {:?}", def_path1, def_path2);
}

// Assert that all DefPathHashes correctly contain the local crate's
// StableCrateId
#[cfg(debug_assertions)]
if let Some(root) = self.def_path_hashes.get(CRATE_DEF_INDEX) {
assert!(def_path_hash.stable_crate_id() == root.stable_crate_id());
}

index
}

Expand Down Expand Up @@ -108,13 +131,10 @@ pub struct DefKey {
}

impl DefKey {
fn compute_stable_hash(&self, parent_hash: DefPathHash) -> DefPathHash {
fn compute_stable_hash(&self, parent: DefPathHash) -> DefPathHash {
let mut hasher = StableHasher::new();

// We hash a `0u8` here to disambiguate between regular `DefPath` hashes,
// and the special "root_parent" below.
0u8.hash(&mut hasher);
parent_hash.hash(&mut hasher);
parent.hash(&mut hasher);

let DisambiguatedDefPathData { ref data, disambiguator } = self.disambiguated_data;

Expand All @@ -127,19 +147,13 @@ impl DefKey {

disambiguator.hash(&mut hasher);

DefPathHash(hasher.finish())
}
let local_hash: u64 = hasher.finish();

fn root_parent_stable_hash(
crate_name: &str,
crate_disambiguator: CrateDisambiguator,
) -> DefPathHash {
let mut hasher = StableHasher::new();
// Disambiguate this from a regular `DefPath` hash; see `compute_stable_hash()` above.
1u8.hash(&mut hasher);
crate_name.hash(&mut hasher);
crate_disambiguator.hash(&mut hasher);
DefPathHash(hasher.finish())
// Construct the new DefPathHash, making sure that the `crate_id`
// portion of the hash is properly copied from the parent. This way the
// `crate_id` part will be recursively propagated from the root to all
// DefPathHashes in this DefPathTable.
DefPathHash::new(parent.stable_crate_id(), local_hash)
}
}

Expand Down Expand Up @@ -295,6 +309,12 @@ impl Definitions {
self.table.def_path_hash(id.local_def_index)
}

#[inline]
pub fn def_path_hash_to_def_id(&self, def_path_hash: DefPathHash) -> LocalDefId {
let local_def_index = self.table.def_path_hash_to_index[&def_path_hash];
LocalDefId { local_def_index }
}

/// Returns the path from the crate root to `index`. The root
/// nodes are not included in the path (i.e., this will be an
/// empty vector for the crate root). For an inlined item, this
Expand Down Expand Up @@ -332,7 +352,8 @@ impl Definitions {
},
};

let parent_hash = DefKey::root_parent_stable_hash(crate_name, crate_disambiguator);
let stable_crate_id = StableCrateId::new(crate_name, crate_disambiguator);
let parent_hash = DefPathHash::new(stable_crate_id, 0);
let def_path_hash = key.compute_stable_hash(parent_hash);

// Create the root definition.
Expand Down
30 changes: 28 additions & 2 deletions compiler/rustc_metadata/src/creader.rs
Expand Up @@ -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::FxHashSet;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::Lrc;
use rustc_expand::base::SyntaxExtension;
use rustc_hir::def_id::{CrateNum, LocalDefId, LOCAL_CRATE};
use rustc_hir::def_id::{CrateNum, LocalDefId, StableCrateId, LOCAL_CRATE};
use rustc_hir::definitions::Definitions;
use rustc_index::vec::IndexVec;
use rustc_middle::middle::cstore::{CrateDepKind, CrateSource, ExternCrate};
Expand Down Expand Up @@ -40,6 +40,10 @@ pub struct CStore {
allocator_kind: Option<AllocatorKind>,
/// This crate has a `#[global_allocator]` item.
has_global_allocator: bool,

/// This map is used to verify we get no hash conflicts between
/// `StableCrateId` values.
stable_crate_ids: FxHashMap<StableCrateId, CrateNum>,
}

pub struct CrateLoader<'a> {
Expand Down Expand Up @@ -192,6 +196,11 @@ impl<'a> CrateLoader<'a> {
metadata_loader: &'a MetadataLoaderDyn,
local_crate_name: &str,
) -> Self {
let local_crate_stable_id =
StableCrateId::new(local_crate_name, sess.local_crate_disambiguator());
let mut stable_crate_ids = FxHashMap::default();
stable_crate_ids.insert(local_crate_stable_id, LOCAL_CRATE);

CrateLoader {
sess,
metadata_loader,
Expand All @@ -205,6 +214,7 @@ impl<'a> CrateLoader<'a> {
injected_panic_runtime: None,
allocator_kind: None,
has_global_allocator: false,
stable_crate_ids,
},
used_extern_options: Default::default(),
}
Expand Down Expand Up @@ -311,6 +321,20 @@ impl<'a> CrateLoader<'a> {
res
}

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<Library>,
Expand All @@ -332,6 +356,8 @@ impl<'a> CrateLoader<'a> {
// Claim this crate number and cache it
let cnum = self.cstore.alloc_new_crate_num();

self.verify_no_stable_crate_id_hash_conflicts(&crate_root, cnum)?;

info!(
"register crate `{}` (cnum = {}. private_dep = {})",
crate_root.name(),
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_metadata/src/locator.rs
Expand Up @@ -888,6 +888,7 @@ crate enum CrateError {
MultipleMatchingCrates(Symbol, FxHashMap<Svh, Library>),
SymbolConflictsCurrent(Symbol),
SymbolConflictsOthers(Symbol),
StableCrateIdCollision(Symbol, Symbol),
DlOpen(String),
DlSym(String),
LocatorCombined(CombinedLocatorError),
Expand Down Expand Up @@ -970,6 +971,13 @@ impl CrateError {
`-C metadata`. This will result in symbol conflicts between the two.",
root_name,
),
CrateError::StableCrateIdCollision(crate_name0, crate_name1) => {
let msg = format!(
"found crates (`{}` and `{}`) with colliding StableCrateId values.",
crate_name0, crate_name1
);
sess.struct_span_err(span, &msg)
}
CrateError::DlOpen(s) | CrateError::DlSym(s) => sess.struct_span_err(span, &s),
CrateError::LocatorCombined(locator) => {
let crate_name = locator.crate_name;
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Expand Up @@ -635,6 +635,10 @@ impl CrateRoot<'_> {
self.hash
}

crate fn stable_crate_id(&self) -> StableCrateId {
self.stable_crate_id
}

crate fn triple(&self) -> &TargetTriple {
&self.triple
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Expand Up @@ -651,6 +651,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
triple: tcx.sess.opts.target_triple.clone(),
hash: tcx.crate_hash(LOCAL_CRATE),
disambiguator: tcx.sess.local_crate_disambiguator(),
stable_crate_id: tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id(),
panic_strategy: tcx.sess.panic_strategy(),
edition: tcx.sess.edition(),
has_global_allocator: tcx.has_global_allocator(LOCAL_CRATE),
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_metadata/src/rmeta/mod.rs
Expand Up @@ -7,7 +7,7 @@ use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::MetadataRef;
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::def_id::{DefId, DefIndex, DefPathHash};
use rustc_hir::def_id::{DefId, DefIndex, DefPathHash, StableCrateId};
use rustc_hir::definitions::DefKey;
use rustc_hir::lang_items;
use rustc_index::{bit_set::FiniteBitSet, vec::IndexVec};
Expand Down Expand Up @@ -203,6 +203,7 @@ crate struct CrateRoot<'tcx> {
extra_filename: String,
hash: Svh,
disambiguator: CrateDisambiguator,
stable_crate_id: StableCrateId,
panic_strategy: PanicStrategy,
edition: Edition,
has_global_allocator: bool,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_session/src/session.rs
Expand Up @@ -8,7 +8,6 @@ use crate::parse::ParseSess;
use crate::search_paths::{PathKind, SearchPath};

pub use rustc_ast::attr::MarkedAttrs;
pub use rustc_ast::crate_disambiguator::CrateDisambiguator;
pub use rustc_ast::Attribute;
use rustc_data_structures::flock;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
Expand All @@ -23,6 +22,7 @@ use rustc_errors::json::JsonEmitter;
use rustc_errors::registry::Registry;
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, DiagnosticId, ErrorReported};
use rustc_lint_defs::FutureBreakage;
pub use rustc_span::crate_disambiguator::CrateDisambiguator;
use rustc_span::edition::Edition;
use rustc_span::source_map::{FileLoader, MultiSpan, RealFileLoader, SourceMap, Span};
use rustc_span::{sym, SourceFileHashAlgorithm, Symbol};
Expand Down

0 comments on commit 22d489b

Please sign in to comment.