Add more robust handling of nested query cycles

Author: Zoxc

compiler/rustc_middle/src/queries.rs

@@ -60,7 +60,7 @@ use rustc_data_structures::sorted_map::SortedMap;
 use rustc_data_structures::steal::Steal;
 use rustc_data_structures::svh::Svh;
 use rustc_data_structures::unord::{UnordMap, UnordSet};
-use rustc_errors::ErrorGuaranteed;
+use rustc_errors::{ErrorGuaranteed, catch_fatal_errors};
 use rustc_hir as hir;
 use rustc_hir::attrs::{EiiDecl, EiiImpl, StrippedCfgItem};
 use rustc_hir::def::{DefKind, DocLinkResMap};

compiler/rustc_middle/src/query/plumbing.rs

@@ -167,6 +167,8 @@ pub struct QuerySystem<'tcx> {
     pub extern_providers: ExternProviders,
 
     pub jobs: AtomicU64,
+
+    pub cycle_handler_nesting: Lock<u8>,
 }
 
 #[derive(Copy, Clone)]
@@ -446,6 +448,11 @@ macro_rules! define_callbacks {
                 }
             }
 
+            /// Calls `self.description` or returns a fallback if there was a fatal error
+            pub fn catch_description(&self, tcx: TyCtxt<'tcx>) -> String {
+                catch_fatal_errors(|| self.description(tcx)).unwrap_or_else(|_| format!("<error describing {}>", self.query_name()))
+            }
+
             /// Returns the default span for this query if `span` is a dummy span.
             pub fn default_span(&self, tcx: TyCtxt<'tcx>, span: Span) -> Span {
                 if !span.is_dummy() {
@@ -463,6 +470,11 @@ macro_rules! define_callbacks {
                     )*
                 }
             }
+
+            /// Calls `self.default_span` or returns `DUMMY_SP` if there was a fatal error
+            pub fn catch_default_span(&self, tcx: TyCtxt<'tcx>, span: Span) -> Span {
+                catch_fatal_errors(|| self.default_span(tcx, span)).unwrap_or(DUMMY_SP)
+            }
         }
 
         /// Holds a `QueryVTable` for each query.

compiler/rustc_query_impl/src/error.rs

@@ -79,3 +79,28 @@ pub(crate) struct Cycle {
     )]
     pub note_span: (),
 }
+
+#[derive(Subdiagnostic)]
+#[note("...when {$stack_bottom}")]
+pub(crate) struct NestedCycleBottom {
+    pub stack_bottom: String,
+}
+
+#[derive(Diagnostic)]
+#[diag("internal compiler error: query cycle when printing cycle detected")]
+pub(crate) struct NestedCycle {
+    #[primary_span]
+    pub span: Span,
+    #[subdiagnostic]
+    pub stack_bottom: NestedCycleBottom,
+    #[subdiagnostic]
+    pub cycle_stack: Vec<CycleStack>,
+    #[subdiagnostic]
+    pub stack_count: StackCount,
+    #[subdiagnostic]
+    pub cycle_usage: Option<CycleUsage>,
+    #[note(
+        "see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information"
+    )]
+    pub note_span: (),
+}

compiler/rustc_query_impl/src/execution.rs

@@ -4,7 +4,7 @@ use std::mem::ManuallyDrop;
 use rustc_data_structures::hash_table::{Entry, HashTable};
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_data_structures::sync::{DynSend, DynSync};
-use rustc_data_structures::{outline, sharded, sync};
+use rustc_data_structures::{defer, outline, sharded, sync};
 use rustc_errors::FatalError;
 use rustc_middle::dep_graph::{DepGraphData, DepNodeKey, SerializedDepNodeIndex};
 use rustc_middle::query::{
@@ -17,6 +17,7 @@ use rustc_span::{DUMMY_SP, Span};
 use tracing::warn;
 
 use crate::dep_graph::{DepNode, DepNodeIndex};
+use crate::handle_cycle_error;
 use crate::job::{QueryJobInfo, QueryJobMap, create_cycle_error, find_cycle_in_stack};
 use crate::plumbing::{current_query_job, loadable_from_disk, next_job_id, start_query};
 use crate::query_impl::for_each_query_vtable;
@@ -114,8 +115,29 @@ fn handle_cycle<'tcx, C: QueryCache>(
     key: C::Key,
     cycle: Cycle<'tcx>,
 ) -> C::Value {
-    let error = create_cycle_error(tcx, &cycle);
-    (query.handle_cycle_error_fn)(tcx, key, cycle, error)
+    let nested;
+    {
+        let mut nesting = tcx.query_system.cycle_handler_nesting.lock();
+        nested = match *nesting {
+            0 => false,
+            1 => true,
+            _ => {
+                // Don't print further nested errors to avoid cases of infinite recursion
+                tcx.dcx().delayed_bug("doubly nested cycle error").raise_fatal()
+            }
+        };
+        *nesting += 1;
+    }
+    let _guard = defer(|| *tcx.query_system.cycle_handler_nesting.lock() -= 1);
+
+    let error = create_cycle_error(tcx, &cycle, nested);
+
+    if nested {
+        // Avoid custom handlers and only use the robust `create_cycle_error` for nested cycle errors
+        handle_cycle_error::default(error)
+    } else {
+        (query.handle_cycle_error_fn)(tcx, key, cycle, error)
+    }
 }
 
 /// Guard object representing the responsibility to execute a query job and

compiler/rustc_query_impl/src/handle_cycle_error.rs

@@ -200,7 +200,7 @@ fn layout_of<'tcx>(tcx: TyCtxt<'tcx>, cycle: Cycle<'tcx>) -> &'tcx ty::layout::L
                 ControlFlow::Continue(())
             }
         },
-        || create_cycle_error(tcx, &cycle),
+        || create_cycle_error(tcx, &cycle, false),
     );
 
     let guar = diag.emit();

compiler/rustc_query_impl/src/job.rs

@@ -454,15 +454,16 @@ pub fn print_query_stack<'tcx>(
 pub(crate) fn create_cycle_error<'tcx>(
     tcx: TyCtxt<'tcx>,
     Cycle { usage, frames }: &Cycle<'tcx>,
+    nested: bool,
 ) -> Diag<'tcx> {
     assert!(!frames.is_empty());
 
-    let span = frames[0].tagged_key.default_span(tcx, frames[1 % frames.len()].span);
+    let span = frames[0].tagged_key.catch_default_span(tcx, frames[1 % frames.len()].span);
 
     let mut cycle_stack = Vec::new();
 
     use crate::error::StackCount;
-    let stack_bottom = frames[0].tagged_key.description(tcx);
+    let stack_bottom = frames[0].tagged_key.catch_description(tcx);
     let stack_count = if frames.len() == 1 {
         StackCount::Single { stack_bottom: stack_bottom.clone() }
     } else {
@@ -471,14 +472,14 @@ pub(crate) fn create_cycle_error<'tcx>(
 
     for i in 1..frames.len() {
         let frame = &frames[i];
-        let span = frame.tagged_key.default_span(tcx, frames[(i + 1) % frames.len()].span);
+        let span = frame.tagged_key.catch_default_span(tcx, frames[(i + 1) % frames.len()].span);
         cycle_stack
-            .push(crate::error::CycleStack { span, desc: frame.tagged_key.description(tcx) });
+            .push(crate::error::CycleStack { span, desc: frame.tagged_key.catch_description(tcx) });
     }
 
     let cycle_usage = usage.as_ref().map(|usage| crate::error::CycleUsage {
-        span: usage.tagged_key.default_span(tcx, usage.span),
-        usage: usage.tagged_key.description(tcx),
+        span: usage.tagged_key.catch_default_span(tcx, usage.span),
+        usage: usage.tagged_key.catch_description(tcx),
     });
 
     let is_all_def_kind = |def_kind| {
@@ -495,23 +496,36 @@ pub(crate) fn create_cycle_error<'tcx>(
         })
     };
 
-    let alias = if is_all_def_kind(DefKind::TyAlias) {
-        Some(crate::error::Alias::Ty)
-    } else if is_all_def_kind(DefKind::TraitAlias) {
-        Some(crate::error::Alias::Trait)
+    let alias = if !nested {
+        if is_all_def_kind(DefKind::TyAlias) {
+            Some(crate::error::Alias::Ty)
+        } else if is_all_def_kind(DefKind::TraitAlias) {
+            Some(crate::error::Alias::Trait)
+        } else {
+            None
+        }
     } else {
         None
     };
 
-    let cycle_diag = crate::error::Cycle {
-        span,
-        cycle_stack,
-        stack_bottom,
-        alias,
-        cycle_usage,
-        stack_count,
-        note_span: (),
-    };
-
-    tcx.sess.dcx().create_err(cycle_diag)
+    if nested {
+        tcx.sess.dcx().create_err(crate::error::NestedCycle {
+            span,
+            cycle_stack,
+            stack_bottom: crate::error::NestedCycleBottom { stack_bottom },
+            cycle_usage,
+            stack_count,
+            note_span: (),
+        })
+    } else {
+        tcx.sess.dcx().create_err(crate::error::Cycle {
+            span,
+            cycle_stack,
+            stack_bottom,
+            alias,
+            cycle_usage,
+            stack_count,
+            note_span: (),
+        })
+    }
 }

compiler/rustc_query_impl/src/lib.rs

@@ -8,7 +8,7 @@
 #![feature(try_blocks)]
 // tidy-alphabetical-end
 
-use rustc_data_structures::sync::AtomicU64;
+use rustc_data_structures::sync::{AtomicU64, Lock};
 use rustc_middle::dep_graph;
 use rustc_middle::queries::{ExternProviders, Providers};
 use rustc_middle::query::on_disk_cache::OnDiskCache;
@@ -58,6 +58,7 @@ pub fn query_system<'tcx>(
         local_providers,
         extern_providers,
         jobs: AtomicU64::new(1),
+        cycle_handler_nesting: Lock::new(0),
     }
 }
 

tests/ui/parallel-rustc/default-trait-shadow-cycle-issue-151358.rs

@@ -1,5 +1,6 @@
 // Test for #151358, assertion failed: !worker_thread.is_null()
-//~^ ERROR cycle detected when looking up span for `Default`
+//~^ ERROR internal compiler error: query cycle when printing cycle detected
+//~^^ ERROR cycle detected when getting the resolver for lowering
 //
 //@ compile-flags: -Z threads=2
 //@ compare-output-by-lines

tests/ui/parallel-rustc/default-trait-shadow-cycle-issue-151358.stderr

@@ -1,9 +1,21 @@
-error[E0391]: cycle detected when looking up span for `Default`
+error: internal compiler error: query cycle when printing cycle detected
    |
-   = note: ...which immediately requires looking up span for `Default` again
-   = note: cycle used when perform lints prior to AST lowering
+   = note: ...when getting HIR ID of `Default`
+   = note: ...which requires getting the crate HIR...
+   = note: ...which requires perform lints prior to AST lowering...
+   = note: ...which requires looking up span for `Default`...
+   = note: ...which again requires getting HIR ID of `Default`, completing the cycle
+   = note: cycle used when getting the resolver for lowering
    = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information
 
-error: aborting due to 1 previous error
+error[E0391]: cycle detected when getting the resolver for lowering
+   |
+   = note: ...which requires getting HIR ID of `Default`...
+   = note: ...which requires getting the crate HIR...
+   = note: ...which requires perform lints prior to AST lowering...
+   = note: ...which again requires getting the resolver for lowering, completing the cycle
+   = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0391`.

tests/ui/query-system/query-cycle-printing-issue-151358.rs

@@ -1,4 +1,5 @@
-//~ ERROR: cycle detected when looking up span for `Default`
+//~ ERROR: cycle when printing cycle detected
+//~^ ERROR: cycle detected
 trait Default {}
 use std::num::NonZero;
 fn main() {