Skip to content

Commit

Permalink
Auto merge of rust-lang#115711 - Zoxc:freeze-cstore, r=oli-obk
Browse files Browse the repository at this point in the history
Use `FreezeLock` for `CStore`

This uses `FreezeLock` to protect the `CStore`. `FreezeReadGuard` and `FreezeWriteGuard` are changed to support a `map` operation.

r? `@oli-obk`
  • Loading branch information
bors committed Sep 10, 2023
2 parents 01ce2d0 + 1d8fdc0 commit 6645a93
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 38 deletions.
61 changes: 45 additions & 16 deletions compiler/rustc_data_structures/src/sync/freeze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{
intrinsics::likely,
marker::PhantomData,
ops::{Deref, DerefMut},
ptr::NonNull,
sync::atomic::Ordering,
};

Expand Down Expand Up @@ -79,7 +80,7 @@ impl<T> FreezeLock<T> {
} else {
Some(self.lock.read())
},
lock: self,
data: unsafe { NonNull::new_unchecked(self.data.get()) },
}
}

Expand All @@ -101,7 +102,12 @@ impl<T> FreezeLock<T> {
if self.frozen.load(Ordering::Relaxed) {
None
} else {
Some(FreezeWriteGuard { _lock_guard, lock: self, marker: PhantomData })
Some(FreezeWriteGuard {
_lock_guard,
data: unsafe { NonNull::new_unchecked(self.data.get()) },
frozen: &self.frozen,
marker: PhantomData,
})
}
}

Expand All @@ -120,52 +126,75 @@ impl<T> FreezeLock<T> {

/// A guard holding shared access to a `FreezeLock` which is in a locked state or frozen.
#[must_use = "if unused the FreezeLock may immediately unlock"]
pub struct FreezeReadGuard<'a, T> {
pub struct FreezeReadGuard<'a, T: ?Sized> {
_lock_guard: Option<ReadGuard<'a, ()>>,
lock: &'a FreezeLock<T>,
data: NonNull<T>,
}

impl<'a, T: 'a> Deref for FreezeReadGuard<'a, T> {
impl<'a, T: ?Sized + 'a> Deref for FreezeReadGuard<'a, T> {
type Target = T;
#[inline]
fn deref(&self) -> &T {
// SAFETY: If `lock` is not frozen, `_lock_guard` holds the lock to the `UnsafeCell` so
// this has shared access until the `FreezeReadGuard` is dropped. If `lock` is frozen,
// SAFETY: If the lock is not frozen, `_lock_guard` holds the lock to the `UnsafeCell` so
// this has shared access until the `FreezeReadGuard` is dropped. If the lock is frozen,
// the data cannot be modified and shared access is sound.
unsafe { &*self.lock.data.get() }
unsafe { &*self.data.as_ptr() }
}
}

impl<'a, T: ?Sized> FreezeReadGuard<'a, T> {
#[inline]
pub fn map<U: ?Sized>(this: Self, f: impl FnOnce(&T) -> &U) -> FreezeReadGuard<'a, U> {
FreezeReadGuard { data: NonNull::from(f(&*this)), _lock_guard: this._lock_guard }
}
}

/// A guard holding mutable access to a `FreezeLock` which is in a locked state or frozen.
#[must_use = "if unused the FreezeLock may immediately unlock"]
pub struct FreezeWriteGuard<'a, T> {
pub struct FreezeWriteGuard<'a, T: ?Sized> {
_lock_guard: WriteGuard<'a, ()>,
lock: &'a FreezeLock<T>,
frozen: &'a AtomicBool,
data: NonNull<T>,
marker: PhantomData<&'a mut T>,
}

impl<'a, T> FreezeWriteGuard<'a, T> {
pub fn freeze(self) -> &'a T {
self.lock.frozen.store(true, Ordering::Release);
self.frozen.store(true, Ordering::Release);

// SAFETY: This is frozen so the data cannot be modified and shared access is sound.
unsafe { &*self.lock.data.get() }
unsafe { &*self.data.as_ptr() }
}
}

impl<'a, T: ?Sized> FreezeWriteGuard<'a, T> {
#[inline]
pub fn map<U: ?Sized>(
mut this: Self,
f: impl FnOnce(&mut T) -> &mut U,
) -> FreezeWriteGuard<'a, U> {
FreezeWriteGuard {
data: NonNull::from(f(&mut *this)),
_lock_guard: this._lock_guard,
frozen: this.frozen,
marker: PhantomData,
}
}
}

impl<'a, T: 'a> Deref for FreezeWriteGuard<'a, T> {
impl<'a, T: ?Sized + 'a> Deref for FreezeWriteGuard<'a, T> {
type Target = T;
#[inline]
fn deref(&self) -> &T {
// SAFETY: `self._lock_guard` holds the lock to the `UnsafeCell` so this has shared access.
unsafe { &*self.lock.data.get() }
unsafe { &*self.data.as_ptr() }
}
}

impl<'a, T: 'a> DerefMut for FreezeWriteGuard<'a, T> {
impl<'a, T: ?Sized + 'a> DerefMut for FreezeWriteGuard<'a, T> {
#[inline]
fn deref_mut(&mut self) -> &mut T {
// SAFETY: `self._lock_guard` holds the lock to the `UnsafeCell` so this has mutable access.
unsafe { &mut *self.lock.data.get() }
unsafe { &mut *self.data.as_ptr() }
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ fn resolver_for_lowering<'tcx>(
let krate = configure_and_expand(krate, &pre_configured_attrs, &mut resolver);

// Make sure we don't mutate the cstore from here on.
tcx.untracked().cstore.leak();
tcx.untracked().cstore.freeze();

let ty::ResolverOutputs {
global_ctxt: untracked_resolutions,
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use rustc_codegen_ssa::traits::CodegenBackend;
use rustc_codegen_ssa::CodegenResults;
use rustc_data_structures::steal::Steal;
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::{
AppendOnlyIndexVec, FreezeLock, Lrc, OnceLock, RwLock, WorkerLocal,
};
use rustc_data_structures::sync::{AppendOnlyIndexVec, FreezeLock, Lrc, OnceLock, WorkerLocal};
use rustc_hir::def_id::{StableCrateId, CRATE_DEF_ID, LOCAL_CRATE};
use rustc_hir::definitions::Definitions;
use rustc_incremental::DepGraphFuture;
Expand Down Expand Up @@ -195,7 +193,7 @@ impl<'tcx> Queries<'tcx> {
self.compiler.register_lints.as_deref(),
&pre_configured_attrs,
));
let cstore = RwLock::new(Box::new(CStore::new(
let cstore = FreezeLock::new(Box::new(CStore::new(
self.codegen_backend().metadata_loader(),
stable_crate_id,
)) as _);
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_ast::expand::allocator::{alloc_error_handler_name, global_fn_name, All
use rustc_ast::{self as ast, *};
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::{MappedReadGuard, MappedWriteGuard, ReadGuard, WriteGuard};
use rustc_data_structures::sync::{FreezeReadGuard, FreezeWriteGuard};
use rustc_expand::base::SyntaxExtension;
use rustc_hir::def_id::{CrateNum, LocalDefId, StableCrateId, StableCrateIdMap, LOCAL_CRATE};
use rustc_hir::definitions::Definitions;
Expand Down Expand Up @@ -134,14 +134,14 @@ impl<'a> std::fmt::Debug for CrateDump<'a> {
}

impl CStore {
pub fn from_tcx(tcx: TyCtxt<'_>) -> MappedReadGuard<'_, CStore> {
ReadGuard::map(tcx.untracked().cstore.read(), |cstore| {
pub fn from_tcx(tcx: TyCtxt<'_>) -> FreezeReadGuard<'_, CStore> {
FreezeReadGuard::map(tcx.untracked().cstore.read(), |cstore| {
cstore.as_any().downcast_ref::<CStore>().expect("`tcx.cstore` is not a `CStore`")
})
}

pub fn from_tcx_mut(tcx: TyCtxt<'_>) -> MappedWriteGuard<'_, CStore> {
WriteGuard::map(tcx.untracked().cstore.write(), |cstore| {
pub fn from_tcx_mut(tcx: TyCtxt<'_>) -> FreezeWriteGuard<'_, CStore> {
FreezeWriteGuard::map(tcx.untracked().cstore.write(), |cstore| {
cstore.untracked_as_any().downcast_mut().expect("`tcx.cstore` is not a `CStore`")
})
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ macro_rules! provide_one {
$tcx.ensure().crate_hash($def_id.krate);
}

let cdata = rustc_data_structures::sync::MappedReadGuard::map(CStore::from_tcx($tcx), |c| {
let cdata = rustc_data_structures::sync::FreezeReadGuard::map(CStore::from_tcx($tcx), |c| {
c.get_crate_data($def_id.krate).cdata
});
let $cdata = crate::creader::CrateMetadataRef {
Expand Down Expand Up @@ -510,7 +510,7 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
crates: |tcx, ()| {
// The list of loaded crates is now frozen in query cache,
// so make sure cstore is not mutably accessed from here on.
tcx.untracked().cstore.leak();
tcx.untracked().cstore.freeze();
tcx.arena.alloc_from_iter(CStore::from_tcx(tcx).iter_crate_data().map(|(cnum, _)| cnum))
},
..*providers
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ use rustc_data_structures::profiling::SelfProfilerRef;
use rustc_data_structures::sharded::{IntoPointer, ShardedHashMap};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::steal::Steal;
use rustc_data_structures::sync::{
self, FreezeReadGuard, Lock, Lrc, MappedReadGuard, ReadGuard, WorkerLocal,
};
use rustc_data_structures::sync::{self, FreezeReadGuard, Lock, Lrc, WorkerLocal};
use rustc_data_structures::unord::UnordSet;
use rustc_errors::{
DecorateLint, DiagnosticBuilder, DiagnosticMessage, ErrorGuaranteed, MultiSpan,
Expand Down Expand Up @@ -995,8 +993,8 @@ impl<'tcx> TyCtxt<'tcx> {
/// Note that this is *untracked* and should only be used within the query
/// system if the result is otherwise tracked through queries
#[inline]
pub fn cstore_untracked(self) -> MappedReadGuard<'tcx, CrateStoreDyn> {
ReadGuard::map(self.untracked.cstore.read(), |c| &**c)
pub fn cstore_untracked(self) -> FreezeReadGuard<'tcx, CrateStoreDyn> {
FreezeReadGuard::map(self.untracked.cstore.read(), |c| &**c)
}

/// Give out access to the untracked data without any sanity checks.
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use rustc_ast::{AngleBracketedArg, Crate, Expr, ExprKind, GenericArg, GenericArg
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::intern::Interned;
use rustc_data_structures::steal::Steal;
use rustc_data_structures::sync::{Lrc, MappedReadGuard};
use rustc_data_structures::sync::{FreezeReadGuard, Lrc};
use rustc_errors::{
Applicability, DiagnosticBuilder, DiagnosticMessage, ErrorGuaranteed, SubdiagnosticMessage,
};
Expand Down Expand Up @@ -1544,7 +1544,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
))
}

fn cstore(&self) -> MappedReadGuard<'_, CStore> {
fn cstore(&self) -> FreezeReadGuard<'_, CStore> {
CStore::from_tcx(self.tcx)
}

Expand Down Expand Up @@ -1599,7 +1599,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
});

// Make sure we don't mutate the cstore from here on.
self.tcx.untracked().cstore.leak();
self.tcx.untracked().cstore.freeze();
}

fn traits_in_scope(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_session/src/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::utils::NativeLibKind;
use crate::Session;
use rustc_ast as ast;
use rustc_data_structures::owned_slice::OwnedSlice;
use rustc_data_structures::sync::{self, AppendOnlyIndexVec, FreezeLock, RwLock};
use rustc_data_structures::sync::{self, AppendOnlyIndexVec, FreezeLock};
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, StableCrateId, LOCAL_CRATE};
use rustc_hir::definitions::{DefKey, DefPath, DefPathHash, Definitions};
use rustc_span::hygiene::{ExpnHash, ExpnId};
Expand Down Expand Up @@ -258,7 +258,7 @@ pub trait CrateStore: std::fmt::Debug {
pub type CrateStoreDyn = dyn CrateStore + sync::DynSync + sync::DynSend;

pub struct Untracked {
pub cstore: RwLock<Box<CrateStoreDyn>>,
pub cstore: FreezeLock<Box<CrateStoreDyn>>,
/// Reference span for definitions.
pub source_span: AppendOnlyIndexVec<LocalDefId, Span>,
pub definitions: FreezeLock<Definitions>,
Expand Down

0 comments on commit 6645a93

Please sign in to comment.