From 9752d3dc8e866c16c5d31e393dbc045a7d56c349 Mon Sep 17 00:00:00 2001
From: Anders Kaseorg <andersk@mit.edu>
Date: Sat, 4 Feb 2023 16:30:21 -0800
Subject: [PATCH] Allow dereferencing a rooted Gc during collection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ordinarily, objects dropped during collections would only contain
unrooted Gc pointers.  But with `#[unsafe_ignore_trace]` it’s possible
to smuggle a rooted Gc pointer into the heap.  When we collect a
rooted Gc pointer, we need to dereference it in order to unroot its
contents.  This should be safe because the associated allocation
cannot have been collected while the Gc is rooted.

Fixes an “assertion failed: finalizer_safe()” panic in the included
test case (refs #52).

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
 gc/src/lib.rs            |  4 ++--
 gc/tests/ignore_trace.rs | 12 ++++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)
 create mode 100644 gc/tests/ignore_trace.rs

diff --git a/gc/src/lib.rs b/gc/src/lib.rs
index b38e4ba..957d12f 100644
--- a/gc/src/lib.rs
+++ b/gc/src/lib.rs
@@ -141,12 +141,12 @@ impl<T: ?Sized> Gc<T> {
     #[inline]
     fn inner_ptr(&self) -> *mut GcBox<T> {
         // If we are currently in the dropping phase of garbage collection,
-        // it would be undefined behavior to dereference this pointer.
+        // it would be undefined behavior to dereference an unrooted Gc.
         // By opting into `Trace` you agree to not dereference this pointer
         // within your drop method, meaning that it should be safe.
         //
         // This assert exists just in case.
-        assert!(finalizer_safe());
+        assert!(finalizer_safe() || self.rooted());
 
         unsafe { clear_root_bit(self.ptr_root.get()).as_ptr() }
     }
diff --git a/gc/tests/ignore_trace.rs b/gc/tests/ignore_trace.rs
new file mode 100644
index 0000000..5fe0e69
--- /dev/null
+++ b/gc/tests/ignore_trace.rs
@@ -0,0 +1,12 @@
+use gc::{force_collect, Finalize, Gc, Trace};
+
+#[derive(Finalize, Trace)]
+struct S(#[unsafe_ignore_trace] Gc<()>);
+
+/// Using `#[unsafe_ignore_trace]` on a `Gc` may inhibit collection of
+/// cycles through that `Gc`, but it should not result in panics.
+#[test]
+fn ignore_trace_gc() {
+    Gc::new(S(Gc::new(())));
+    force_collect();
+}