Don't perform any new queries while reading a query result on disk

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -171,6 +171,62 @@ impl<K: DepKind> DepGraph<K> {
         K::with_deps(None, op)
     }
 
+    /// Used to wrap the deserialization of a query result from disk,
+    /// This method enforces that no new `DepNodes` are created during
+    /// query result deserialization.
+    ///
+    /// Enforcing this makes the query dep graph simpler - all nodes
+    /// must be created during the query execution, and should be
+    /// created from inside the 'body' of a query (the implementation
+    /// provided by a particular compiler crate).
+    ///
+    /// Consider the case of three queries `A`, `B`, and `C`, where
+    /// `A` invokes `B` and `B` invokes `C`:
+    ///
+    /// `A -> B -> C`
+    ///
+    /// Suppose that decoding the result of query `B` required re-computing
+    /// the query `C`. If we did not create a fresh `TaskDeps` when
+    /// decoding `B`, we would still be using the `TaskDeps` for query `A`
+    /// (if we needed to re-execute `A`). This would cause us to create
+    /// a new edge `A -> C`. If this edge did not previously
+    /// exist in the `DepGraph`, then we could end up with a different
+    /// `DepGraph` at the end of compilation, even if there were no
+    /// meaningful changes to the overall program (e.g. a newline was added).
+    /// In addition, this edge might cause a subsequent compilation run
+    /// to try to force `C` before marking other necessary nodes green. If
+    /// `C` did not exist in the new compilation session, then we could
+    /// get an ICE. Normally, we would have tried (and failed) to mark
+    /// some other query green (e.g. `item_children`) which was used
+    /// to obtain `C`, which would prevent us from ever trying to force
+    /// a non-existent `D`.
+    ///
+    /// It might be possible to enforce that all `DepNode`s read during
+    /// deserialization already exist in the previous `DepGraph`. In
+    /// the above example, we would invoke `D` during the deserialization
+    /// of `B`. Since we correctly create a new `TaskDeps` from the decoding
+    /// of `B`, this would result in an edge `B -> D`. If that edge already
+    /// existed (with the same `DepPathHash`es), then it should be correct
+    /// to allow the invocation of the query to proceed during deserialization
+    /// of a query result. We would merely assert that the dep-graph fragment
+    /// that would have been added by invoking `C` while decoding `B`
+    /// is equivalent to the dep-graph fragment that we already instantiated for B
+    /// (at the point where we successfully marked B as green).
+    ///
+    /// However, this would require additional complexity
+    /// in the query infrastructure, and is not currently needed by the
+    /// decoding of any query results. Should the need arise in the future,
+    /// we should consider extending the query system with this functionality.
+    pub fn with_query_deserialization<OP, R>(&self, op: OP) -> R
+    where
+        OP: FnOnce() -> R,
+    {
+        let mut deps = TaskDeps::default();
+        deps.read_allowed = false;
+        let deps = Lock::new(deps);
+        K::with_deps(Some(&deps), op)
+    }
+
     /// Starts a new dep-graph task. Dep-graph tasks are specified
     /// using a free function (`task`) and **not** a closure -- this
     /// is intentional because we want to exercise tight control over
@@ -251,6 +307,7 @@ impl<K: DepKind> DepGraph<K> {
                 reads: SmallVec::new(),
                 read_set: Default::default(),
                 phantom_data: PhantomData,
+                read_allowed: true,
             }))
         };
         let result = K::with_deps(task_deps.as_ref(), || task(cx, arg));
@@ -362,6 +419,11 @@ impl<K: DepKind> DepGraph<K> {
                 if let Some(task_deps) = task_deps {
                     let mut task_deps = task_deps.lock();
                     let task_deps = &mut *task_deps;
+
+                    if !task_deps.read_allowed {
+                        panic!("Illegal read of: {:?}", dep_node_index);
+                    }
+
                     if cfg!(debug_assertions) {
                         data.current.total_read_count.fetch_add(1, Relaxed);
                     }
@@ -1115,6 +1177,12 @@ pub struct TaskDeps<K> {
     reads: EdgesVec,
     read_set: FxHashSet<DepNodeIndex>,
     phantom_data: PhantomData<DepNode<K>>,
+    /// Whether or not we allow `DepGraph::read_index` to run.
+    /// This is normally true, except inside `with_query_deserialization`,
+    /// where it set to `false` to enforce that no new `DepNode` edges are
+    /// created. See the documentation of `with_query_deserialization` for
+    /// more details.
+    read_allowed: bool,
 }
 
 impl<K> Default for TaskDeps<K> {
@@ -1125,6 +1193,7 @@ impl<K> Default for TaskDeps<K> {
             reads: EdgesVec::new(),
             read_set: FxHashSet::default(),
             phantom_data: PhantomData,
+            read_allowed: true,
         }
     }
 }

compiler/rustc_query_system/src/query/plumbing.rs

@@ -9,7 +9,6 @@ use crate::query::job::{
     report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryShardJobId,
 };
 use crate::query::{QueryContext, QueryMap, QuerySideEffects, QueryStackFrame};
-
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::{FxHashMap, FxHasher};
 #[cfg(parallel_compiler)]
@@ -515,7 +514,13 @@ where
     // Some things are never cached on disk.
     if query.cache_on_disk {
         let prof_timer = tcx.dep_context().profiler().incr_cache_loading();
-        let result = query.try_load_from_disk(tcx, prev_dep_node_index);
+
+        // The call to `with_query_deserialization` enforces that no new `DepNodes`
+        // are created during deserialization. See the docs of that method for more
+        // details.
+        let result = dep_graph
+            .with_query_deserialization(|| query.try_load_from_disk(tcx, prev_dep_node_index));
+
         prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
         if let Some(result) = result {