about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAaron Hill <aa1ronham@gmail.com>2021-03-11 01:12:07 -0500
committerAaron Hill <aa1ronham@gmail.com>2021-03-13 12:00:38 -0500
commit7d7c81a114eac8054d9e7e1d238ad05b116f12f1 (patch)
tree5aa14285869317f3f746d1bb7c44f3f20198ce6e
parent066f01d81bfbed746f6b6cf27a0426d829e8e832 (diff)
downloadrust-7d7c81a114eac8054d9e7e1d238ad05b116f12f1.tar.gz
rust-7d7c81a114eac8054d9e7e1d238ad05b116f12f1.zip
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.
-rw-r--r--compiler/rustc_query_system/src/query/plumbing.rs29
1 files 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 @@ where
         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 @@ where
 
         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<CTX, K, V: Debug>(
     tcx: CTX::DepContext,
     result: &V,