From 3bb2e40a3ed9ef203c4a47161e6bf7660b63720e Mon Sep 17 00:00:00 2001 From: Makro Date: Wed, 29 Jul 2026 09:15:38 +0000 Subject: [PATCH] Tag disk-cached query values with their dep node, not the node index The node index only names a node relative to the session's serialized graph; cross-session value carry needs a tag that still identifies a value's node when the bytes outlive the session that wrote them. `(kind, key_fingerprint)` is unique per node for every kind that caches query values, so tagging with the `DepNode` itself is exactly as strong a within-session check as the index tag while staying valid across sessions. The expected node is threaded from the query layer through `try_load_from_disk_fn`, so the load path checks end to end that the decoded bytes belong to the node the query layer asked for, without consulting the dep graph. Side effects keep their index tags: side-effect nodes are exempt from the uniqueness guarantee, so a node tag would be ambiguous for them. The header format version is bumped because the tag changed. --- .../src/persist/file_format.rs | 2 +- .../rustc_middle/src/dep_graph/dep_node.rs | 20 +++++++++++++ .../rustc_middle/src/query/on_disk_cache.rs | 30 ++++++++++++++----- compiler/rustc_middle/src/query/plumbing.rs | 9 ++++-- compiler/rustc_query_impl/src/execution.rs | 2 +- compiler/rustc_query_impl/src/plumbing.rs | 10 +++++-- compiler/rustc_query_impl/src/query_impl.rs | 6 ++-- 7 files changed, 61 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_incremental/src/persist/file_format.rs b/compiler/rustc_incremental/src/persist/file_format.rs index 853a5c9ba7ab0..0e4102e54b77f 100644 --- a/compiler/rustc_incremental/src/persist/file_format.rs +++ b/compiler/rustc_incremental/src/persist/file_format.rs @@ -26,7 +26,7 @@ use crate::diagnostics; const FILE_MAGIC: &[u8] = b"RSIC"; /// Change this if the header format changes. -const HEADER_FORMAT_VERSION: u16 = 0; +const HEADER_FORMAT_VERSION: u16 = 1; pub(crate) fn write_file_header(stream: &mut FileEncoder<'_>, sess: &Session) { stream.emit_raw_bytes(FILE_MAGIC); diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index b6fda22775c2a..c52e3732a6047 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -55,6 +55,7 @@ use rustc_data_structures::stable_hash::{StableHasher, StableOrd}; use rustc_hir::def_id::DefId; use rustc_hir::definitions::DefPathHash; use rustc_macros::{Decodable, Encodable, StableHash}; +use rustc_serialize::{Decodable, Decoder, Encodable, Encoder}; use rustc_span::Symbol; use super::{DepNodeIndex, KeyFingerprintStyle, SerializedDepNodeIndex}; @@ -152,6 +153,25 @@ impl fmt::Debug for DepNode { } } +// `DepKind` is encoded as its `u16` discriminant, which is only meaningful to +// the compiler build that assigned it, so anything containing an encoded +// `DepNode` must be discarded on version mismatch. +impl Encodable for DepNode { + fn encode(&self, e: &mut E) { + e.emit_u16(self.kind.as_u16()); + self.key_fingerprint.encode(e); + } +} + +impl Decodable for DepNode { + fn decode(d: &mut D) -> Self { + DepNode { + kind: DepKind::from_u16(d.read_u16()), + key_fingerprint: PackedFingerprint::decode(d), + } + } +} + /// This struct stores function pointers and other metadata for a particular DepKind. /// /// Information is retrieved by indexing the `DEP_KINDS` array using the integer value diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs index 1dd510da06886..d59211f25b78f 100644 --- a/compiler/rustc_middle/src/query/on_disk_cache.rs +++ b/compiler/rustc_middle/src/query/on_disk_cache.rs @@ -22,7 +22,7 @@ use rustc_span::{ SourceFile, Span, SpanDecoder, SpanEncoder, Spanned, StableSourceFileId, Symbol, }; -use crate::dep_graph::{DepNodeIndex, QuerySideEffect, SerializedDepNodeIndex}; +use crate::dep_graph::{DepNode, DepNodeIndex, QuerySideEffect, SerializedDepNodeIndex}; use crate::mir::interpret::{AllocDecodingSession, AllocDecodingState}; use crate::mir::{self, interpret}; use crate::mono::MonoItem; @@ -330,8 +330,10 @@ impl OnDiskCache { tcx: TyCtxt<'_>, dep_node_index: SerializedDepNodeIndex, ) -> Option { + // Side-effect nodes are exempt from the `(kind, key_fingerprint)` + // uniqueness guarantee, so their values are tagged with the index. let side_effect: Option = - self.load_indexed(tcx, dep_node_index, &self.side_effects_index); + self.load_indexed(tcx, dep_node_index, &self.side_effects_index, dep_node_index); side_effect } @@ -347,24 +349,29 @@ impl OnDiskCache { &self, tcx: TyCtxt<'tcx>, dep_node_index: SerializedDepNodeIndex, + node: DepNode, ) -> Option where T: for<'a> Decodable>, { - self.load_indexed(tcx, dep_node_index, &self.query_values_index) + // See `encode_query_value` for why values are tagged with the node + // itself instead of its index. + self.load_indexed(tcx, dep_node_index, &self.query_values_index, node) } - fn load_indexed<'tcx, T>( + fn load_indexed<'tcx, T, Tag>( &self, tcx: TyCtxt<'tcx>, dep_node_index: SerializedDepNodeIndex, index: &FxHashMap, + expected_tag: Tag, ) -> Option where T: for<'a> Decodable>, + Tag: for<'a> Decodable> + Eq + fmt::Debug, { let pos = index.get(&dep_node_index).cloned()?; - let value = self.with_decoder(tcx, pos, |decoder| decode_tagged(decoder, dep_node_index)); + let value = self.with_decoder(tcx, pos, |decoder| decode_tagged(decoder, expected_tag)); Some(value) } @@ -825,11 +832,20 @@ impl<'a, 'tcx> CacheEncoder<'a, 'tcx> { ((end_pos - start_pos) as u64).encode(self); } - pub fn encode_query_value>(&mut self, index: DepNodeIndex, value: &V) { + pub fn encode_query_value>( + &mut self, + index: DepNodeIndex, + node: DepNode, + value: &V, + ) { let index = SerializedDepNodeIndex::from_curr_for_serialization(index); self.query_values_index.push((index, AbsoluteBytePos::new(self.position()))); - self.encode_tagged(index, value); + // The tag lets the load path check that the bytes at this position + // belong to the node it asked for. Unlike the index the position is + // looked up by, the node identifies the value across sessions: indices + // are reassigned per session, `(kind, key_fingerprint)` is stable. + self.encode_tagged(node, value); } fn encode_side_effect(&mut self, index: DepNodeIndex, side_effect: &QuerySideEffect) { diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index 2e121f0246b29..2b22f5fd8cf0a 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -10,7 +10,7 @@ use rustc_errors::Diag; use rustc_hir::def_id::LocalDefId; use rustc_span::Span; -use crate::dep_graph::{DepKind, DepNodeIndex, QuerySideEffect, SerializedDepNodeIndex}; +use crate::dep_graph::{DepKind, DepNode, DepNodeIndex, QuerySideEffect, SerializedDepNodeIndex}; use crate::ich::StableHashState; use crate::queries::{ExternProviders, Providers, QueryArenas, QueryVTables, TaggedQueryKey}; use crate::query::on_disk_cache::OnDiskCache; @@ -93,8 +93,11 @@ pub struct QueryVTable<'tcx, C: QueryCache> { /// Function pointer that tries to load a query value from disk. /// /// This should only be called after a successful check of [`Self::will_cache_on_disk_for_key`]. - pub try_load_from_disk_fn: - fn(tcx: TyCtxt<'tcx>, prev_index: SerializedDepNodeIndex) -> Option, + pub try_load_from_disk_fn: fn( + tcx: TyCtxt<'tcx>, + prev_index: SerializedDepNodeIndex, + node: DepNode, + ) -> Option, /// Function pointer that hashes this query's result values. /// diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index a9192d0417712..8d775e50906fc 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -520,7 +520,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>( // First try to load the result from the on-disk cache. Some things are never cached on disk. let try_value = if query.will_cache_on_disk_for_key(key) { let prof_timer = tcx.prof.incr_cache_loading(); - let value = (query.try_load_from_disk_fn)(tcx, prev_index); + let value = (query.try_load_from_disk_fn)(tcx, prev_index, *dep_node); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); value } else { diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index c53293447040b..8248ba2e0228e 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -95,7 +95,8 @@ fn encode_query_values_inner<'a, 'tcx, C, V>( assert!(all_inactive(&query.state)); query.cache.for_each(&mut |key, value, dep_node| { if query.will_cache_on_disk_for_key(*key) { - encoder.encode_query_value::(dep_node, &erase::restore_val::(*value)); + let node = DepNode::construct(tcx, query.dep_kind, key); + encoder.encode_query_value::(dep_node, node, &erase::restore_val::(*value)); } }); } @@ -169,7 +170,8 @@ pub(crate) fn promote_from_disk_inner<'tcx, C: QueryCache>( tcx.dep_graph.data().expect("should always be present in incremental mode"); let prof_timer = tcx.prof.incr_cache_loading(); - let value = ensure_sufficient_stack(|| (query.try_load_from_disk_fn)(tcx, prev_index)); + let value = + ensure_sufficient_stack(|| (query.try_load_from_disk_fn)(tcx, prev_index, dep_node)); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); let Some(value) = value else { @@ -197,6 +199,7 @@ pub(crate) fn promote_from_disk_inner<'tcx, C: QueryCache>( pub(crate) fn try_load_from_disk<'tcx, V>( tcx: TyCtxt<'tcx>, prev_index: SerializedDepNodeIndex, + node: DepNode, ) -> Option where V: for<'a> Decodable>, @@ -206,5 +209,6 @@ where // The call to `with_query_deserialization` enforces that no new `DepNodes` // are created during deserialization. See the docs of that method for more // details. - tcx.dep_graph.with_query_deserialization(|| on_disk_cache.try_load_query_value(tcx, prev_index)) + tcx.dep_graph + .with_query_deserialization(|| on_disk_cache.try_load_query_value(tcx, prev_index, node)) } diff --git a/compiler/rustc_query_impl/src/query_impl.rs b/compiler/rustc_query_impl/src/query_impl.rs index 3720d9fd80547..1e8f6f4c48d09 100644 --- a/compiler/rustc_query_impl/src/query_impl.rs +++ b/compiler/rustc_query_impl/src/query_impl.rs @@ -145,17 +145,17 @@ macro_rules! define_queries { invoke_provider_fn: self::invoke_provider_fn::__rust_begin_short_backtrace, #[cfg($cache_on_disk)] - try_load_from_disk_fn: |tcx, prev_index| { + try_load_from_disk_fn: |tcx, prev_index, node| { use rustc_middle::queries::$name::{ProvidedValue, provided_to_erased}; let loaded_value: ProvidedValue<'tcx> = - $crate::plumbing::try_load_from_disk(tcx, prev_index)?; + $crate::plumbing::try_load_from_disk(tcx, prev_index, node)?; // Arena-alloc the value if appropriate, and erase it. Some(provided_to_erased(tcx, loaded_value)) }, #[cfg(not($cache_on_disk))] - try_load_from_disk_fn: |_tcx, _prev_index| None, + try_load_from_disk_fn: |_tcx, _prev_index, _node| None, #[cfg($handle_cycle_error)] handle_cycle_error_fn: |tcx, key, cycle, err| {