Skip to content

Commit f3448e6

Browse files
committed
Avoiding calling queries when collecting active queries
1 parent 43f0014 commit f3448e6

File tree

10 files changed

+314
-163
lines changed

10 files changed

+314
-163
lines changed

compiler/rustc_interface/src/util.rs

+23-21
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use rustc_session::{EarlyDiagCtxt, Session, filesearch};
1818
use rustc_span::edit_distance::find_best_match_for_name;
1919
use rustc_span::edition::Edition;
2020
use rustc_span::source_map::SourceMapInputs;
21-
use rustc_span::{Symbol, sym};
21+
use rustc_span::{SessionGlobals, Symbol, sym};
2222
use rustc_target::spec::Target;
2323
use tracing::info;
2424

@@ -188,26 +188,11 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce(CurrentGcx) -> R + Send,
188188
// On deadlock, creates a new thread and forwards information in thread
189189
// locals to it. The new thread runs the deadlock handler.
190190

191-
// Get a `GlobalCtxt` reference from `CurrentGcx` as we cannot rely on having a
192-
// `TyCtxt` TLS reference here.
193-
let query_map = current_gcx2.access(|gcx| {
194-
tls::enter_context(&tls::ImplicitCtxt::new(gcx), || {
195-
tls::with(|tcx| {
196-
match QueryCtxt::new(tcx).collect_active_jobs() {
197-
Ok(query_map) => query_map,
198-
Err(_) => {
199-
// There was an unexpected error collecting all active jobs, which we need
200-
// to find cycles to break.
201-
// We want to avoid panicking in the deadlock handler, so we abort instead.
202-
eprintln!("internal compiler error: failed to get query map in deadlock handler, aborting process");
203-
process::abort();
204-
}
205-
}
206-
})
207-
})
208-
});
209-
let query_map = FromDyn::from(query_map);
191+
let current_gcx2 = current_gcx2.clone();
210192
let registry = rayon_core::Registry::current();
193+
let session_globals = rustc_span::with_session_globals(|session_globals| {
194+
session_globals as *const SessionGlobals as usize
195+
});
211196
thread::Builder::new()
212197
.name("rustc query cycle handler".to_string())
213198
.spawn(move || {
@@ -217,7 +202,24 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce(CurrentGcx) -> R + Send,
217202
// otherwise the compiler could just hang,
218203
process::abort();
219204
});
220-
break_query_cycles(query_map.into_inner(), &registry);
205+
206+
// Get a `GlobalCtxt` reference from `CurrentGcx` as we cannot rely on having a
207+
// `TyCtxt` TLS reference here.
208+
current_gcx2.access(|gcx| {
209+
tls::enter_context(&tls::ImplicitCtxt::new(gcx), || {
210+
tls::with(|tcx| {
211+
// Accessing session globals is sound as they outlive `GlobalCtxt`.
212+
// They are needed to hash query keys containing spans or symbols.
213+
let query_map = rustc_span::set_session_globals_then(unsafe { &*(session_globals as *const SessionGlobals) }, || {
214+
// Ensure there was no errors collecting all active jobs.
215+
// We need the complete map to ensure we find a cycle to break.
216+
QueryCtxt::new(tcx).collect_active_jobs().ok().expect("failed to collect active queries in deadlock handler")
217+
});
218+
break_query_cycles(query_map, &registry);
219+
})
220+
})
221+
});
222+
221223
on_panic.disable();
222224
})
223225
.unwrap();

compiler/rustc_middle/src/query/mod.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ use rustc_index::IndexVec;
3030
use rustc_lint_defs::LintId;
3131
use rustc_macros::rustc_queries;
3232
use rustc_query_system::ich::StableHashingContext;
33-
use rustc_query_system::query::{QueryCache, QueryMode, QueryState, try_get_cached};
33+
use rustc_query_system::query::{
34+
QueryCache, QueryMode, QueryStackDeferred, QueryState, try_get_cached,
35+
};
3436
use rustc_session::Limits;
3537
use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion};
3638
use rustc_session::cstore::{

compiler/rustc_middle/src/query/plumbing.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ macro_rules! define_callbacks {
488488
#[derive(Default)]
489489
pub struct QueryStates<'tcx> {
490490
$(
491-
pub $name: QueryState<$($K)*>,
491+
pub $name: QueryState<$($K)*, QueryStackDeferred<'tcx>>,
492492
)*
493493
}
494494

compiler/rustc_middle/src/values.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl<'tcx> Value<TyCtxt<'tcx>> for Representability {
8888
if info.query.dep_kind == dep_kinds::representability
8989
&& let Some(field_id) = info.query.def_id
9090
&& let Some(field_id) = field_id.as_local()
91-
&& let Some(DefKind::Field) = info.query.def_kind
91+
&& let Some(DefKind::Field) = info.query.info.def_kind
9292
{
9393
let parent_id = tcx.parent(field_id.to_def_id());
9494
let item_id = match tcx.def_kind(parent_id) {
@@ -216,7 +216,7 @@ impl<'tcx, T> Value<TyCtxt<'tcx>> for Result<T, &'_ ty::layout::LayoutError<'_>>
216216
continue;
217217
};
218218
let frame_span =
219-
frame.query.default_span(cycle[(i + 1) % cycle.len()].span);
219+
frame.query.info.default_span(cycle[(i + 1) % cycle.len()].span);
220220
if frame_span.is_dummy() {
221221
continue;
222222
}

compiler/rustc_query_impl/src/lib.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ use rustc_middle::ty::TyCtxt;
2626
use rustc_query_system::dep_graph::SerializedDepNodeIndex;
2727
use rustc_query_system::ich::StableHashingContext;
2828
use rustc_query_system::query::{
29-
CycleError, HashResult, QueryCache, QueryConfig, QueryMap, QueryMode, QueryState,
30-
get_query_incr, get_query_non_incr,
29+
CycleError, HashResult, QueryCache, QueryConfig, QueryMap, QueryMode, QueryStackDeferred,
30+
QueryState, get_query_incr, get_query_non_incr,
3131
};
3232
use rustc_query_system::{HandleCycleError, Value};
3333
use rustc_span::{ErrorGuaranteed, Span};
@@ -84,7 +84,10 @@ where
8484
}
8585

8686
#[inline(always)]
87-
fn query_state<'a>(self, qcx: QueryCtxt<'tcx>) -> &'a QueryState<Self::Key>
87+
fn query_state<'a>(
88+
self,
89+
qcx: QueryCtxt<'tcx>,
90+
) -> &'a QueryState<Self::Key, QueryStackDeferred<'tcx>>
8891
where
8992
QueryCtxt<'tcx>: 'a,
9093
{
@@ -93,7 +96,7 @@ where
9396
unsafe {
9497
&*(&qcx.tcx.query_system.states as *const QueryStates<'tcx>)
9598
.byte_add(self.dynamic.query_state)
96-
.cast::<QueryState<Self::Key>>()
99+
.cast::<QueryState<Self::Key, QueryStackDeferred<'tcx>>>()
97100
}
98101
}
99102

compiler/rustc_query_impl/src/plumbing.rs

+64-33
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
//! manage the caches, and so forth.
44
55
use std::num::NonZero;
6+
use std::sync::Arc;
67

78
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
9+
use rustc_data_structures::sync::{DynSend, DynSync};
810
use rustc_data_structures::unord::UnordMap;
911
use rustc_hashes::Hash64;
1012
use rustc_index::Idx;
@@ -24,8 +26,8 @@ use rustc_middle::ty::{self, TyCtxt};
2426
use rustc_query_system::dep_graph::{DepNodeParams, HasDepContext};
2527
use rustc_query_system::ich::StableHashingContext;
2628
use rustc_query_system::query::{
27-
QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffect, QueryStackFrame,
28-
force_query,
29+
QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffect,
30+
QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra, force_query,
2931
};
3032
use rustc_query_system::{QueryOverflow, QueryOverflowNote};
3133
use rustc_serialize::{Decodable, Encodable};
@@ -65,7 +67,9 @@ impl<'tcx> HasDepContext for QueryCtxt<'tcx> {
6567
}
6668
}
6769

68-
impl QueryContext for QueryCtxt<'_> {
70+
impl<'tcx> QueryContext for QueryCtxt<'tcx> {
71+
type QueryInfo = QueryStackDeferred<'tcx>;
72+
6973
#[inline]
7074
fn next_job_id(self) -> QueryJobId {
7175
QueryJobId(
@@ -82,7 +86,9 @@ impl QueryContext for QueryCtxt<'_> {
8286
/// Returns a query map representing active query jobs.
8387
/// It returns an incomplete map as an error if it fails
8488
/// to take locks.
85-
fn collect_active_jobs(self) -> Result<QueryMap, QueryMap> {
89+
fn collect_active_jobs(
90+
self,
91+
) -> Result<QueryMap<QueryStackDeferred<'tcx>>, QueryMap<QueryStackDeferred<'tcx>>> {
8692
let mut jobs = QueryMap::default();
8793
let mut complete = true;
8894

@@ -95,6 +101,13 @@ impl QueryContext for QueryCtxt<'_> {
95101
if complete { Ok(jobs) } else { Err(jobs) }
96102
}
97103

104+
fn lift_query_info(
105+
self,
106+
info: &QueryStackDeferred<'tcx>,
107+
) -> rustc_query_system::query::QueryStackFrameExtra {
108+
info.extract()
109+
}
110+
98111
// Interactions with on_disk_cache
99112
fn load_side_effect(
100113
self,
@@ -159,7 +172,10 @@ impl QueryContext for QueryCtxt<'_> {
159172

160173
self.sess.dcx().emit_fatal(QueryOverflow {
161174
span: info.job.span,
162-
note: QueryOverflowNote { desc: info.query.description, depth },
175+
note: QueryOverflowNote {
176+
desc: self.lift_query_info(&info.query.info).description,
177+
depth,
178+
},
163179
suggested_limit,
164180
crate_name: self.crate_name(LOCAL_CRATE),
165181
});
@@ -298,39 +314,45 @@ macro_rules! should_ever_cache_on_disk {
298314

299315
pub(crate) fn create_query_frame<
300316
'tcx,
301-
K: Copy + Key + for<'a> HashStable<StableHashingContext<'a>>,
317+
K: Copy + DynSend + DynSync + Key + for<'a> HashStable<StableHashingContext<'a>> + 'tcx,
302318
>(
303319
tcx: TyCtxt<'tcx>,
304320
do_describe: fn(TyCtxt<'tcx>, K) -> String,
305321
key: K,
306322
kind: DepKind,
307323
name: &'static str,
308-
) -> QueryStackFrame {
309-
// If reduced queries are requested, we may be printing a query stack due
310-
// to a panic. Avoid using `default_span` and `def_kind` in that case.
311-
let reduce_queries = with_reduced_queries();
312-
313-
// Avoid calling queries while formatting the description
314-
let description = ty::print::with_no_queries!(do_describe(tcx, key));
315-
let description = if tcx.sess.verbose_internals() {
316-
format!("{description} [{name:?}]")
317-
} else {
318-
description
319-
};
320-
let span = if kind == dep_graph::dep_kinds::def_span || reduce_queries {
321-
// The `def_span` query is used to calculate `default_span`,
322-
// so exit to avoid infinite recursion.
323-
None
324-
} else {
325-
Some(key.default_span(tcx))
326-
};
324+
) -> QueryStackFrame<QueryStackDeferred<'tcx>> {
327325
let def_id = key.key_as_def_id();
328-
let def_kind = if kind == dep_graph::dep_kinds::def_kind || reduce_queries {
329-
// Try to avoid infinite recursion.
330-
None
331-
} else {
332-
def_id.and_then(|def_id| def_id.as_local()).map(|def_id| tcx.def_kind(def_id))
326+
327+
let extra = move || {
328+
// If reduced queries are requested, we may be printing a query stack due
329+
// to a panic. Avoid using `default_span` and `def_kind` in that case.
330+
let reduce_queries = with_reduced_queries();
331+
332+
// Avoid calling queries while formatting the description
333+
let description = ty::print::with_no_queries!(do_describe(tcx, key));
334+
let description = if tcx.sess.verbose_internals() {
335+
format!("{description} [{name:?}]")
336+
} else {
337+
description
338+
};
339+
let span = if kind == dep_graph::dep_kinds::def_span || reduce_queries {
340+
// The `def_span` query is used to calculate `default_span`,
341+
// so exit to avoid infinite recursion.
342+
None
343+
} else {
344+
Some(key.default_span(tcx))
345+
};
346+
347+
let def_kind = if kind == dep_graph::dep_kinds::def_kind || reduce_queries {
348+
// Try to avoid infinite recursion.
349+
None
350+
} else {
351+
def_id.and_then(|def_id| def_id.as_local()).map(|def_id| tcx.def_kind(def_id))
352+
};
353+
QueryStackFrameExtra::new(description, span, def_kind)
333354
};
355+
334356
let hash = || {
335357
tcx.with_stable_hashing_context(|mut hcx| {
336358
let mut hasher = StableHasher::new();
@@ -341,7 +363,11 @@ pub(crate) fn create_query_frame<
341363
};
342364
let def_id_for_ty_in_cycle = key.def_id_for_ty_in_cycle();
343365

344-
QueryStackFrame::new(description, span, def_id, def_kind, kind, def_id_for_ty_in_cycle, hash)
366+
// SAFETY: None of the captures in `extra` have destructors that access 'tcx
367+
// as they don't have destructors.
368+
let info = unsafe { QueryStackDeferred::new(Arc::new(extra)) };
369+
370+
QueryStackFrame::new(info, kind, hash, def_id, def_id_for_ty_in_cycle)
345371
}
346372

347373
pub(crate) fn encode_query_results<'a, 'tcx, Q>(
@@ -688,7 +714,10 @@ macro_rules! define_queries {
688714
}
689715
}
690716

691-
pub(crate) fn try_collect_active_jobs<'tcx>(tcx: TyCtxt<'tcx>, qmap: &mut QueryMap) -> Option<()> {
717+
pub(crate) fn try_collect_active_jobs<'tcx>(
718+
tcx: TyCtxt<'tcx>,
719+
qmap: &mut QueryMap<QueryStackDeferred<'tcx>>,
720+
) -> Option<()> {
692721
let make_query = |tcx, key| {
693722
let kind = rustc_middle::dep_graph::dep_kinds::$name;
694723
let name = stringify!($name);
@@ -768,7 +797,9 @@ macro_rules! define_queries {
768797

769798
// These arrays are used for iteration and can't be indexed by `DepKind`.
770799

771-
const TRY_COLLECT_ACTIVE_JOBS: &[for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap) -> Option<()>] =
800+
const TRY_COLLECT_ACTIVE_JOBS: &[
801+
for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap<QueryStackDeferred<'tcx>>) -> Option<()>
802+
] =
772803
&[$(query_impl::$name::try_collect_active_jobs),*];
773804

774805
const ALLOC_SELF_PROFILE_QUERY_STRINGS: &[

compiler/rustc_query_system/src/query/config.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::hash::Hash;
66
use rustc_data_structures::fingerprint::Fingerprint;
77
use rustc_span::ErrorGuaranteed;
88

9+
use super::QueryStackFrameExtra;
910
use crate::dep_graph::{DepKind, DepNode, DepNodeParams, SerializedDepNodeIndex};
1011
use crate::error::HandleCycleError;
1112
use crate::ich::StableHashingContext;
@@ -27,7 +28,7 @@ pub trait QueryConfig<Qcx: QueryContext>: Copy {
2728
fn format_value(self) -> fn(&Self::Value) -> String;
2829

2930
// Don't use this method to access query results, instead use the methods on TyCtxt
30-
fn query_state<'a>(self, tcx: Qcx) -> &'a QueryState<Self::Key>
31+
fn query_state<'a>(self, tcx: Qcx) -> &'a QueryState<Self::Key, Qcx::QueryInfo>
3132
where
3233
Qcx: 'a;
3334

@@ -57,7 +58,7 @@ pub trait QueryConfig<Qcx: QueryContext>: Copy {
5758
fn value_from_cycle_error(
5859
self,
5960
tcx: Qcx::DepContext,
60-
cycle_error: &CycleError,
61+
cycle_error: &CycleError<QueryStackFrameExtra>,
6162
guar: ErrorGuaranteed,
6263
) -> Self::Value;
6364

0 commit comments

Comments
 (0)