refactor: make Trace an unsafe trait for boa_engine compatibility

oscars/src/collectors/mark_sweep_branded/cell.rs

@@ -64,8 +64,11 @@ impl<T: Trace> DerefMut for GcRefMut<'_, T> {
 
 impl<T: Trace> Finalize for GcRefCell<T> {}
 
-impl<T: Trace> Trace for GcRefCell<T> {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        self.inner.get_mut().trace(tracer);
+unsafe impl<T: Trace> Trace for GcRefCell<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        let val = unsafe { &*self.inner.as_ptr() };
+        unsafe {
+            val.trace(tracer);
+        }
     }
 }

oscars/src/collectors/mark_sweep_branded/ephemeron.rs

@@ -54,6 +54,6 @@ impl<'id, K: Trace, V: Trace> Copy for Ephemeron<'id, K, V> {}
 
 impl<'id, K: Trace, V: Trace> Finalize for Ephemeron<'id, K, V> {}
 
-impl<'id, K: Trace, V: Trace> Trace for Ephemeron<'id, K, V> {
-    fn trace(&mut self, _tracer: &mut Tracer) {}
+unsafe impl<'id, K: Trace, V: Trace> Trace for Ephemeron<'id, K, V> {
+    unsafe fn trace(&self, _tracer: &mut Tracer) {}
 }

oscars/src/collectors/mark_sweep_branded/gc.rs

@@ -61,8 +61,8 @@ impl<'gc, T: Trace + 'gc> Deref for Gc<'gc, T> {
 }
 
 impl<T: Trace> Finalize for Gc<'_, T> {}
-impl<T: Trace> Trace for Gc<'_, T> {
-    fn trace(&mut self, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {
+unsafe impl<T: Trace> Trace for Gc<'_, T> {
+    unsafe fn trace(&self, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {
         tracer.mark(self);
     }
 }

oscars/src/collectors/mark_sweep_branded/tests/mod.rs

@@ -6,8 +6,8 @@ struct JsObject {
     value: i32,
 }
 
-impl crate::collectors::mark_sweep_branded::Trace for JsObject {
-    fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {}
+unsafe impl crate::collectors::mark_sweep_branded::Trace for JsObject {
+    unsafe fn trace(&self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {}
 }
 impl crate::collectors::mark_sweep_branded::Finalize for JsObject {}
 

oscars/src/collectors/mark_sweep_branded/tests/uaf.rs

@@ -5,8 +5,8 @@ use core::cell::Cell;
 
 struct DetectDrop<'a>(&'a Cell<bool>);
 
-impl<'a> Trace for DetectDrop<'a> {
-    fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {}
+unsafe impl<'a> Trace for DetectDrop<'a> {
+    unsafe fn trace(&self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {}
 }
 
 impl Finalize for DetectDrop<'_> {}

oscars/src/collectors/mark_sweep_branded/trace.rs

@@ -1,5 +1,7 @@
 //! Trace and Finalize traits for the lifetime branded GC
 
+#![allow(unsafe_op_in_unsafe_fn)]
+
 use crate::{
     alloc::mempool3::PoolItem,
     collectors::mark_sweep_branded::{gc::Gc, gc_box::GcColor},
@@ -20,9 +22,13 @@ pub use crate::collectors::common::Finalize;
 /// # Safety
 ///
 /// Use `Tracer::mark` for every reachable `Gc` pointer.
-pub trait Trace {
+pub unsafe trait Trace {
     /// Marks all `Gc` pointers reachable from `self`.
-    fn trace(&mut self, tracer: &mut Tracer);
+    ///
+    /// # Safety
+    ///
+    /// See `boa_gc::Trace` for safety contract details.
+    unsafe fn trace(&self, tracer: &mut Tracer);
 }
 
 pub(crate) type TraceFn = unsafe fn(core::ptr::NonNull<u8>, &mut Tracer<'_>);
@@ -108,19 +114,19 @@ impl<'a> Tracer<'a> {
     }
 }
 
-impl<T: ?Sized> Trace for &T {
+unsafe impl<T: ?Sized> Trace for &T {
     #[inline]
-    fn trace(&mut self, _tracer: &mut Tracer) {}
+    unsafe fn trace(&self, _tracer: &mut Tracer) {}
 }
 
 // primitive + std-lib Trace impls
 
 macro_rules! empty_trace {
     ($($T:ty),* $(,)?) => {
         $(
-            impl Trace for $T {
+            unsafe impl Trace for $T {
                 #[inline]
-                fn trace(&mut self, _tracer: &mut Tracer) {}
+                unsafe fn trace(&self, _tracer: &mut Tracer) {}
             }
         )*
     };
@@ -159,122 +165,122 @@ empty_trace![
     core::num::NonZeroU128,
 ];
 
-impl<T: Trace, const N: usize> Trace for [T; N] {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        for v in self.iter_mut() {
+unsafe impl<T: Trace, const N: usize> Trace for [T; N] {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        for v in self.iter() {
             v.trace(tracer);
         }
     }
 }
 
-impl<T: Trace> Trace for Box<T> {
-    fn trace(&mut self, tracer: &mut Tracer) {
+unsafe impl<T: Trace> Trace for Box<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
         (**self).trace(tracer);
     }
 }
 
-impl<T: Trace> Trace for Option<T> {
-    fn trace(&mut self, tracer: &mut Tracer) {
+unsafe impl<T: Trace> Trace for Option<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
         if let Some(v) = self {
             v.trace(tracer);
         }
     }
 }
 
-impl<T: Trace, E: Trace> Trace for Result<T, E> {
-    fn trace(&mut self, tracer: &mut Tracer) {
+unsafe impl<T: Trace, E: Trace> Trace for Result<T, E> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
         match self {
             Ok(v) => v.trace(tracer),
             Err(e) => e.trace(tracer),
         }
     }
 }
 
-impl<T: Trace> Trace for Vec<T> {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        for v in self.iter_mut() {
+unsafe impl<T: Trace> Trace for Vec<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        for v in self.iter() {
             v.trace(tracer);
         }
     }
 }
 
-impl<T: Trace> Trace for VecDeque<T> {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        for v in self.iter_mut() {
+unsafe impl<T: Trace> Trace for VecDeque<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        for v in self.iter() {
             v.trace(tracer);
         }
     }
 }
 
-impl<T: Trace> Trace for LinkedList<T> {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        for v in self.iter_mut() {
+unsafe impl<T: Trace> Trace for LinkedList<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        for v in self.iter() {
             v.trace(tracer);
         }
     }
 }
 
-impl<T> Trace for PhantomData<T> {
+unsafe impl<T> Trace for PhantomData<T> {
     #[inline]
-    fn trace(&mut self, _tracer: &mut Tracer) {}
+    unsafe fn trace(&self, _tracer: &mut Tracer) {}
 }
 
 // Cell<Option<T>> requires T: Copy to safely read the value via Cell::get().
 // For non-Copy types, use GcRefCell instead.
-impl<T: Copy + Trace> Trace for Cell<Option<T>> {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        if let Some(mut v) = self.get() {
-            v.trace(tracer);
-        }
+unsafe impl<T: Trace + Default> Trace for Cell<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        let v = self.take();
+        v.trace(tracer);
+        self.set(v);
     }
 }
 
-impl<T: Trace> Trace for OnceCell<T> {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        if let Some(v) = self.get_mut() {
+unsafe impl<T: Trace> Trace for OnceCell<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        if let Some(v) = self.get() {
             v.trace(tracer);
         }
     }
 }
 
-impl<T: ToOwned + Trace + ?Sized> Trace for Cow<'static, T>
+unsafe impl<T: ToOwned + Trace + ?Sized> Trace for Cow<'static, T>
 where
     T::Owned: Trace,
 {
-    fn trace(&mut self, tracer: &mut Tracer) {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
         if let Cow::Owned(v) = self {
             v.trace(tracer);
         }
     }
 }
 
-impl<A: Trace> Trace for (A,) {
+unsafe impl<A: Trace> Trace for (A,) {
     #[inline]
-    fn trace(&mut self, tracer: &mut Tracer) {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
         self.0.trace(tracer);
     }
 }
 
-impl<A: Trace, B: Trace> Trace for (A, B) {
+unsafe impl<A: Trace, B: Trace> Trace for (A, B) {
     #[inline]
-    fn trace(&mut self, tracer: &mut Tracer) {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
         self.0.trace(tracer);
         self.1.trace(tracer);
     }
 }
 
-impl<A: Trace, B: Trace, C: Trace> Trace for (A, B, C) {
+unsafe impl<A: Trace, B: Trace, C: Trace> Trace for (A, B, C) {
     #[inline]
-    fn trace(&mut self, tracer: &mut Tracer) {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
         self.0.trace(tracer);
         self.1.trace(tracer);
         self.2.trace(tracer);
     }
 }
 
-impl<A: Trace, B: Trace, C: Trace, D: Trace> Trace for (A, B, C, D) {
+unsafe impl<A: Trace, B: Trace, C: Trace, D: Trace> Trace for (A, B, C, D) {
     #[inline]
-    fn trace(&mut self, tracer: &mut Tracer) {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
         self.0.trace(tracer);
         self.1.trace(tracer);
         self.2.trace(tracer);
@@ -285,27 +291,27 @@ impl<A: Trace, B: Trace, C: Trace, D: Trace> Trace for (A, B, C, D) {
 // Rc and Arc do not contain Gc pointers (they use reference counting, not GC).
 // If you need to store Gc pointers inside Rc/Arc, wrap them in a GC-allocated
 // struct instead.
-impl<T: ?Sized> Trace for rust_alloc::rc::Rc<T> {
+unsafe impl<T: ?Sized> Trace for rust_alloc::rc::Rc<T> {
     #[inline]
-    fn trace(&mut self, _tracer: &mut Tracer) {}
+    unsafe fn trace(&self, _tracer: &mut Tracer) {}
 }
 
-impl<T: ?Sized> Trace for rust_alloc::sync::Arc<T> {
+unsafe impl<T: ?Sized> Trace for rust_alloc::sync::Arc<T> {
     #[inline]
-    fn trace(&mut self, _tracer: &mut Tracer) {}
+    unsafe fn trace(&self, _tracer: &mut Tracer) {}
 }
 
-impl<K, V: Trace> Trace for BTreeMap<K, V> {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        for v in self.values_mut() {
+unsafe impl<K, V: Trace> Trace for BTreeMap<K, V> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        for v in self.values() {
             v.trace(tracer);
         }
     }
 }
 
-impl<T> Trace for BTreeSet<T> {
+unsafe impl<T> Trace for BTreeSet<T> {
     #[inline]
-    fn trace(&mut self, _tracer: &mut Tracer) {
+    unsafe fn trace(&self, _tracer: &mut Tracer) {
         // BTreeSet keys are immutable and cannot contain Gc pointers
         // that need tracing (Gc requires &mut self to trace).
     }

oscars/src/collectors/mark_sweep_branded/weak.rs

@@ -56,7 +56,7 @@ impl<'id, T: Trace + ?Sized> Clone for WeakGc<'id, T> {
 impl<'id, T: Trace + ?Sized> Copy for WeakGc<'id, T> {}
 
 impl<'id, T: Trace> Finalize for WeakGc<'id, T> {}
-impl<'id, T: Trace> Trace for WeakGc<'id, T> {
+unsafe impl<'id, T: Trace> Trace for WeakGc<'id, T> {
     // Weak references do not mark their target, upgrade() returning None after collection is the intended behaviour.
-    fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {}
+    unsafe fn trace(&self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {}
 }

oscars/src/collectors/null_collector_branded/cell.rs

@@ -61,8 +61,13 @@ impl<T: Trace> DerefMut for GcRefMut<'_, T> {
 
 impl<T: Trace> Finalize for GcRefCell<T> {}
 
-impl<T: Trace> Trace for GcRefCell<T> {
-    fn trace(&mut self, tracer: &mut Tracer) {
-        self.inner.get_mut().trace(tracer);
+unsafe impl<T: Trace> Trace for GcRefCell<T> {
+    unsafe fn trace(&self, tracer: &mut Tracer) {
+        // SAFETY: We only access the inner value for tracing and do not mutate it.
+        // The null collector's trace is a no-op, so this is safe.
+        let val = unsafe { &*self.inner.as_ptr() };
+        unsafe {
+            val.trace(tracer);
+        }
     }
 }

oscars/src/collectors/null_collector_branded/ephemeron.rs

@@ -48,6 +48,6 @@ impl<'id, K: Trace, V: Trace> Copy for Ephemeron<'id, K, V> {}
 
 impl<'id, K: Trace, V: Trace> Finalize for Ephemeron<'id, K, V> {}
 
-impl<'id, K: Trace, V: Trace> Trace for Ephemeron<'id, K, V> {
-    fn trace(&mut self, _tracer: &mut Tracer) {}
+unsafe impl<'id, K: Trace, V: Trace> Trace for Ephemeron<'id, K, V> {
+    unsafe fn trace(&self, _tracer: &mut Tracer) {}
 }

oscars/src/collectors/null_collector_branded/gc.rs

@@ -35,31 +35,36 @@ impl<'gc, T: Trace + ?Sized + 'gc> Gc<'gc, T> {
     }
 }
 
-impl<'gc, T: Trace + 'gc> Gc<'gc, T> {
+impl<'gc, T: Trace + ?Sized + 'gc> Gc<'gc, T> {
     /// Returns a shared reference to the value.
     #[inline]
     pub fn get(&self) -> &T {
         // SAFETY: `ptr` is non-null and valid for `'gc` by construction.
         unsafe { &(*self.ptr.as_ptr().as_ptr()).0.value }
     }
+
+    #[inline]
+    pub fn as_ptr(&self) -> *const T {
+        self.get() as *const T
+    }
 }
 
-impl<'gc, T: Trace + fmt::Display + 'gc> fmt::Display for Gc<'gc, T> {
+impl<'gc, T: Trace + ?Sized + fmt::Display + 'gc> fmt::Display for Gc<'gc, T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Display::fmt(self.get(), f)
     }
 }
 
-impl<'gc, T: Trace + 'gc> Deref for Gc<'gc, T> {
+impl<'gc, T: Trace + ?Sized + 'gc> Deref for Gc<'gc, T> {
     type Target = T;
     fn deref(&self) -> &T {
         self.get()
     }
 }
 
-impl<T: Trace> Finalize for Gc<'_, T> {}
-impl<T: Trace> Trace for Gc<'_, T> {
-    fn trace(&mut self, tracer: &mut crate::collectors::null_collector_branded::trace::Tracer) {
+impl<T: Trace + ?Sized> Finalize for Gc<'_, T> {}
+unsafe impl<T: Trace + ?Sized> Trace for Gc<'_, T> {
+    unsafe fn trace(&self, tracer: &mut crate::collectors::null_collector_branded::trace::Tracer) {
         tracer.mark(self);
     }
 }