diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 9d08e8b98cb31..1297e4120b905 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -16,7 +16,7 @@ use tracing::{debug, instrument}; use crate::diagnostics::{ParamKindInEnumDiscriminant, ParamKindInNonTrivialAnonConst}; use crate::hygiene::Macros20NormalizedSyntaxContext; -use crate::imports::{Import, NameResolution}; +use crate::imports::{Import, NameResolution, cycle_detection}; use crate::late::{ ConstantHasGenerics, DiagMetadata, NoConstantGenericsReason, PathSource, Rib, RibKind, }; @@ -1148,13 +1148,23 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ignore_import: Option>, ) -> Result, ControlFlow> { let key = BindingKey::new(ident, ns); - // `try_borrow_mut` is required to ensure exclusive access, even if the resulting binding - // doesn't need to be mutable. It will fail when there is a cycle of imports, and without - // the exclusive access infinite recursion will crash the compiler with stack overflow. - let resolution = &*self - .resolution_or_default(module.to_module(), key, orig_ident_span) - .try_borrow_mut_unchecked() - .map_err(|_| ControlFlow::Continue(Determined))?; + let resolution = self.resolution_or_default(module.to_module(), key, orig_ident_span); + // We need to detect resolution cycles to avoid infinite recursion. The guard ensures + // the resolution is removed when this resolve call ends. + // + // We only track resolutions that are imports, as these are the only things that + // can cause cycles during resolution. + let _cycle_guard = if let Some(decl) = resolution.borrow().best_decl() + && !decl.is_import() + { + None + } else { + Some( + cycle_detection::enter_cycle_detector(resolution) + .map_err(|_| ControlFlow::Continue(Determined))?, + ) + }; + let resolution = resolution.borrow(); let binding = resolution.non_glob_decl.filter(|b| Some(*b) != ignore_decl); @@ -1210,13 +1220,23 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ignore_import: Option>, ) -> Result, ControlFlow> { let key = BindingKey::new(ident, ns); - // `try_borrow_mut` is required to ensure exclusive access, even if the resulting binding - // doesn't need to be mutable. It will fail when there is a cycle of imports, and without - // the exclusive access infinite recursion will crash the compiler with stack overflow. - let resolution = &*self - .resolution_or_default(module.to_module(), key, orig_ident_span) - .try_borrow_mut_unchecked() - .map_err(|_| ControlFlow::Continue(Determined))?; + let resolution = self.resolution_or_default(module.to_module(), key, orig_ident_span); + // We need to detect resolution cycles to avoid infinite recursion. The guard ensures + // the resolution is removed when this resolve call ends. + // + // We only track resolutions that are imports, as these are the only things that + // can cause cycles during resolution. + let _cycle_guard = if let Some(decl) = resolution.borrow().best_decl() + && !decl.is_import() + { + None + } else { + Some( + cycle_detection::enter_cycle_detector(resolution) + .map_err(|_| ControlFlow::Continue(Determined))?, + ) + }; + let resolution = resolution.borrow(); let binding = resolution.glob_decl.filter(|b| Some(*b) != ignore_decl); diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index a3cb526e60a87..08807616d5097 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -32,7 +32,7 @@ use crate::diagnostics::{ ConsiderMarkingAsPubCrate, }; use crate::error_helper::{OnUnknownData, Suggestion}; -use crate::ref_mut::CmCell; +use crate::ref_mut::{CmCell, CmRefCell}; use crate::{ AmbiguityError, BindingKey, CmResolver, Decl, DeclData, DeclKind, Determinacy, Finalize, IdentKey, ImportSuggestion, ImportSummary, LocalModule, ModuleOrUniformRoot, ParentScope, @@ -292,6 +292,8 @@ pub(crate) struct NameResolution<'ra> { pub orig_ident_span: Span, } +pub(crate) type NameResolutionRef<'ra> = Interned<'ra, CmRefCell>>; + impl<'ra> NameResolution<'ra> { pub(crate) fn new(orig_ident_span: Span) -> Self { NameResolution { single_imports: FxIndexSet::default(), orig_ident_span, .. } @@ -320,6 +322,57 @@ impl<'ra> NameResolution<'ra> { } } +// module to keep the TLS private and only accessible through the function `enter_cycle_detector`. +pub(crate) mod cycle_detection { + use std::ptr; + + use crate::CacheRefCell; + use crate::imports::NameResolutionRef; + + thread_local!( + /// During import resolution, recursive imports can form cycles. + /// This set stores the active resolution stack for the current thread. + /// So it's essentially a recursion stack. + /// + /// The key is the interned address of a `RefCell>` allocated + /// in the `Resolver Arenas` (lifetime `'ra`), it is thus stable and allows casting + /// to a `*const ()` for comparison. This is done because we can't use lifetimes + /// other than `'static` in thread local storage. + static ACTIVE_RESOLUTIONS: CacheRefCell> = Default::default(); + ); + + pub(crate) struct ActiveResolutionGuard { + key: *const (), + } + + impl Drop for ActiveResolutionGuard { + fn drop(&mut self) { + ACTIVE_RESOLUTIONS.with_borrow_mut(|ar| { + // Only this guard is allowed to remove this key. + assert!( + Some(self.key) == ar.pop(), + "This guard should be the only one removing this key" + ); + }); + } + } + + /// Returns `Err(())` if a cycle is detected, otherwise this returns a + /// guard that will remove the resolution when dropped. + pub(crate) fn enter_cycle_detector<'ra>( + resolution: NameResolutionRef<'ra>, + ) -> Result { + let key = ptr::from_ref(resolution.0).cast::<()>(); + ACTIVE_RESOLUTIONS.with_borrow_mut(|ar| { + if ar.contains(&key) { + return Err(()); + } + ar.push(key); + Ok(ActiveResolutionGuard { key }) + }) + } +} + /// An error that may be transformed into a diagnostic later. Used to combine multiple unresolved /// import errors within the same use tree into a single diagnostic. #[derive(Debug, Clone)] @@ -640,6 +693,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let (binding, t) = { let resolution = &mut *self .resolution_or_default(module.to_module(), key, orig_ident_span) + .0 .borrow_mut(self); let old_decl = resolution.determined_decl(); let old_vis = old_decl.map(|d| d.vis()); diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 1f1884ea0cbe2..be76951572427 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -77,6 +77,7 @@ use smallvec::{SmallVec, smallvec}; use tracing::{debug, instrument}; use crate::error_helper::OnUnknownData; +use crate::imports::NameResolutionRef; use crate::ref_mut::{CmCell, CmRefCell}; mod build_reduced_graph; @@ -639,7 +640,7 @@ impl BindingKey { } } -type Resolutions<'ra> = CmRefCell>>>; +type Resolutions<'ra> = CmRefCell>>; /// One node in the tree of modules. /// @@ -1597,11 +1598,10 @@ impl<'ra> ResolverArenas<'ra> { fn alloc_import(&'ra self, import: ImportData<'ra>) -> Import<'ra> { Interned::new_unchecked(self.imports.alloc(import)) } - fn alloc_name_resolution( - &'ra self, - orig_ident_span: Span, - ) -> &'ra CmRefCell> { - self.name_resolutions.alloc(CmRefCell::new(NameResolution::new(orig_ident_span))) + fn alloc_name_resolution(&'ra self, orig_ident_span: Span) -> NameResolutionRef<'ra> { + Interned::new_unchecked( + self.name_resolutions.alloc(CmRefCell::new(NameResolution::new(orig_ident_span))), + ) } fn alloc_macro_rules_scope(&'ra self, scope: MacroRulesScope<'ra>) -> MacroRulesScopeRef<'ra> { self.dropless.alloc(CacheCell::new(scope)) @@ -2212,7 +2212,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { module: Module<'ra>, key: BindingKey, ) -> Option>> { - self.resolutions(module).borrow().get(&key).map(|resolution| resolution.borrow()) + self.resolutions(module).borrow().get(&key).map(|resolution| resolution.0.borrow()) } fn resolution_or_default( @@ -2220,8 +2220,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { module: Module<'ra>, key: BindingKey, orig_ident_span: Span, - ) -> &'ra CmRefCell> { - self.resolutions(module) + ) -> NameResolutionRef<'ra> { + *self + .resolutions(module) .borrow_mut_unchecked() .entry(key) .or_insert_with(|| self.arenas.alloc_name_resolution(orig_ident_span)) @@ -2832,8 +2833,6 @@ type CmResolver<'r, 'ra, 'tcx> = ref_mut::RefOrMut<'r, Resolver<'ra, 'tcx>>; // parallel name resolution. use std::cell::{Cell as CacheCell, RefCell as CacheRefCell}; -// FIXME: `*_unchecked` methods in the module below should be eliminated in the process -// of migration to parallel name resolution. mod ref_mut { use std::cell::{BorrowMutError, Cell, Ref, RefCell, RefMut}; use std::fmt; @@ -2941,6 +2940,8 @@ mod ref_mut { } #[track_caller] + // FIXME: this should be eliminated in the process of migration + // to parallel name resolution. pub(crate) fn borrow_mut_unchecked(&self) -> RefMut<'_, T> { self.0.borrow_mut() } @@ -2953,10 +2954,6 @@ mod ref_mut { self.0.borrow_mut() } - pub(crate) fn try_borrow_mut_unchecked(&self) -> Result, BorrowMutError> { - self.0.try_borrow_mut() - } - #[track_caller] pub(crate) fn try_borrow_mut<'ra, 'tcx>( &self,