Skip to content

Commit

Permalink
Auto merge of #122227 - Zoxc:query-hash-verify, r=michaelwoerister
Browse files Browse the repository at this point in the history
Verify that query keys result in unique dep nodes

This implements checking that query keys result into unique dep nodes as mentioned in #112469.

We could do a perf check to see how expensive this is.

r? `@michaelwoerister`
  • Loading branch information
bors committed Mar 13, 2024
2 parents e61dcc7 + 55ba7a7 commit d3555f3
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 2 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,8 @@ impl Compiler {
}

self.sess.time("serialize_dep_graph", || gcx.enter(rustc_incremental::save_dep_graph));

gcx.enter(rustc_query_impl::query_key_hash_verify_all);
}

// The timer's lifetime spans the dropping of `queries`, which contains
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_query_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use rustc_span::{ErrorGuaranteed, Span};

#[macro_use]
mod plumbing;
pub use crate::plumbing::QueryCtxt;
pub use crate::plumbing::{query_key_hash_verify_all, QueryCtxt};

mod profiling_support;
pub use self::profiling_support::alloc_self_profile_query_strings;
Expand Down
50 changes: 50 additions & 0 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::rustc_middle::ty::TyEncoder;
use crate::QueryConfigRestored;
use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher};
use rustc_data_structures::sync::Lock;
use rustc_data_structures::unord::UnordMap;
use rustc_errors::DiagInner;

use rustc_index::Idx;
Expand Down Expand Up @@ -189,6 +190,16 @@ pub(super) fn encode_all_query_results<'tcx>(
}
}

pub fn query_key_hash_verify_all<'tcx>(tcx: TyCtxt<'tcx>) {
if tcx.sess().opts.unstable_opts.incremental_verify_ich || cfg!(debug_assertions) {
tcx.sess.time("query_key_hash_verify_all", || {
for verify in super::QUERY_KEY_HASH_VERIFY.iter() {
verify(tcx);
}
})
}
}

macro_rules! handle_cycle_error {
([]) => {{
rustc_query_system::HandleCycleError::Error
Expand Down Expand Up @@ -370,6 +381,34 @@ pub(crate) fn encode_query_results<'a, 'tcx, Q>(
});
}

pub(crate) fn query_key_hash_verify<'tcx>(
query: impl QueryConfig<QueryCtxt<'tcx>>,
qcx: QueryCtxt<'tcx>,
) {
let _timer =
qcx.profiler().generic_activity_with_arg("query_key_hash_verify_for", query.name());

let mut map = UnordMap::default();

let cache = query.query_cache(qcx);
cache.iter(&mut |key, _, _| {
let node = DepNode::construct(qcx.tcx, query.dep_kind(), key);
if let Some(other_key) = map.insert(node, *key) {
bug!(
"query key:\n\
`{:?}`\n\
and key:\n\
`{:?}`\n\
mapped to the same dep node:\n\
{:?}",
key,
other_key,
node
);
}
});
}

fn try_load_from_on_disk_cache<'tcx, Q>(query: Q, tcx: TyCtxt<'tcx>, dep_node: DepNode)
where
Q: QueryConfig<QueryCtxt<'tcx>>,
Expand Down Expand Up @@ -691,6 +730,13 @@ macro_rules! define_queries {
)
}
}}

pub fn query_key_hash_verify<'tcx>(tcx: TyCtxt<'tcx>) {
$crate::plumbing::query_key_hash_verify(
query_impl::$name::QueryType::config(tcx),
QueryCtxt::new(tcx),
)
}
})*}

pub(crate) fn engine(incremental: bool) -> QueryEngine {
Expand Down Expand Up @@ -730,6 +776,10 @@ macro_rules! define_queries {
>
] = &[$(expand_if_cached!([$($modifiers)*], query_impl::$name::encode_query_results)),*];

const QUERY_KEY_HASH_VERIFY: &[
for<'tcx> fn(TyCtxt<'tcx>)
] = &[$(query_impl::$name::query_key_hash_verify),*];

#[allow(nonstandard_style)]
mod query_callbacks {
use super::*;
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,9 @@ options! {
"print high-level information about incremental reuse (or the lack thereof) \
(default: no)"),
incremental_verify_ich: bool = (false, parse_bool, [UNTRACKED],
"verify incr. comp. hashes of green query instances (default: no)"),
"verify extended properties for incr. comp. (default: no):
- hashes of green query instances
- hash collisions of query keys"),
inline_in_all_cgus: Option<bool> = (None, parse_opt_bool, [TRACKED],
"control whether `#[inline]` functions are in all CGUs"),
inline_llvm: bool = (true, parse_bool, [TRACKED],
Expand Down

0 comments on commit d3555f3

Please sign in to comment.