From 457de0848777473ddafda998ab9384cbfbf4b87a Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 19 Sep 2021 22:17:50 +0200 Subject: [PATCH] Forbid hashing HIR outside of indexing. --- Cargo.lock | 2 + compiler/rustc_ast_lowering/Cargo.toml | 1 + compiler/rustc_ast_lowering/src/lib.rs | 45 ++----------- .../rustc_middle/src/hir/map/collector.rs | 22 ++++--- compiler/rustc_middle/src/hir/map/mod.rs | 8 +-- compiler/rustc_middle/src/hir/mod.rs | 13 ++-- compiler/rustc_middle/src/ty/context.rs | 7 +- compiler/rustc_query_system/src/ich/hcx.rs | 64 +++++++++++-------- .../rustc_query_system/src/ich/impls_hir.rs | 10 ++- compiler/rustc_resolve/Cargo.toml | 1 + compiler/rustc_resolve/src/lib.rs | 45 ++----------- 11 files changed, 86 insertions(+), 132 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 00d470017bb..0d89ffb7264 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3552,6 +3552,7 @@ dependencies = [ "rustc_errors", "rustc_hir", "rustc_index", + "rustc_query_system", "rustc_session", "rustc_span", "rustc_target", @@ -4333,6 +4334,7 @@ dependencies = [ "rustc_index", "rustc_metadata", "rustc_middle", + "rustc_query_system", "rustc_session", "rustc_span", "smallvec", diff --git a/compiler/rustc_ast_lowering/Cargo.toml b/compiler/rustc_ast_lowering/Cargo.toml index f4859ee4ae9..7989af24d99 100644 --- a/compiler/rustc_ast_lowering/Cargo.toml +++ b/compiler/rustc_ast_lowering/Cargo.toml @@ -14,6 +14,7 @@ rustc_hir = { path = "../rustc_hir" } rustc_target = { path = "../rustc_target" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_index = { path = "../rustc_index" } +rustc_query_system = { path = "../rustc_query_system" } rustc_span = { path = "../rustc_span" } rustc_errors = { path = "../rustc_errors" } rustc_session = { path = "../rustc_session" } diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 8375a37d32a..80b95b99b16 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -51,13 +51,14 @@ use rustc_hir::intravisit; use rustc_hir::{ConstArg, GenericArg, InferKind, ParamName}; use rustc_index::vec::{Idx, IndexVec}; +use rustc_query_system::ich::StableHashingContext; use rustc_session::lint::builtin::BARE_TRAIT_OBJECTS; use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer}; use rustc_session::utils::{FlattenNonterminals, NtToTokenstream}; use rustc_session::Session; use rustc_span::edition::Edition; use rustc_span::hygiene::ExpnId; -use rustc_span::source_map::{respan, CachingSourceMapView, DesugaringKind}; +use rustc_span::source_map::{respan, DesugaringKind}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{Span, DUMMY_SP}; @@ -179,6 +180,8 @@ pub trait ResolverAstLowering { /// This should only return `None` during testing. fn definitions(&mut self) -> &mut Definitions; + fn create_stable_hashing_context(&self) -> StableHashingContext<'_>; + fn lint_buffer(&mut self) -> &mut LintBuffer; fn next_node_id(&mut self) -> NodeId; @@ -201,37 +204,6 @@ fn create_def( ) -> LocalDefId; } -struct LoweringHasher<'a> { - source_map: CachingSourceMapView<'a>, - resolver: &'a dyn ResolverAstLowering, -} - -impl<'a> rustc_span::HashStableContext for LoweringHasher<'a> { - #[inline] - fn hash_spans(&self) -> bool { - true - } - - #[inline] - fn def_span(&self, id: LocalDefId) -> Span { - self.resolver.def_span(id) - } - - #[inline] - fn def_path_hash(&self, def_id: DefId) -> DefPathHash { - self.resolver.def_path_hash(def_id) - } - - #[inline] - fn span_data_to_lines_and_cols( - &mut self, - span: &rustc_span::SpanData, - ) -> Option<(Lrc, usize, rustc_span::BytePos, usize, rustc_span::BytePos)> - { - self.source_map.span_data_to_lines_and_cols(span) - } -} - /// Context of `impl Trait` in code, which determines whether it is allowed in an HIR subtree, /// and if so, what meaning it has. #[derive(Debug)] @@ -440,13 +412,6 @@ fn lower_crate(mut self, c: &Crate) -> &'hir hir::Crate<'hir> { self.arena.alloc(krate) } - fn create_stable_hashing_context(&self) -> LoweringHasher<'_> { - LoweringHasher { - source_map: CachingSourceMapView::new(self.sess.source_map()), - resolver: self.resolver, - } - } - fn with_hir_id_owner( &mut self, owner: NodeId, @@ -566,7 +531,7 @@ fn mark_span_with_reason( allow_internal_unstable, reason, self.sess.edition(), - self.create_stable_hashing_context(), + self.resolver.create_stable_hashing_context(), ) } diff --git a/compiler/rustc_middle/src/hir/map/collector.rs b/compiler/rustc_middle/src/hir/map/collector.rs index 868c1b7853e..80e48a4f74b 100644 --- a/compiler/rustc_middle/src/hir/map/collector.rs +++ b/compiler/rustc_middle/src/hir/map/collector.rs @@ -51,18 +51,21 @@ fn insert_vec_map(map: &mut IndexVec>, k: K, v: V map[k] = Some(v); } -fn hash_body( - hcx: &mut StableHashingContext<'_>, +fn hash_body<'s, 'hir: 's>( + hcx: &mut StableHashingContext<'s>, item_like: impl for<'a> HashStable>, + hash_bodies: bool, + owner: LocalDefId, + bodies: &'hir IndexVec>>, ) -> Fingerprint { let mut stable_hasher = StableHasher::new(); - hcx.while_hashing_hir_bodies(true, |hcx| { - item_like.hash_stable(hcx, &mut stable_hasher); + hcx.with_hir_bodies(hash_bodies, owner, bodies, |hcx| { + item_like.hash_stable(hcx, &mut stable_hasher) }); stable_hasher.finish() } -impl<'a, 'hir> NodeCollector<'a, 'hir> { +impl<'a, 'hir: 'a> NodeCollector<'a, 'hir> { pub(super) fn root( sess: &'a Session, arena: &'hir Arena<'hir>, @@ -91,15 +94,16 @@ pub(super) fn finalize_and_compute_crate_hash(self) -> IndexedHir<'hir> { } fn insert_owner(&mut self, owner: LocalDefId, node: OwnerNode<'hir>) { - let hash = hash_body(&mut self.hcx, node); - let mut nodes = IndexVec::new(); nodes.push(Some(ParentedNode { parent: ItemLocalId::new(0), node: node.into() })); let bodies = &self.krate.owners[owner].as_ref().unwrap().bodies; + let hash = hash_body(&mut self.hcx, node, true, owner, bodies); + let node_hash = hash_body(&mut self.hcx, node, false, owner, bodies); + debug_assert!(self.map[owner].is_none()); - self.map[owner] = Some(self.arena.alloc(OwnerNodes { hash, nodes, bodies })); + self.map[owner] = Some(self.arena.alloc(OwnerNodes { hash, node_hash, nodes, bodies })); } fn insert(&mut self, span: Span, hir_id: HirId, node: Node<'hir>) { @@ -176,7 +180,7 @@ fn insert_nested(&mut self, item: LocalDefId) { } } -impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { +impl<'a, 'hir: 'a> Visitor<'hir> for NodeCollector<'a, 'hir> { type Map = Map<'hir>; /// Because we want to track parent items and so forth, enable diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 66d4ec2eeb6..8c11fd8a280 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -874,21 +874,21 @@ pub fn get_foreign_abi(&self, hir_id: HirId) -> Abi { pub fn expect_item(&self, id: HirId) -> &'hir Item<'hir> { match self.tcx.hir_owner(id.expect_owner()) { - Some(Owner { node: OwnerNode::Item(item) }) => item, + Some(Owner { node: OwnerNode::Item(item), .. }) => item, _ => bug!("expected item, found {}", self.node_to_string(id)), } } pub fn expect_impl_item(&self, id: HirId) -> &'hir ImplItem<'hir> { match self.tcx.hir_owner(id.expect_owner()) { - Some(Owner { node: OwnerNode::ImplItem(item) }) => item, + Some(Owner { node: OwnerNode::ImplItem(item), .. }) => item, _ => bug!("expected impl item, found {}", self.node_to_string(id)), } } pub fn expect_trait_item(&self, id: HirId) -> &'hir TraitItem<'hir> { match self.tcx.hir_owner(id.expect_owner()) { - Some(Owner { node: OwnerNode::TraitItem(item) }) => item, + Some(Owner { node: OwnerNode::TraitItem(item), .. }) => item, _ => bug!("expected trait item, found {}", self.node_to_string(id)), } } @@ -902,7 +902,7 @@ pub fn expect_variant(&self, id: HirId) -> &'hir Variant<'hir> { pub fn expect_foreign_item(&self, id: HirId) -> &'hir ForeignItem<'hir> { match self.tcx.hir_owner(id.expect_owner()) { - Some(Owner { node: OwnerNode::ForeignItem(item) }) => item, + Some(Owner { node: OwnerNode::ForeignItem(item), .. }) => item, _ => bug!("expected foreign item, found {}", self.node_to_string(id)), } } diff --git a/compiler/rustc_middle/src/hir/mod.rs b/compiler/rustc_middle/src/hir/mod.rs index 094198713cc..6d24190eefb 100644 --- a/compiler/rustc_middle/src/hir/mod.rs +++ b/compiler/rustc_middle/src/hir/mod.rs @@ -39,12 +39,14 @@ pub struct IndexedHir<'hir> { #[derive(Copy, Clone, Debug)] pub struct Owner<'tcx> { node: OwnerNode<'tcx>, + node_hash: Fingerprint, } impl<'a, 'tcx> HashStable> for Owner<'tcx> { + #[inline] fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { - let Owner { node } = self; - hcx.while_hashing_hir_bodies(false, |hcx| node.hash_stable(hcx, hasher)); + let Owner { node: _, node_hash } = self; + node_hash.hash_stable(hcx, hasher) } } @@ -61,6 +63,8 @@ pub struct ParentedNode<'tcx> { pub struct OwnerNodes<'tcx> { /// Pre-computed hash of the full HIR. hash: Fingerprint, + /// Pre-computed hash of the top node. + node_hash: Fingerprint, /// Full HIR for the current owner. // The zeroth node's parent is trash, but is never accessed. nodes: IndexVec>>, @@ -69,10 +73,11 @@ pub struct OwnerNodes<'tcx> { } impl<'a, 'tcx> HashStable> for OwnerNodes<'tcx> { + #[inline] fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { // We ignore the `nodes` and `bodies` fields since these refer to information included in // `hash` which is hashed in the collector and used for the crate hash. - let OwnerNodes { hash, nodes: _, bodies: _ } = *self; + let OwnerNodes { hash, node_hash: _, nodes: _, bodies: _ } = *self; hash.hash_stable(hcx, hasher); } } @@ -130,7 +135,7 @@ pub fn provide(providers: &mut Providers) { let owner = tcx.index_hir(()).map[id].as_ref()?; let node = owner.nodes[ItemLocalId::new(0)].as_ref().unwrap().node; let node = node.as_owner().unwrap(); // Indexing must ensure it is an OwnerNode. - Some(Owner { node }) + Some(Owner { node, node_hash: owner.node_hash }) }; providers.hir_owner_nodes = |tcx, id| tcx.index_hir(()).map[id].as_deref(); providers.hir_owner_parent = |tcx, id| { diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index cbbd89e9033..5dea574c484 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -1342,20 +1342,15 @@ pub fn definitions_untracked(self) -> &'tcx hir::definitions::Definitions { #[inline(always)] pub fn create_stable_hashing_context(self) -> StableHashingContext<'tcx> { - let krate = self.gcx.untracked_crate; let resolutions = &self.gcx.untracked_resolutions; - - StableHashingContext::new(self.sess, krate, &resolutions.definitions, &*resolutions.cstore) + StableHashingContext::new(self.sess, &resolutions.definitions, &*resolutions.cstore) } #[inline(always)] pub fn create_no_span_stable_hashing_context(self) -> StableHashingContext<'tcx> { - let krate = self.gcx.untracked_crate; let resolutions = &self.gcx.untracked_resolutions; - StableHashingContext::ignore_spans( self.sess, - krate, &resolutions.definitions, &*resolutions.cstore, ) diff --git a/compiler/rustc_query_system/src/ich/hcx.rs b/compiler/rustc_query_system/src/ich/hcx.rs index f2e935c59fc..cfef2073373 100644 --- a/compiler/rustc_query_system/src/ich/hcx.rs +++ b/compiler/rustc_query_system/src/ich/hcx.rs @@ -6,6 +6,7 @@ use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::definitions::{DefPathHash, Definitions}; +use rustc_index::vec::IndexVec; use rustc_session::cstore::CrateStore; use rustc_session::Session; use rustc_span::source_map::SourceMap; @@ -27,7 +28,6 @@ pub struct StableHashingContext<'a> { cstore: &'a dyn CrateStore, pub(super) body_resolver: BodyResolver<'a>, hash_spans: bool, - hash_bodies: bool, pub(super) node_id_hashing_mode: NodeIdHashingMode, // Very often, we are hashing something that does not need the @@ -46,24 +46,19 @@ pub enum NodeIdHashingMode { /// We could also just store a plain reference to the `hir::Crate` but we want /// to avoid that the crate is used to get untracked access to all of the HIR. #[derive(Clone, Copy)] -pub(super) struct BodyResolver<'tcx>(&'tcx hir::Crate<'tcx>); - -impl<'tcx> BodyResolver<'tcx> { - /// Returns a reference to the `hir::Body` with the given `BodyId`. - /// **Does not do any tracking**; use carefully. - pub(super) fn body(self, id: hir::BodyId) -> &'tcx hir::Body<'tcx> { - self.0.body(id) - } +pub(super) enum BodyResolver<'tcx> { + Forbidden, + Traverse { + hash_bodies: bool, + owner: LocalDefId, + bodies: &'tcx IndexVec>>, + }, } impl<'a> StableHashingContext<'a> { - /// The `krate` here is only used for mapping `BodyId`s to `Body`s. - /// Don't use it for anything else or you'll run the risk of - /// leaking data out of the tracking system. #[inline] fn new_with_or_without_spans( sess: &'a Session, - krate: &'a hir::Crate<'a>, definitions: &'a Definitions, cstore: &'a dyn CrateStore, always_ignore_spans: bool, @@ -72,13 +67,12 @@ fn new_with_or_without_spans( !always_ignore_spans && !sess.opts.debugging_opts.incremental_ignore_spans; StableHashingContext { - body_resolver: BodyResolver(krate), + body_resolver: BodyResolver::Forbidden, definitions, cstore, caching_source_map: None, raw_source_map: sess.source_map(), hash_spans: hash_spans_initial, - hash_bodies: true, node_id_hashing_mode: NodeIdHashingMode::HashDefPath, } } @@ -86,13 +80,11 @@ fn new_with_or_without_spans( #[inline] pub fn new( sess: &'a Session, - krate: &'a hir::Crate<'a>, definitions: &'a Definitions, cstore: &'a dyn CrateStore, ) -> Self { Self::new_with_or_without_spans( sess, - krate, definitions, cstore, /*always_ignore_spans=*/ false, @@ -102,20 +94,41 @@ pub fn new( #[inline] pub fn ignore_spans( sess: &'a Session, - krate: &'a hir::Crate<'a>, definitions: &'a Definitions, cstore: &'a dyn CrateStore, ) -> Self { let always_ignore_spans = true; - Self::new_with_or_without_spans(sess, krate, definitions, cstore, always_ignore_spans) + Self::new_with_or_without_spans(sess, definitions, cstore, always_ignore_spans) } + /// Allow hashing #[inline] - pub fn while_hashing_hir_bodies(&mut self, hash_bodies: bool, f: F) { - let prev_hash_bodies = self.hash_bodies; - self.hash_bodies = hash_bodies; + pub fn while_hashing_hir_bodies(&mut self, hb: bool, f: impl FnOnce(&mut Self)) { + let prev = match &mut self.body_resolver { + BodyResolver::Forbidden => panic!("Hashing HIR bodies is forbidden."), + BodyResolver::Traverse { ref mut hash_bodies, .. } => { + std::mem::replace(hash_bodies, hb) + } + }; f(self); - self.hash_bodies = prev_hash_bodies; + match &mut self.body_resolver { + BodyResolver::Forbidden => unreachable!(), + BodyResolver::Traverse { ref mut hash_bodies, .. } => *hash_bodies = prev, + } + } + + #[inline] + pub fn with_hir_bodies( + &mut self, + hash_bodies: bool, + owner: LocalDefId, + bodies: &'a IndexVec>>, + f: impl FnOnce(&mut Self), + ) { + let prev = self.body_resolver; + self.body_resolver = BodyResolver::Traverse { hash_bodies, owner, bodies }; + f(self); + self.body_resolver = prev; } #[inline] @@ -152,11 +165,6 @@ pub fn local_def_path_hash(&self, def_id: LocalDefId) -> DefPathHash { self.definitions.def_path_hash(def_id) } - #[inline] - pub fn hash_bodies(&self) -> bool { - self.hash_bodies - } - #[inline] pub fn source_map(&mut self) -> &mut CachingSourceMapView<'a> { match self.caching_source_map { diff --git a/compiler/rustc_query_system/src/ich/impls_hir.rs b/compiler/rustc_query_system/src/ich/impls_hir.rs index 04eb263a977..dc208b36f93 100644 --- a/compiler/rustc_query_system/src/ich/impls_hir.rs +++ b/compiler/rustc_query_system/src/ich/impls_hir.rs @@ -1,6 +1,7 @@ //! This module contains `HashStable` implementations for various HIR data //! types in no particular order. +use crate::ich::hcx::BodyResolver; use crate::ich::{NodeIdHashingMode, StableHashingContext}; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey}; @@ -29,8 +30,13 @@ fn hash_hir_id(&mut self, hir_id: hir::HirId, hasher: &mut StableHasher) { #[inline] fn hash_body_id(&mut self, id: hir::BodyId, hasher: &mut StableHasher) { let hcx = self; - if hcx.hash_bodies() { - hcx.body_resolver.body(id).hash_stable(hcx, hasher); + match hcx.body_resolver { + BodyResolver::Forbidden => panic!("Hashing HIR bodies is forbidden."), + BodyResolver::Traverse { hash_bodies: false, .. } => {} + BodyResolver::Traverse { hash_bodies: true, owner, bodies } => { + assert_eq!(id.hir_id.owner, owner); + bodies[id.hir_id.local_id].unwrap().hash_stable(hcx, hasher); + } } } diff --git a/compiler/rustc_resolve/Cargo.toml b/compiler/rustc_resolve/Cargo.toml index f1d3315d6e6..bd27c16c732 100644 --- a/compiler/rustc_resolve/Cargo.toml +++ b/compiler/rustc_resolve/Cargo.toml @@ -23,6 +23,7 @@ rustc_feature = { path = "../rustc_feature" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } rustc_metadata = { path = "../rustc_metadata" } +rustc_query_system = { path = "../rustc_query_system" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } smallvec = { version = "1.6.1", features = ["union", "may_dangle"] } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 28fe365fb58..f08878ea925 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -54,13 +54,14 @@ use rustc_middle::span_bug; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, DefIdTree, MainDefinition, ResolverOutputs}; +use rustc_query_system::ich::StableHashingContext; use rustc_session::cstore::{CrateStore, MetadataLoaderDyn}; use rustc_session::lint; use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer}; use rustc_session::Session; use rustc_span::edition::Edition; use rustc_span::hygiene::{ExpnId, ExpnKind, LocalExpnId, MacroKind, SyntaxContext, Transparency}; -use rustc_span::source_map::{CachingSourceMapView, Spanned}; +use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{Span, DUMMY_SP}; @@ -1177,6 +1178,10 @@ fn definitions(&mut self) -> &mut Definitions { &mut self.definitions } + fn create_stable_hashing_context(&self) -> StableHashingContext<'_> { + StableHashingContext::new(self.session, &self.definitions, self.crate_loader.cstore()) + } + fn lint_buffer(&mut self) -> &mut LintBuffer { &mut self.lint_buffer } @@ -1245,37 +1250,6 @@ fn create_def( } } -struct ExpandHasher<'a, 'b> { - source_map: CachingSourceMapView<'a>, - resolver: &'a Resolver<'b>, -} - -impl<'a, 'b> rustc_span::HashStableContext for ExpandHasher<'a, 'b> { - #[inline] - fn hash_spans(&self) -> bool { - true - } - - #[inline] - fn def_span(&self, id: LocalDefId) -> Span { - self.resolver.def_span(id) - } - - #[inline] - fn def_path_hash(&self, def_id: DefId) -> DefPathHash { - self.resolver.def_path_hash(def_id) - } - - #[inline] - fn span_data_to_lines_and_cols( - &mut self, - span: &rustc_span::SpanData, - ) -> Option<(Lrc, usize, rustc_span::BytePos, usize, rustc_span::BytePos)> - { - self.source_map.span_data_to_lines_and_cols(span) - } -} - impl<'a> Resolver<'a> { pub fn new( session: &'a Session, @@ -1456,13 +1430,6 @@ fn new_module( self.arenas.new_module(parent, kind, expn_id, span, no_implicit_prelude, module_map) } - fn create_stable_hashing_context(&self) -> ExpandHasher<'_, 'a> { - ExpandHasher { - source_map: CachingSourceMapView::new(self.session.source_map()), - resolver: self, - } - } - pub fn next_node_id(&mut self) -> NodeId { let next = self .next_node_id -- 2.44.0