From d78c457d4d02cf02d12bdd91ea65f3511d3d3e5f 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(); +}