From a08f8c196f9b45a0ba7100738ecaa76a15f7bb84 Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Thu, 7 Dec 2023 11:55:38 +0000 Subject: [PATCH 1/2] Use posix_memalign instead of memalign in GC allocator This mirrors the behaviour of Rust's default system allocator behaviour. The key difference is that posix_memalign requires that any alignment specified must be a power of two and multiple of `size_of::`. --- library/alloc/src/gc.rs | 9 ++++++++- library/boehm/src/lib.rs | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/gc.rs b/library/alloc/src/gc.rs index 5a5c2910cb60f..aa697c1088833 100644 --- a/library/alloc/src/gc.rs +++ b/library/alloc/src/gc.rs @@ -56,7 +56,14 @@ unsafe fn gc_malloc(layout: Layout) -> *mut u8 { if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() { unsafe { boehm::GC_malloc(layout.size()) as *mut u8 } } else { - unsafe { boehm::GC_memalign(layout.align(), layout.size()) as *mut u8 } + let mut out = ptr::null_mut(); + // posix_memalign requires that the alignment be a multiple of `sizeof(void*)`. + // Since these are all powers of 2, we can just use max. + unsafe { + let align = layout.align().max(core::mem::size_of::()); + let ret = boehm::GC_posix_memalign(&mut out, align, layout.size()); + if ret != 0 { ptr::null_mut() } else { out as *mut u8 } + } } } diff --git a/library/boehm/src/lib.rs b/library/boehm/src/lib.rs index f1b8c08dd30c7..6d4b25a558c7e 100644 --- a/library/boehm/src/lib.rs +++ b/library/boehm/src/lib.rs @@ -36,7 +36,7 @@ pub struct ProfileStats { extern "C" { pub fn GC_malloc(nbytes: usize) -> *mut u8; - pub fn GC_memalign(align: usize, 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; From 43b84043430a75dfafea1cedf3d1b9562dd76d17 Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Thu, 7 Dec 2023 12:00:10 +0000 Subject: [PATCH 2/2] Free using `layout` for posix_memalign'ed blocks Due to an issue in the Boehm collector [1], blocks allocated using posix_memalign must pass the base pointer (regardless of alignment) to GC_free in order to be manually freed. We use the `layout` argument to determine whether a block was allocated using posix_memalign, and if so, use Boehm's `GC_base` API to obtain the base pointer. [1]: https://github.com/ivmai/bdwgc/issues/589 --- library/alloc/src/gc.rs | 12 +++++++++--- library/alloc/tests/gc.rs | 29 +++++++++++++++++++++++++++++ library/boehm/src/lib.rs | 2 ++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/gc.rs b/library/alloc/src/gc.rs index aa697c1088833..a7f9e1e124ab7 100644 --- a/library/alloc/src/gc.rs +++ b/library/alloc/src/gc.rs @@ -87,9 +87,15 @@ unsafe fn gc_realloc(ptr: *mut u8, old_layout: Layout, new_size: usize) -> *mut } #[inline] -unsafe fn gc_free(ptr: *mut u8, _: Layout) { - unsafe { - boehm::GC_free(ptr); +unsafe fn gc_free(ptr: *mut u8, layout: Layout) { + if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() { + unsafe { + boehm::GC_free(ptr); + } + } else { + unsafe { + boehm::GC_free(boehm::GC_base(ptr)); + } } } diff --git a/library/alloc/tests/gc.rs b/library/alloc/tests/gc.rs index be25f93d38a4a..cda051e56566f 100644 --- a/library/alloc/tests/gc.rs +++ b/library/alloc/tests/gc.rs @@ -1,12 +1,41 @@ use std::gc::GcAllocator; +use std::alloc::GlobalAlloc; +use std::alloc::Layout; #[repr(align(1024))] struct S(u8); +#[repr(align(16))] +struct T(usize, usize, usize, usize); + #[test] fn large_alignment() { let x = Box::new_in(S(123), GcAllocator); let ptr = Box::into_raw(x); assert!(!ptr.is_null()); assert!(ptr.is_aligned()); + + // When this is freed, GC assertions will check if the base pointer can be + // correctly recovered. + unsafe { let _ = Box::from_raw_in(ptr, GcAllocator); } + + let y = Box::new_in(T(1, 2, 3, 4), GcAllocator); + let ptr = Box::into_raw(y); + assert!(!ptr.is_null()); + assert!(ptr.is_aligned()); + + unsafe { let _ = Box::from_raw_in(ptr, GcAllocator); } +} + +#[test] +fn boehm_issue_589() { + // Test the specific size / alignment problem raised in [1]. + // + // [1]: https://github.com/ivmai/bdwgc/issues/589 + unsafe { + let allocator = GcAllocator; + let layout = Layout::from_size_align_unchecked(512, 64); + let raw_ptr = GlobalAlloc::alloc(&allocator, layout); + GlobalAlloc::dealloc(&allocator, raw_ptr, layout); + } } diff --git a/library/boehm/src/lib.rs b/library/boehm/src/lib.rs index 6d4b25a558c7e..a5ff25852166a 100644 --- a/library/boehm/src/lib.rs +++ b/library/boehm/src/lib.rs @@ -42,6 +42,8 @@ extern "C" { pub fn GC_free(dead: *mut u8); + pub fn GC_base(mem_ptr: *mut u8) -> *mut u8; + pub fn GC_register_finalizer( ptr: *mut u8, finalizer: Option,