Remove cast ref to mut everywhere (#893)

- MMTK type now stores `Plan` inside `UnsafeCell` to allow safe mutable
access when necessary
- Various types that previously had `mut_self()` method now are
separated into `Type` and `TypeInner`, second one is stored inside
`Type` behind `UnsafeCell` which allows to obtain mutable access when
necessary without involving UB.
- `&'static` references that were previously stored in some types are
now converted into `NonNull<>`

---------

Co-authored-by: Yi Lin <[email protected]>

.github/scripts/ci-style.sh

@@ -1,6 +1,6 @@
 . $(dirname "$0")/ci-common.sh
 
-export RUSTFLAGS="-D warnings"
+export RUSTFLAGS="-D warnings -A unknown-lints"
 
 # --- Check main crate ---
 

src/memory_manager.rs

@@ -88,7 +88,12 @@ pub fn mmtk_init<VM: VMBinding>(builder: &MMTKBuilder) -> Box<MMTK<VM>> {
 
 #[cfg(feature = "vm_space")]
 pub fn lazy_init_vm_space<VM: VMBinding>(mmtk: &'static mut MMTK<VM>, start: Address, size: usize) {
-    mmtk.plan.base_mut().vm_space.lazy_initialize(start, size);
+    unsafe {
+        mmtk.get_plan_mut()
+            .base_mut()
+            .vm_space
+            .lazy_initialize(start, size);
+    }
 }
 
 /// Request MMTk to create a mutator for the given thread. The ownership
@@ -345,7 +350,7 @@ pub fn get_allocator_mapping<VM: VMBinding>(
     mmtk: &MMTK<VM>,
     semantics: AllocationSemantics,
 ) -> AllocatorSelector {
-    mmtk.plan.get_allocator_mapping()[semantics]
+    mmtk.get_plan().get_allocator_mapping()[semantics]
 }
 
 /// The standard malloc. MMTk either uses its own allocator, or forward the call to a
@@ -467,11 +472,14 @@ pub fn start_worker<VM: VMBinding>(
 ///   Collection::spawn_gc_thread() so that the VM knows the context.
 pub fn initialize_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>, tls: VMThread) {
     assert!(
-        !mmtk.plan.is_initialized(),
+        !mmtk.get_plan().is_initialized(),
         "MMTk collection has been initialized (was initialize_collection() already called before?)"
     );
     mmtk.scheduler.spawn_gc_threads(mmtk, tls);
-    mmtk.plan.base().initialized.store(true, Ordering::SeqCst);
+    mmtk.get_plan()
+        .base()
+        .initialized
+        .store(true, Ordering::SeqCst);
     probe!(mmtk, collection_initialized);
 }
 
@@ -483,10 +491,10 @@ pub fn initialize_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>, tls: VMThre
 /// * `mmtk`: A reference to an MMTk instance.
 pub fn enable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>) {
     debug_assert!(
-        !mmtk.plan.should_trigger_gc_when_heap_is_full(),
+        !mmtk.get_plan().should_trigger_gc_when_heap_is_full(),
         "enable_collection() is called when GC is already enabled."
     );
-    mmtk.plan
+    mmtk.get_plan()
         .base()
         .trigger_gc_when_heap_is_full
         .store(true, Ordering::SeqCst);
@@ -504,10 +512,10 @@ pub fn enable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>) {
 /// * `mmtk`: A reference to an MMTk instance.
 pub fn disable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>) {
     debug_assert!(
-        mmtk.plan.should_trigger_gc_when_heap_is_full(),
+        mmtk.get_plan().should_trigger_gc_when_heap_is_full(),
         "disable_collection() is called when GC is not enabled."
     );
-    mmtk.plan
+    mmtk.get_plan()
         .base()
         .trigger_gc_when_heap_is_full
         .store(false, Ordering::SeqCst);
@@ -538,7 +546,7 @@ pub fn process_bulk(builder: &mut MMTKBuilder, options: &str) -> bool {
 /// Arguments:
 /// * `mmtk`: A reference to an MMTk instance.
 pub fn used_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
-    mmtk.plan.get_used_pages() << LOG_BYTES_IN_PAGE
+    mmtk.get_plan().get_used_pages() << LOG_BYTES_IN_PAGE
 }
 
 /// Return free memory in bytes. MMTk accounts for memory in pages, thus this method always returns a value in
@@ -547,7 +555,7 @@ pub fn used_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
 /// Arguments:
 /// * `mmtk`: A reference to an MMTk instance.
 pub fn free_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
-    mmtk.plan.get_free_pages() << LOG_BYTES_IN_PAGE
+    mmtk.get_plan().get_free_pages() << LOG_BYTES_IN_PAGE
 }
 
 /// Return the size of all the live objects in bytes in the last GC. MMTk usually accounts for memory in pages.
@@ -558,7 +566,7 @@ pub fn free_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
 /// to call this method is at the end of a GC (e.g. when the runtime is about to resume threads).
 #[cfg(feature = "count_live_bytes_in_gc")]
 pub fn live_bytes_in_last_gc<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
-    mmtk.plan
+    mmtk.get_plan()
         .base()
         .live_bytes_in_last_gc
         .load(Ordering::SeqCst)
@@ -581,7 +589,7 @@ pub fn last_heap_address() -> Address {
 /// Arguments:
 /// * `mmtk`: A reference to an MMTk instance.
 pub fn total_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
-    mmtk.plan.get_total_pages() << LOG_BYTES_IN_PAGE
+    mmtk.get_plan().get_total_pages() << LOG_BYTES_IN_PAGE
 }
 
 /// Trigger a garbage collection as requested by the user.
@@ -590,7 +598,8 @@ pub fn total_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
 /// * `mmtk`: A reference to an MMTk instance.
 /// * `tls`: The thread that triggers this collection request.
 pub fn handle_user_collection_request<VM: VMBinding>(mmtk: &MMTK<VM>, tls: VMMutatorThread) {
-    mmtk.plan.handle_user_collection_request(tls, false, false);
+    mmtk.get_plan()
+        .handle_user_collection_request(tls, false, false);
 }
 
 /// Is the object alive?
@@ -709,7 +718,7 @@ pub fn is_mapped_address(address: Address) -> bool {
 /// * `mmtk`: A reference to an MMTk instance.
 /// * `object`: The object to check.
 pub fn modify_check<VM: VMBinding>(mmtk: &MMTK<VM>, object: ObjectReference) {
-    mmtk.plan.modify_check(object);
+    mmtk.get_plan().modify_check(object);
 }
 
 /// Add a reference to the list of weak references. A binding may

src/mmtk.rs

@@ -15,6 +15,7 @@ use crate::util::reference_processor::ReferenceProcessors;
 use crate::util::sanity::sanity_checker::SanityChecker;
 use crate::vm::ReferenceGlue;
 use crate::vm::VMBinding;
+use std::cell::UnsafeCell;
 use std::default::Default;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
@@ -30,10 +31,10 @@ lazy_static! {
     // TODO: We should refactor this when we know more about how multiple MMTK instances work.
 
     /// A global VMMap that manages the mapping of spaces to virtual memory ranges.
-    pub static ref VM_MAP: Box<dyn VMMap> = layout::create_vm_map();
+    pub static ref VM_MAP: Box<dyn VMMap + Send + Sync> = layout::create_vm_map();
 
     /// A global Mmapper for mmaping and protection of virtual memory.
-    pub static ref MMAPPER: Box<dyn Mmapper> = layout::create_mmapper();
+    pub static ref MMAPPER: Box<dyn Mmapper + Send + Sync> = layout::create_mmapper();
 }
 
 use crate::util::rust_util::InitializeOnce;
@@ -88,7 +89,7 @@ impl Default for MMTKBuilder {
 /// *Note that multi-instances is not fully supported yet*
 pub struct MMTK<VM: VMBinding> {
     pub(crate) options: Arc<Options>,
-    pub(crate) plan: Box<dyn Plan<VM = VM>>,
+    pub(crate) plan: UnsafeCell<Box<dyn Plan<VM = VM>>>,
     pub(crate) reference_processors: ReferenceProcessors,
     pub(crate) finalizable_processor:
         Mutex<FinalizableProcessor<<VM::VMReferenceGlue as ReferenceGlue<VM>>::FinalizableType>>,
@@ -100,6 +101,9 @@ pub struct MMTK<VM: VMBinding> {
     inside_harness: AtomicBool,
 }
 
+unsafe impl<VM: VMBinding> Sync for MMTK<VM> {}
+unsafe impl<VM: VMBinding> Send for MMTK<VM> {}
+
 impl<VM: VMBinding> MMTK<VM> {
     pub fn new(options: Arc<Options>) -> Self {
         // Initialize SFT first in case we need to use this in the constructor.
@@ -136,7 +140,7 @@ impl<VM: VMBinding> MMTK<VM> {
 
         MMTK {
             options,
-            plan,
+            plan: UnsafeCell::new(plan),
             reference_processors: ReferenceProcessors::new(),
             finalizable_processor: Mutex::new(FinalizableProcessor::<
                 <VM::VMReferenceGlue as ReferenceGlue<VM>>::FinalizableType,
@@ -152,20 +156,31 @@ impl<VM: VMBinding> MMTK<VM> {
 
     pub fn harness_begin(&self, tls: VMMutatorThread) {
         probe!(mmtk, harness_begin);
-        self.plan.handle_user_collection_request(tls, true, true);
+        self.get_plan()
+            .handle_user_collection_request(tls, true, true);
         self.inside_harness.store(true, Ordering::SeqCst);
-        self.plan.base().stats.start_all();
+        self.get_plan().base().stats.start_all();
         self.scheduler.enable_stat();
     }
 
     pub fn harness_end(&'static self) {
-        self.plan.base().stats.stop_all(self);
+        self.get_plan().base().stats.stop_all(self);
         self.inside_harness.store(false, Ordering::SeqCst);
         probe!(mmtk, harness_end);
     }
 
     pub fn get_plan(&self) -> &dyn Plan<VM = VM> {
-        self.plan.as_ref()
+        unsafe { &**(self.plan.get()) }
+    }
+
+    /// Get the plan as mutable reference.
+    ///
+    /// # Safety
+    ///
+    /// This is unsafe because the caller must ensure that the plan is not used by other threads.
+    #[allow(clippy::mut_from_ref)]
+    pub unsafe fn get_plan_mut(&self) -> &mut dyn Plan<VM = VM> {
+        &mut **(self.plan.get())
     }
 
     pub fn get_options(&self) -> &Options {

src/plan/generational/copying/mutator.rs

@@ -32,16 +32,19 @@ pub fn create_gencopy_mutator<VM: VMBinding>(
     mutator_tls: VMMutatorThread,
     mmtk: &'static MMTK<VM>,
 ) -> Mutator<VM> {
-    let gencopy = mmtk.plan.downcast_ref::<GenCopy<VM>>().unwrap();
+    let gencopy = mmtk.get_plan().downcast_ref::<GenCopy<VM>>().unwrap();
     let config = MutatorConfig {
         allocator_mapping: &ALLOCATOR_MAPPING,
-        space_mapping: Box::new(create_gen_space_mapping(&*mmtk.plan, &gencopy.gen.nursery)),
+        space_mapping: Box::new(create_gen_space_mapping(
+            mmtk.get_plan(),
+            &gencopy.gen.nursery,
+        )),
         prepare_func: &gencopy_mutator_prepare,
         release_func: &gencopy_mutator_release,
     };
 
     Mutator {
-        allocators: Allocators::<VM>::new(mutator_tls, &*mmtk.plan, &config.space_mapping),
+        allocators: Allocators::<VM>::new(mutator_tls, mmtk.get_plan(), &config.space_mapping),
         barrier: Box::new(ObjectBarrier::new(GenObjectBarrierSemantics::new(
             mmtk, gencopy,
         ))),

src/plan/generational/gc_work.rs

@@ -102,7 +102,12 @@ impl<E: ProcessEdgesWork> GCWork<E::VM> for ProcessModBuf<E> {
             );
         }
         // scan modbuf only if the current GC is a nursery GC
-        if mmtk.plan.generational().unwrap().is_current_gc_nursery() {
+        if mmtk
+            .get_plan()
+            .generational()
+            .unwrap()
+            .is_current_gc_nursery()
+        {
             // Scan objects in the modbuf and forward pointers
             let modbuf = std::mem::take(&mut self.modbuf);
             GCWork::do_work(
@@ -135,7 +140,12 @@ impl<E: ProcessEdgesWork> ProcessRegionModBuf<E> {
 impl<E: ProcessEdgesWork> GCWork<E::VM> for ProcessRegionModBuf<E> {
     fn do_work(&mut self, worker: &mut GCWorker<E::VM>, mmtk: &'static MMTK<E::VM>) {
         // Scan modbuf only if the current GC is a nursery GC
-        if mmtk.plan.generational().unwrap().is_current_gc_nursery() {
+        if mmtk
+            .get_plan()
+            .generational()
+            .unwrap()
+            .is_current_gc_nursery()
+        {
             // Collect all the entries in all the slices
             let mut edges = vec![];
             for slice in &self.modbuf {

src/plan/generational/immix/mutator.rs

@@ -30,16 +30,19 @@ pub fn create_genimmix_mutator<VM: VMBinding>(
     mutator_tls: VMMutatorThread,
     mmtk: &'static MMTK<VM>,
 ) -> Mutator<VM> {
-    let genimmix = mmtk.plan.downcast_ref::<GenImmix<VM>>().unwrap();
+    let genimmix = mmtk.get_plan().downcast_ref::<GenImmix<VM>>().unwrap();
     let config = MutatorConfig {
         allocator_mapping: &ALLOCATOR_MAPPING,
-        space_mapping: Box::new(create_gen_space_mapping(&*mmtk.plan, &genimmix.gen.nursery)),
+        space_mapping: Box::new(create_gen_space_mapping(
+            mmtk.get_plan(),
+            &genimmix.gen.nursery,
+        )),
         prepare_func: &genimmix_mutator_prepare,
         release_func: &genimmix_mutator_release,
     };
 
     Mutator {
-        allocators: Allocators::<VM>::new(mutator_tls, &*mmtk.plan, &config.space_mapping),
+        allocators: Allocators::<VM>::new(mutator_tls, mmtk.get_plan(), &config.space_mapping),
         barrier: Box::new(ObjectBarrier::new(GenObjectBarrierSemantics::new(
             mmtk, genimmix,
         ))),

src/plan/global.rs

@@ -40,9 +40,9 @@ pub fn create_mutator<VM: VMBinding>(
     mmtk: &'static MMTK<VM>,
 ) -> Box<Mutator<VM>> {
     Box::new(match *mmtk.options.plan {
-        PlanSelector::NoGC => crate::plan::nogc::mutator::create_nogc_mutator(tls, &*mmtk.plan),
+        PlanSelector::NoGC => crate::plan::nogc::mutator::create_nogc_mutator(tls, mmtk.get_plan()),
         PlanSelector::SemiSpace => {
-            crate::plan::semispace::mutator::create_ss_mutator(tls, &*mmtk.plan)
+            crate::plan::semispace::mutator::create_ss_mutator(tls, mmtk.get_plan())
         }
         PlanSelector::GenCopy => {
             crate::plan::generational::copying::mutator::create_gencopy_mutator(tls, mmtk)
@@ -51,14 +51,16 @@ pub fn create_mutator<VM: VMBinding>(
             crate::plan::generational::immix::mutator::create_genimmix_mutator(tls, mmtk)
         }
         PlanSelector::MarkSweep => {
-            crate::plan::marksweep::mutator::create_ms_mutator(tls, &*mmtk.plan)
+            crate::plan::marksweep::mutator::create_ms_mutator(tls, mmtk.get_plan())
+        }
+        PlanSelector::Immix => {
+            crate::plan::immix::mutator::create_immix_mutator(tls, mmtk.get_plan())
         }
-        PlanSelector::Immix => crate::plan::immix::mutator::create_immix_mutator(tls, &*mmtk.plan),
         PlanSelector::PageProtect => {
-            crate::plan::pageprotect::mutator::create_pp_mutator(tls, &*mmtk.plan)
+            crate::plan::pageprotect::mutator::create_pp_mutator(tls, mmtk.get_plan())
         }
         PlanSelector::MarkCompact => {
-            crate::plan::markcompact::mutator::create_markcompact_mutator(tls, &*mmtk.plan)
+            crate::plan::markcompact::mutator::create_markcompact_mutator(tls, mmtk.get_plan())
         }
         PlanSelector::StickyImmix => {
             crate::plan::sticky::immix::mutator::create_stickyimmix_mutator(tls, mmtk)
@@ -137,7 +139,7 @@ pub fn create_gc_worker_context<VM: VMBinding>(
     tls: VMWorkerThread,
     mmtk: &'static MMTK<VM>,
 ) -> GCWorkerCopyContext<VM> {
-    GCWorkerCopyContext::<VM>::new(tls, &*mmtk.plan, mmtk.plan.create_copy_config())
+    GCWorkerCopyContext::<VM>::new(tls, mmtk.get_plan(), mmtk.get_plan().create_copy_config())
 }
 
 /// A plan describes the global core functionality for all memory management schemes.
@@ -857,6 +859,7 @@ impl<VM: VMBinding> BasePlan<VM> {
     }
 
     #[allow(unused_variables)] // depending on the enabled features, base may not be used.
+    #[allow(clippy::needless_pass_by_ref_mut)] // depending on the enabled features, base may not be used.
     pub(crate) fn verify_side_metadata_sanity(
         &self,
         side_metadata_sanity_checker: &mut SideMetadataSanity,

src/plan/markcompact/gc_work.rs

@@ -39,7 +39,7 @@ impl<VM: VMBinding> GCWork<VM> for UpdateReferences<VM> {
     fn do_work(&mut self, _worker: &mut GCWorker<VM>, mmtk: &'static MMTK<VM>) {
         // The following needs to be done right before the second round of root scanning
         VM::VMScanning::prepare_for_roots_re_scanning();
-        mmtk.plan.base().prepare_for_stack_scanning();
+        mmtk.get_plan().base().prepare_for_stack_scanning();
         #[cfg(feature = "extreme_assertions")]
         mmtk.edge_logger.reset();
 

src/plan/sticky/immix/mutator.rs

@@ -24,12 +24,12 @@ pub fn create_stickyimmix_mutator<VM: VMBinding>(
     mutator_tls: VMMutatorThread,
     mmtk: &'static MMTK<VM>,
 ) -> Mutator<VM> {
-    let stickyimmix = mmtk.plan.downcast_ref::<StickyImmix<VM>>().unwrap();
+    let stickyimmix = mmtk.get_plan().downcast_ref::<StickyImmix<VM>>().unwrap();
     let config = MutatorConfig {
         allocator_mapping: &ALLOCATOR_MAPPING,
         space_mapping: Box::new({
             let mut vec =
-                create_space_mapping(immix::mutator::RESERVED_ALLOCATORS, true, &*mmtk.plan);
+                create_space_mapping(immix::mutator::RESERVED_ALLOCATORS, true, mmtk.get_plan());
             vec.push((AllocatorSelector::Immix(0), stickyimmix.get_immix_space()));
             vec
         }),
@@ -38,13 +38,13 @@ pub fn create_stickyimmix_mutator<VM: VMBinding>(
     };
 
     Mutator {
-        allocators: Allocators::<VM>::new(mutator_tls, &*mmtk.plan, &config.space_mapping),
+        allocators: Allocators::<VM>::new(mutator_tls, mmtk.get_plan(), &config.space_mapping),
         barrier: Box::new(ObjectBarrier::new(GenObjectBarrierSemantics::new(
             mmtk,
             stickyimmix,
         ))),
         mutator_tls,
         config,
-        plan: &*mmtk.plan,
+        plan: mmtk.get_plan(),
     }
 }

src/policy/immix/immixspace.rs

@@ -195,7 +195,7 @@ impl<VM: VMBinding> crate::policy::gc_work::PolicyTraceObject<VM> for ImmixSpace
             if Block::containing::<VM>(object).is_defrag_source() {
                 debug_assert!(self.in_defrag());
                 debug_assert!(
-                    !crate::plan::is_nursery_gc(&*worker.mmtk.plan),
+                    !crate::plan::is_nursery_gc(worker.mmtk.get_plan()),
                     "Calling PolicyTraceObject on Immix in nursery GC"
                 );
                 self.trace_object_with_opportunistic_copy(