From 242f94d10cd2f786e5b7214af42e2151231093ed Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Fri, 23 Aug 2024 19:28:35 +0100 Subject: [PATCH 1/3] Simplify GC_free implementation Retrieving the base pointer for memaligned values is no longer necessary since an upstream fix in BDWGC landed (f9a0ee08). --- library/std/src/gc.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/library/std/src/gc.rs b/library/std/src/gc.rs index 7af0e547c25d5..108fcd4a9c5ff 100644 --- a/library/std/src/gc.rs +++ b/library/std/src/gc.rs @@ -144,15 +144,9 @@ unsafe fn gc_realloc(ptr: *mut u8, old_layout: Layout, new_size: usize) -> *mut } #[inline] -unsafe fn gc_free(ptr: *mut u8, layout: Layout) { - if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() { - unsafe { - bdwgc::GC_free(ptr); - } - } else { - unsafe { - bdwgc::GC_free(bdwgc::GC_base(ptr)); - } +unsafe fn gc_free(ptr: *mut u8, _: Layout) { + unsafe { + bdwgc::GC_free(ptr); } } From ada825d97caeb7c89c88c59f013522ab8ff8cfb0 Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Fri, 23 Aug 2024 19:28:35 +0100 Subject: [PATCH 2/3] Remove use of old POSIX TLS API --- library/bdwgc/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/bdwgc/src/lib.rs b/library/bdwgc/src/lib.rs index dbb732a1e8d78..9d22ddbf057ab 100644 --- a/library/bdwgc/src/lib.rs +++ b/library/bdwgc/src/lib.rs @@ -83,10 +83,6 @@ extern "C" { pub fn GC_set_warn_proc(level: *mut u8); - pub fn GC_tls_rootset() -> *mut u8; - - pub fn GC_init_tls_rootset(rootset: *mut u8); - pub fn GC_ignore_warn_proc(proc: *mut u8, word: usize); pub fn GC_finalized_total() -> u64; From 29a6c203da98f7ab73ae99b7b0e1e3e010a871b2 Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Fri, 23 Aug 2024 19:28:35 +0100 Subject: [PATCH 3/3] Switch to using BDWGC's GC_finalize API --- library/bdwgc/build.rs | 1 - library/bdwgc/src/lib.rs | 2 -- library/std/src/gc.rs | 72 +++++++++++++++------------------------- src/bdwgc | 2 +- 4 files changed, 27 insertions(+), 50 deletions(-) diff --git a/library/bdwgc/build.rs b/library/bdwgc/build.rs index d95da26bfd721..eda1b3d39b4a8 100644 --- a/library/bdwgc/build.rs +++ b/library/bdwgc/build.rs @@ -32,7 +32,6 @@ fn main() { .pic(true) .define("BUILD_SHARED_LIBS", "OFF") .cflag("-DGC_ALWAYS_MULTITHREADED") - .cflag("-DBUFFERED_FINALIZATION") .cflag("-DGC_JAVA_FINALIZATION"); if env::var("ENABLE_GC_ASSERTIONS").map_or(false, |v| v == "true") { diff --git a/library/bdwgc/src/lib.rs b/library/bdwgc/src/lib.rs index 9d22ddbf057ab..e32dcca593223 100644 --- a/library/bdwgc/src/lib.rs +++ b/library/bdwgc/src/lib.rs @@ -34,8 +34,6 @@ pub struct ProfileStats { extern "C" { pub fn GC_malloc(nbytes: usize) -> *mut u8; - pub fn GC_buffered_finalize_malloc(nbytes: usize) -> *mut u8; - pub fn GC_posix_memalign(mem_ptr: *mut *mut u8, align: usize, nbytes: usize) -> i32; pub fn GC_realloc(old: *mut u8, new_size: usize) -> *mut u8; diff --git a/library/std/src/gc.rs b/library/std/src/gc.rs index 108fcd4a9c5ff..7d1773104e560 100644 --- a/library/std/src/gc.rs +++ b/library/std/src/gc.rs @@ -44,7 +44,7 @@ use core::{ fmt, hash::{Hash, Hasher}, marker::Unsize, - mem::{align_of_val_raw, transmute, MaybeUninit}, + mem::{align_of_val_raw, MaybeUninit}, ops::{CoerceUnsized, Deref, DispatchFromDyn, Receiver}, ptr::{self, drop_in_place, NonNull}, }; @@ -72,25 +72,6 @@ pub const MIN_ALIGN: usize = 8; #[derive(Debug)] pub struct GcAllocator; -#[derive(Debug)] -pub struct GcFinalizedAllocator; - -unsafe impl Allocator for GcFinalizedAllocator { - #[inline] - fn allocate(&self, layout: Layout) -> Result, AllocError> { - match layout.size() { - 0 => Ok(NonNull::slice_from_raw_parts(layout.dangling(), 0)), - size => unsafe { - let ptr = bdwgc::GC_buffered_finalize_malloc(layout.size()) as *mut u8; - let ptr = NonNull::new(ptr).ok_or(AllocError)?; - Ok(NonNull::slice_from_raw_parts(ptr, size)) - }, - } - } - - unsafe fn deallocate(&self, _: NonNull, _: Layout) {} -} - unsafe impl GlobalAlloc for GcAllocator { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { @@ -192,21 +173,11 @@ pub fn thread_registered() -> bool { unsafe { bdwgc::GC_thread_is_registered() != 0 } } -// The function pointer to be executed on the finalization thread. -// -// This is not polymorphic because type information about the object inside the box is not known -// during collection. However, it is enough to use the () type because we ensure that during -// allocation that it points to the correct drop_in_place fn for the underlying value. -type Finalizer = unsafe fn(*mut GcBox<()>); - //////////////////////////////////////////////////////////////////////////////// // GC API //////////////////////////////////////////////////////////////////////////////// struct GcBox { - /// The finalizer fn pointer for `GcBox`. `None` if needs_finalize == false. - #[allow(dead_code)] - finalizer: Option, /// The object being garbage collected. value: T, } @@ -411,27 +382,36 @@ impl Gc { unsafe fn new_internal(value: T) -> Self { #[cfg(not(bootstrap))] if !crate::mem::needs_finalizer::() { - return Self::from_inner( - Box::leak(Box::new_in(GcBox { finalizer: None, value }, GcAllocator)).into(), - ); + return Self::from_inner(Box::leak(Box::new_in(GcBox { value }, GcAllocator)).into()); + } + + unsafe extern "C" fn finalizer_shim(obj: *mut u8, _: *mut u8) { + let drop_fn = drop_in_place::>; + drop_fn(obj as *mut GcBox); } // By explicitly using type parameters here, we force rustc to compile monomorphised drop // glue for `GcBox`. This ensures that the fn pointer points to the correct drop method - // (or chain of drop methods) for the type `T`. After this, it's safe to cast it to the - // generic function pointer `Finalizer` and then pass that around inside the collector where - // the type of the object is unknown. + // (or chain of drop methods) for the type `T`. // - // Note that we reify a `drop_in_place` for `GcBox` here and not just `T` -- even though - // `GcBox` has no drop implementation! This is because `T` is stored at some offset inside - // `GcBox`, and doing it this way means that we don't have to manually add these offsets - // later when we call the finaliser. - let drop_fn = drop_in_place::> as unsafe fn(_); - let tagged = transmute::<_, usize>(drop_fn) | 0x1; - let finalizer = Some(transmute::(tagged)); - Self::from_inner( - Box::leak(Box::new_in(GcBox { finalizer, value }, GcFinalizedAllocator)).into(), - ) + // Note that even though `GcBox` has no drop implementation, we still reify a + // `drop_in_place` for `GcBox`, and not`T`. This is because `T` may have an alignment + // such that it is stored at some offset inside `GcBox`. Calling `drop_in_place::>` + // will therefore generates drop glue for `T` which offsets the object pointer by the + // required amount of padding for `T` if necessary. If we did not do this, we'd have to + // manually ensure that the object pointer is correctly offset before the collector calls + // the finaliser. + let ptr = Box::leak(Box::new_in(GcBox { value }, GcAllocator)); + unsafe { + bdwgc::GC_register_finalizer_no_order( + ptr as *mut _ as *mut u8, + Some(finalizer_shim::), + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + ); + } + Self::from_inner(ptr.into()) } } diff --git a/src/bdwgc b/src/bdwgc index 5f21e15f028cf..0da451d7a9bee 160000 --- a/src/bdwgc +++ b/src/bdwgc @@ -1 +1 @@ -Subproject commit 5f21e15f028cf92cdbcfc659b76fb9efe78fbd1c +Subproject commit 0da451d7a9bee23ce839a2f7e203afc97a77ad83