From 7d7c81a114eac8054d9e7e1d238ad05b116f12f1 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 11 Mar 2021 01:12:07 -0500 Subject: [PATCH] Always run `incremental_verify_ich` when re-computing query results Issue #82920 showed that the kind of bugs caught by this flag have soundness implications. This causes performance regressions of up to 15.2% during incremental compilation, but this is necessary to catch miscompilations caused by bugs in query implementations. --- .../rustc_query_system/src/query/plumbing.rs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 0050670b338..fe0401e0941 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -533,7 +533,13 @@ fn load_from_disk_and_cache_in_memory( None }; - let result = if let Some(result) = result { + if let Some(result) = result { + // If `-Zincremental-verify-ich` is specified, re-hash results from + // the cache and make sure that they have the expected fingerprint. + if unlikely!(tcx.dep_context().sess().opts.debugging_opts.incremental_verify_ich) { + incremental_verify_ich(*tcx.dep_context(), &result, dep_node, dep_node_index, query); + } + result } else { // We could not load a result from the on-disk cache, so @@ -545,20 +551,21 @@ fn load_from_disk_and_cache_in_memory( prof_timer.finish_with_query_invocation_id(dep_node_index.into()); - result - }; - - // If `-Zincremental-verify-ich` is specified, re-hash results from - // the cache and make sure that they have the expected fingerprint. - if unlikely!(tcx.dep_context().sess().opts.debugging_opts.incremental_verify_ich) { + // Verify that re-running the query produced a result with the expected hash + // This catches bugs in query implementations, turning them into ICEs. + // For example, a query might sort its result by `DefId` - since `DefId`s are + // not stable across compilation sessions, the result could get up getting sorted + // in a different order when the query is re-run, even though all of the inputs + // (e.g. `DefPathHash` values) were green. + // + // See issue #82920 for an example of a miscompilation that would get turned into + // an ICE by this check incremental_verify_ich(*tcx.dep_context(), &result, dep_node, dep_node_index, query); - } - result + result + } } -#[inline(never)] -#[cold] fn incremental_verify_ich( tcx: CTX::DepContext, result: &V,