From ee7a9a8641b79329ed4c221a2ae0e1e0c3d3d75d Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 4 Sep 2022 22:42:05 +0200 Subject: [PATCH] Expand hash check. --- .../rustc_query_system/src/dep_graph/graph.rs | 17 ++++++---- .../rustc_query_system/src/query/plumbing.rs | 32 +++++++++++-------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index d3d2ac25660..e44857a0238 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -513,15 +513,18 @@ impl DepGraph { hash_result: fn(&mut StableHashingContext<'_>, &R) -> Fingerprint, ) -> DepNodeIndex { if let Some(data) = self.data.as_ref() { + // The caller query has more dependencies than the node we are creating. We may + // encounter a case where this created node is marked as green, but the caller query is + // subsequently marked as red or recomputed. In this case, we will end up feeding a + // value to an existing node. + // + // For sanity, we still check that the loaded stable hash and the new one match. if let Some(dep_node_index) = self.dep_node_index_of_opt(&node) { + let _current_fingerprint = + crate::query::incremental_verify_ich(cx, result, &node, Some(hash_result)); + #[cfg(debug_assertions)] - { - let hashing_timer = cx.profiler().incr_result_hashing(); - let current_fingerprint = - cx.with_stable_hashing_context(|mut hcx| hash_result(&mut hcx, result)); - hashing_timer.finish_with_query_invocation_id(dep_node_index.into()); - data.current.record_edge(dep_node_index, node, current_fingerprint); - } + data.current.record_edge(dep_node_index, node, _current_fingerprint); return dep_node_index; } diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index f8d93a27d1c..eb5a35b3449 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -3,6 +3,7 @@ //! manage the caches, and so forth. use crate::dep_graph::{DepContext, DepNode, DepNodeIndex, DepNodeParams}; +use crate::ich::StableHashingContext; use crate::query::caches::QueryCache; use crate::query::config::QueryVTable; use crate::query::job::{report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobInfo}; @@ -525,7 +526,7 @@ where if std::intrinsics::unlikely( try_verify || qcx.dep_context().sess().opts.unstable_opts.incremental_verify_ich, ) { - incremental_verify_ich(*qcx.dep_context(), &result, dep_node, query); + incremental_verify_ich(*qcx.dep_context(), &result, dep_node, query.hash_result); } return Some((result, dep_node_index)); @@ -558,39 +559,42 @@ where // // See issue #82920 for an example of a miscompilation that would get turned into // an ICE by this check - incremental_verify_ich(*qcx.dep_context(), &result, dep_node, query); + incremental_verify_ich(*qcx.dep_context(), &result, dep_node, query.hash_result); Some((result, dep_node_index)) } -#[instrument(skip(qcx, result, query), level = "debug")] -fn incremental_verify_ich( - qcx: Qcx::DepContext, +#[instrument(skip(tcx, result, hash_result), level = "debug")] +pub(crate) fn incremental_verify_ich( + tcx: Tcx, result: &V, - dep_node: &DepNode, - query: &QueryVTable, -) where - Qcx: QueryContext, + dep_node: &DepNode, + hash_result: Option, &V) -> Fingerprint>, +) -> Fingerprint +where + Tcx: DepContext, { assert!( - qcx.dep_graph().is_green(dep_node), + tcx.dep_graph().is_green(dep_node), "fingerprint for green query instance not loaded from cache: {:?}", dep_node, ); - let new_hash = query.hash_result.map_or(Fingerprint::ZERO, |f| { - qcx.with_stable_hashing_context(|mut hcx| f(&mut hcx, result)) + let new_hash = hash_result.map_or(Fingerprint::ZERO, |f| { + tcx.with_stable_hashing_context(|mut hcx| f(&mut hcx, result)) }); - let old_hash = qcx.dep_graph().prev_fingerprint_of(dep_node); + let old_hash = tcx.dep_graph().prev_fingerprint_of(dep_node); if Some(new_hash) != old_hash { incremental_verify_ich_failed( - qcx.sess(), + tcx.sess(), DebugArg::from(&dep_node), DebugArg::from(&result), ); } + + new_hash } // This DebugArg business is largely a mirror of std::fmt::ArgumentV1, which is