-
Notifications
You must be signed in to change notification settings - Fork 14k
Description
In some places, we have HashStable implementations that cache a computed Fingerprint to avoid re-computing it:
rust/compiler/rustc_middle/src/ty/impls_ty.rs
Lines 14 to 22 in aad4f10
| impl<'a, 'tcx, T> HashStable<StableHashingContext<'a>> for &'tcx ty::List<T> | |
| where | |
| T: HashStable<StableHashingContext<'a>>, | |
| { | |
| fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { | |
| thread_local! { | |
| static CACHE: RefCell<FxHashMap<(usize, usize), Fingerprint>> = | |
| RefCell::new(Default::default()); | |
| } |
rust/compiler/rustc_middle/src/ty/adt.rs
Lines 136 to 140 in aad4f10
| impl<'a> HashStable<StableHashingContext<'a>> for AdtDef { | |
| fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { | |
| thread_local! { | |
| static CACHE: RefCell<FxHashMap<usize, Fingerprint>> = Default::default(); | |
| } |
rust/compiler/rustc_span/src/hygiene.rs
Lines 164 to 166 in aad4f10
| pub fn fresh(mut expn_data: ExpnData, ctx: impl HashStableContext) -> LocalExpnId { | |
| debug_assert_eq!(expn_data.parent.krate, LOCAL_CRATE); | |
| let expn_hash = update_disambiguator(&mut expn_data, ctx); |
However, this does not take into account that the StableHashingContext may be configured to hash certain things different. Spans can be skipped using hcx.while_hashing_spans, and HirIds can be skipped using hcx.with_node_id_hashing_mode. These settings are not used in the cache key, so we will end up caching using whatever settings are used the first time the structure is hashed. As a result, later uses of hcx.while_hashing_spans and hcx.with_node_id_hashing_mode may end up using an incorrect cache entry.
In particular, we need to ignore spans while computing the legacy symbol hash:
rust/compiler/rustc_symbol_mangling/src/legacy.rs
Lines 113 to 117 in aad4f10
| hcx.while_hashing_spans(false, |hcx| { | |
| hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| { | |
| item_type.hash_stable(hcx, &mut hasher); | |
| }); | |
| }); |
If we've already populated one of the caches with a value that included hashed spans, then our symbol hash might actually take spans into account, even though we've explicitly requested that it should not.
We need to either include the relevant setting in the cache key, or enforce that we only ever try to hash the structure (e.g. ExpnData) with the same settings.
Discovered while working on #92210