Hidden field access and debugging features

.github/scripts/ci-build.sh

@@ -10,7 +10,7 @@ pushd $BINDING_PATH
             cargo build
             ;;
         release)
-            cargo build --release
+            cargo build --features extra_assert --release
             ;;
         vanilla)
             echo "Skipped for vanilla build"

mmtk/Cargo.toml

@@ -12,7 +12,7 @@ edition = "2021"
 # Metadata for the Ruby repository
 [package.metadata.ci-repos.ruby]
 repo = "mmtk/ruby" # This is used by actions/checkout, so the format is "owner/repo", not URL.
-rev = "f38ff7179b58479b2f9cdddbfdaf457cd1521bfc"
+rev = "c300b86a5260508a46348bd5d3a023a8271c6487"
 
 [lib]
 name = "mmtk_ruby"
@@ -37,7 +37,7 @@ features = ["is_mmtk_object", "object_pinning", "sticky_immix_non_moving_nursery
 
 # Uncomment the following lines to use mmtk-core from the official repository.
 git = "https://github.com/mmtk/mmtk-core.git"
-rev = "73be50d1fce1c2f6559ffbc5b7a04623ba62e6c4"
+rev = "c4fdce02274a4b3fee49318cce9f6bf3eb378ae0"
 
 # Uncomment the following line to use mmtk-core from a local repository.
 #path = "../../mmtk-core"
@@ -47,3 +47,9 @@ default = []
 
 # When moving an object, clear its original copy.
 clear_old_copy = []
+
+# Enable extra assertions in release build.  For debugging.
+extra_assert = []
+
+# Force Immix-based plans to move as many objects as possible.  For debugging.
+immix_stress_copying = ["mmtk/immix_stress_copying"]

mmtk/cbindgen.toml

@@ -24,6 +24,9 @@ typedef void* MMTk_NullableObjectReference;
 typedef uint32_t MMTk_AllocationSemantics;
 """
 
+[export]
+include = ["HiddenHeader"]
+
 [export.rename]
 "MMTKBuilder" = "MMTk_Builder"
 "RubyMutator" = "MMTk_Mutator"
@@ -42,3 +45,6 @@ typedef uint32_t MMTk_AllocationSemantics;
 "GC_THREAD_KIND_WORKER" = "MMTK_GC_THREAD_KIND_WORKER"
 "OBJREF_OFFSET" = "MMTK_OBJREF_OFFSET"
 "MIN_OBJ_ALIGN" = "MMTK_MIN_OBJ_ALIGN"
+"HiddenHeader" = "MMTk_HiddenHeader"
+"HAS_MOVED_GIVTBL" = "MMTK_HAS_MOVED_GIVTBL"
+"HIDDEN_SIZE_MASK" = "MMTK_HIDDEN_SIZE_MASK"

mmtk/src/abi.rs

@@ -1,5 +1,5 @@
 use crate::api::RubyMutator;
-use crate::{upcalls, Ruby};
+use crate::{extra_assert, upcalls, Ruby};
 use mmtk::scheduler::GCWorker;
 use mmtk::util::{Address, ObjectReference, VMMutatorThread, VMWorkerThread};
 
@@ -9,8 +9,8 @@ pub const MIN_OBJ_ALIGN: usize = 8; // Even on 32-bit machine.  A Ruby object is
 
 pub const GC_THREAD_KIND_WORKER: libc::c_int = 1;
 
-const HAS_MOVED_GIVTBL: usize = 1 << 63;
-const HIDDEN_SIZE_MASK: usize = 0x0000FFFFFFFFFFFF;
+pub const HAS_MOVED_GIVTBL: usize = 0x8000000000000000;
+pub const HIDDEN_SIZE_MASK: usize = 0x0000FFFFFFFFFFFF;
 
 // Should keep in sync with C code.
 const RUBY_FL_EXIVAR: usize = 1 << 10;
@@ -19,6 +19,42 @@ const RUBY_FL_EXIVAR: usize = 1 << 10;
 #[allow(non_camel_case_types)]
 pub struct st_table;
 
+#[repr(C)]
+pub struct HiddenHeader {
+    prefix: usize,
+}
+
+impl HiddenHeader {
+    #[inline(always)]
+    fn assert_sane(&self) {
+        extra_assert!(
+            self.prefix & !(HAS_MOVED_GIVTBL | HIDDEN_SIZE_MASK) == 0,
+            "Hidden header is corrupted: {:x}",
+            self.prefix
+        );
+    }
+
+    pub fn payload_size(&self) -> usize {
+        self.assert_sane();
+        self.prefix & HIDDEN_SIZE_MASK
+    }
+
+    pub fn has_moved_givtbl(&mut self) -> bool {
+        self.assert_sane();
+        self.prefix & HAS_MOVED_GIVTBL != 0
+    }
+
+    pub fn set_has_moved_givtbl(&mut self) {
+        self.assert_sane();
+        self.prefix |= HAS_MOVED_GIVTBL;
+    }
+
+    pub fn clear_has_moved_givtbl(&mut self) {
+        self.assert_sane();
+        self.prefix &= !HAS_MOVED_GIVTBL;
+    }
+}
+
 /// Provide convenient methods for accessing Ruby objects.
 /// TODO: Wrap C functions in `RubyUpcalls` as Rust-friendly methods.
 pub struct RubyObjectAccess {
@@ -46,32 +82,28 @@ impl RubyObjectAccess {
         self.suffix_addr() + Self::suffix_size()
     }
 
-    fn hidden_field(&self) -> Address {
-        self.obj_start()
+    fn hidden_header(&self) -> &'static HiddenHeader {
+        unsafe { self.obj_start().as_ref() }
     }
 
-    fn load_hidden_field(&self) -> usize {
-        unsafe { self.hidden_field().load::<usize>() }
+    fn hidden_header_mut(&self) -> &'static mut HiddenHeader {
+        unsafe { self.obj_start().as_mut_ref() }
     }
 
-    fn update_hidden_field<F>(&self, f: F)
-    where
-        F: FnOnce(usize) -> usize,
-    {
-        let old_value = self.load_hidden_field();
-        let new_value = f(old_value);
-        unsafe {
-            self.hidden_field().store(new_value);
-        }
+    pub fn payload_size(&self) -> usize {
+        self.hidden_header().payload_size()
     }
 
-    pub fn payload_size(&self) -> usize {
-        self.load_hidden_field() & HIDDEN_SIZE_MASK
+    pub fn has_moved_givtbl(&self) -> bool {
+        self.hidden_header_mut().has_moved_givtbl()
     }
 
-    pub fn set_payload_size(&self, size: usize) {
-        debug_assert!((size & HIDDEN_SIZE_MASK) == size);
-        self.update_hidden_field(|old| old & !HIDDEN_SIZE_MASK | size & HIDDEN_SIZE_MASK);
+    pub fn set_has_moved_givtbl(&self) {
+        self.hidden_header_mut().set_has_moved_givtbl()
+    }
+
+    pub fn clear_has_moved_givtbl(&self) {
+        self.hidden_header_mut().clear_has_moved_givtbl()
     }
 
     fn flags_field(&self) -> Address {
@@ -86,18 +118,6 @@ impl RubyObjectAccess {
         (self.load_flags() & RUBY_FL_EXIVAR) != 0
     }
 
-    pub fn has_moved_givtbl(&self) -> bool {
-        (self.load_hidden_field() & HAS_MOVED_GIVTBL) != 0
-    }
-
-    pub fn set_has_moved_givtbl(&self) {
-        self.update_hidden_field(|old| old | HAS_MOVED_GIVTBL)
-    }
-
-    pub fn clear_has_moved_givtbl(&self) {
-        self.update_hidden_field(|old| old & !HAS_MOVED_GIVTBL)
-    }
-
     pub fn prefix_size() -> usize {
         // Currently, a hidden size field of word size is placed before each object.
         OBJREF_OFFSET

mmtk/src/lib.rs

@@ -12,6 +12,7 @@ use std::thread::ThreadId;
 
 use abi::RubyUpcalls;
 use binding::{RubyBinding, RubyBindingFast, RubyBindingFastMut};
+use mmtk::util::Address;
 use mmtk::vm::slot::{SimpleSlot, UnimplementedMemorySlice};
 use mmtk::vm::VMBinding;
 use mmtk::MMTK;
@@ -133,3 +134,20 @@ pub(crate) fn set_panic_hook() {
         }
     }));
 }
+
+/// This kind of assertion is enabled if either building in debug mode or the
+/// "extra_assert" feature is enabled.
+#[macro_export]
+macro_rules! extra_assert {
+    ($($arg:tt)*) => {
+        if std::cfg!(any(debug_assertions, feature = "extra_assert")) {
+            std::assert!($($arg)*);
+        }
+    };
+}
+
+pub(crate) fn is_mmtk_object_safe(addr: Address) -> bool {
+    !addr.is_zero()
+        && addr.is_aligned_to(mmtk::util::is_mmtk_object::VO_BIT_REGION_SIZE)
+        && mmtk::memory_manager::is_mmtk_object(addr).is_some()
+}

mmtk/src/scanning.rs

@@ -1,7 +1,7 @@
 use crate::abi::GCThreadTLS;
 
 use crate::utils::ChunkedVecCollector;
-use crate::{upcalls, Ruby, RubySlot};
+use crate::{extra_assert, is_mmtk_object_safe, upcalls, Ruby, RubySlot};
 use mmtk::scheduler::{GCWork, GCWorker, WorkBucketStage};
 use mmtk::util::{ObjectReference, VMWorkerThread};
 use mmtk::vm::{ObjectTracer, RootsWorkFactory, Scanning, SlotVisitor};
@@ -27,8 +27,8 @@ impl Scanning<Ruby> for VMScanning {
         object: ObjectReference,
         object_tracer: &mut OT,
     ) {
-        debug_assert!(
-            mmtk::memory_manager::is_mmtk_object(object.to_raw_address()).is_some(),
+        extra_assert!(
+            is_mmtk_object_safe(object.to_raw_address()),
             "Not an MMTk object: {object}",
         );
         let gc_tls = unsafe { GCThreadTLS::from_vwt_check(tls) };
@@ -39,8 +39,8 @@ impl Scanning<Ruby> for VMScanning {
                 target_object,
                 if pin { " pin" } else { "" }
             );
-            debug_assert!(
-                mmtk::memory_manager::is_mmtk_object(target_object.to_raw_address()).is_some(),
+            extra_assert!(
+                is_mmtk_object_safe(target_object.to_raw_address()),
                 "Destination is not an MMTk object. Src: {object} dst: {target_object}"
             );
             let forwarded_target = object_tracer.trace_object(target_object);
@@ -173,8 +173,8 @@ impl VMScanning {
                     "(movable, but we pin it anyway)"
                 }
             );
-            debug_assert!(
-                mmtk::memory_manager::is_mmtk_object(object.to_raw_address()).is_some(),
+            extra_assert!(
+                is_mmtk_object_safe(object.to_raw_address()),
                 "Root does not point to MMTk object.  object: {object}"
             );
             buffer.push(object);

mmtk/src/weak_proc.rs

@@ -9,7 +9,7 @@ use mmtk::{
 use crate::{
     abi::{st_table, GCThreadTLS, RubyObjectAccess},
     binding::MovedGIVTblEntry,
-    upcalls,
+    extra_assert, is_mmtk_object_safe, upcalls,
     utils::AfterAll,
     Ruby,
 };
@@ -228,8 +228,8 @@ trait GlobalTableProcessingWork {
         // `hash_foreach_replace` depends on `gb_object_moved_p` which has to have the semantics
         // of `trace_object` due to the way it is used in `UPDATE_IF_MOVED`.
         let forward_object = |_worker, object: ObjectReference, _pin| {
-            debug_assert!(
-                mmtk::memory_manager::is_mmtk_object(object.to_raw_address()).is_some(),
+            extra_assert!(
+                is_mmtk_object_safe(object.to_raw_address()),
                 "{} is not an MMTk object",
                 object
             );