From 6f8d9cc57422fa0f759fe605241c6a836ad47375 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Mon, 1 Jul 2024 19:53:31 +0200 Subject: [PATCH] Fix the allocators after discovering bad bugs in there --- stabby-abi/src/alloc/allocators/libc_alloc.rs | 22 ++++-------------- stabby-abi/src/alloc/allocators/mod.rs | 10 +++++--- stabby-abi/src/alloc/allocators/rust_alloc.rs | 23 +++++++++++++++---- stabby-abi/src/alloc/collections/arc_btree.rs | 18 +++++++-------- stabby-abi/src/alloc/mod.rs | 22 ++++++++++-------- 5 files changed, 51 insertions(+), 44 deletions(-) diff --git a/stabby-abi/src/alloc/allocators/libc_alloc.rs b/stabby-abi/src/alloc/allocators/libc_alloc.rs index 5b1d70c..0cd57ed 100644 --- a/stabby-abi/src/alloc/allocators/libc_alloc.rs +++ b/stabby-abi/src/alloc/allocators/libc_alloc.rs @@ -12,8 +12,6 @@ // Pierre Avital, // -use std::io::Write; - use crate::alloc::Layout; #[cfg(not(windows))] @@ -31,7 +29,7 @@ unsafe fn posix_memalign(this: &mut *mut core::ffi::c_void, size: usize, align: use libc::aligned_free; #[cfg(not(windows))] use libc::free as aligned_free; -use libc::{malloc, realloc}; +use libc::realloc; /// An allocator based on `libc::posix_memalign` or `libc::aligned_malloc` depending on the platform. /// @@ -69,26 +67,14 @@ impl crate::alloc::IAlloc for LibcAlloc { unsafe { aligned_free(ptr.cast()) } } unsafe fn realloc(&mut self, ptr: *mut (), prev: Layout, new_size: usize) -> *mut () { - dbg!(prev); if new_size == 0 { return core::ptr::null_mut(); } - let mut new_ptr = if prev.align <= 8 { - eprintln!( - "Previous ({ptr:?}): {:?}", - core::slice::from_raw_parts(ptr.cast::(), prev.size) - ); - let new_ptr = unsafe { realloc(ptr.cast(), new_size) }; - eprintln!( - "Reallocd ({new_ptr:?}): {:?}", - core::slice::from_raw_parts(new_ptr.cast::(), prev.size) - ); - new_ptr - } else { - core::ptr::null_mut() + let mut new_ptr = core::ptr::null_mut::(); + if prev.align <= 8 { + new_ptr = unsafe { realloc(ptr.cast(), new_size) }; }; if new_ptr.is_null() { - new_ptr = core::ptr::null_mut(); let err = unsafe { posix_memalign(&mut new_ptr, prev.align, new_size) }; if err == 0 { unsafe { diff --git a/stabby-abi/src/alloc/allocators/mod.rs b/stabby-abi/src/alloc/allocators/mod.rs index aa82ac6..5d00843 100644 --- a/stabby-abi/src/alloc/allocators/mod.rs +++ b/stabby-abi/src/alloc/allocators/mod.rs @@ -8,13 +8,17 @@ /// This allocator is based on an ordered linked list of free memory blocks. // pub mod freelist_alloc; -#[cfg(not(any(target_arch = "wasm32")))] +#[cfg(all(feature = "libc", not(target_arch = "wasm32")))] /// [`IAlloc`](crate::alloc::IAlloc) bindings for `libc::malloc` -pub mod libc_alloc; +pub(crate) mod libc_alloc; +#[cfg(all(feature = "libc", not(target_arch = "wasm32")))] +pub use libc_alloc::LibcAlloc; #[cfg(feature = "alloc-rs")] /// Rust's GlobalAlloc, accessed through a vtable to ensure no incompatible function calls are performed -pub mod rust_alloc; +mod rust_alloc; +#[cfg(feature = "alloc-rs")] +pub use rust_alloc::RustAlloc; #[cfg(target_arch = "wasm32")] pub(crate) mod paging { diff --git a/stabby-abi/src/alloc/allocators/rust_alloc.rs b/stabby-abi/src/alloc/allocators/rust_alloc.rs index dce3d5c..d3a94c8 100644 --- a/stabby-abi/src/alloc/allocators/rust_alloc.rs +++ b/stabby-abi/src/alloc/allocators/rust_alloc.rs @@ -29,19 +29,34 @@ extern "C" fn realloc( new_size: usize, ) -> *mut () { let prev_layout = unsafe { ptr.cast::().sub(1).read() }; - let alloc_start = unsafe { ptr.cast::().sub(prev_layout.align) }; + let realloc_start = unsafe { + ptr.cast::() + .sub(prev_layout.align.max(core::mem::size_of::())) + }; let Ok(layout) = core::alloc::Layout::from_size_align(prev_layout.size, prev_layout.align) else { return core::ptr::null_mut(); }; - unsafe { alloc_rs::alloc::realloc(alloc_start, layout, new_size).cast() } + unsafe { + let requested = Layout::of::().concat(Layout { + size: new_size, + align: prev_layout.align, + }); + let alloc_start = alloc_rs::alloc::realloc(realloc_start, layout, requested.size); + let ret = alloc_start.add(layout.align().max(core::mem::size_of::())); + ret.cast::().sub(1).write(requested); + ret.cast() + } } extern "C" fn free(ptr: *mut ()) { let prev_layout = unsafe { ptr.cast::().sub(1).read() }; - let alloc_start = unsafe { ptr.cast::().sub(prev_layout.align) }; + let dealloc_start = unsafe { + ptr.cast::() + .sub(prev_layout.align.max(core::mem::size_of::())) + }; unsafe { alloc_rs::alloc::dealloc( - alloc_start, + dealloc_start, core::alloc::Layout::from_size_align_unchecked(prev_layout.size, prev_layout.align), ) } diff --git a/stabby-abi/src/alloc/collections/arc_btree.rs b/stabby-abi/src/alloc/collections/arc_btree.rs index 1eed364..95f7453 100644 --- a/stabby-abi/src/alloc/collections/arc_btree.rs +++ b/stabby-abi/src/alloc/collections/arc_btree.rs @@ -640,8 +640,6 @@ fn btree_insert_libc() { let mut vec = crate::alloc::vec::Vec::new(); let mut btree = ArcBTreeSet::new(); for _ in 0..rng.gen_range(0..800) { - eprintln!("btree: {btree:?}"); - eprintln!("vec: {vec:?}"); let val = rng.gen_range(0..100u8); if vec.binary_search(&val).is_ok() { assert_eq!(btree.insert(val), Some(val)); @@ -670,12 +668,10 @@ fn btree_insert_rs() { let mut rng = rand::thread_rng(); for i in 0..1000 { dbg!(i); - let mut vec = crate::alloc::vec::Vec::new_in( - crate::alloc::allocators::rust_alloc::RustAlloc::default(), - ); - let mut btree = ArcBTreeSet::<_, _, false, 5>::new_in( - crate::alloc::allocators::rust_alloc::RustAlloc::default(), - ); + let mut vec = + crate::alloc::vec::Vec::new_in(crate::alloc::allocators::RustAlloc::default()); + let mut btree = + ArcBTreeSet::<_, _, false, 5>::new_in(crate::alloc::allocators::RustAlloc::default()); for _ in 0..rng.gen_range(0..800) { let val = rng.gen_range(0..100); if vec.binary_search(&val).is_ok() { @@ -683,7 +679,11 @@ fn btree_insert_rs() { } else { vec.push(val); vec.sort(); - assert_eq!(btree.insert(val), None); + assert_eq!( + btree.insert(val), + None, + "The BTree contained an unexpected value: {btree:?}, {vec:?}" + ); } } vec.sort(); diff --git a/stabby-abi/src/alloc/mod.rs b/stabby-abi/src/alloc/mod.rs index 47ee0b2..776ffd3 100644 --- a/stabby-abi/src/alloc/mod.rs +++ b/stabby-abi/src/alloc/mod.rs @@ -20,7 +20,10 @@ use self::vec::ptr_diff; /// Allocators provided by `stabby` pub mod allocators; #[cfg(all(feature = "libc", not(any(target_arch = "wasm32"))))] -pub use allocators::libc_alloc; +/// The `libc_alloc` module, kept for API-stability. +pub mod libc_alloc { + pub use super::allocators::libc_alloc::LibcAlloc; +} /// A generic allocation error. #[crate::stabby] @@ -49,7 +52,7 @@ pub mod vec; /// The default allocator: libc malloc based if the libc feature is enabled, or unavailable otherwise. #[cfg(all(feature = "libc", not(any(target_arch = "wasm32"))))] -pub type DefaultAllocator = libc_alloc::LibcAlloc; +pub type DefaultAllocator = allocators::LibcAlloc; #[cfg(not(all(feature = "libc", not(any(target_arch = "wasm32")))))] pub type DefaultAllocator = core::convert::Infallible; @@ -358,11 +361,10 @@ impl AllocPtr { ), marker: PhantomData, }; - assert!(core::ptr::eq( + debug_assert!(core::ptr::eq( prefix.as_ptr().cast(), this.prefix() as *const _ )); - dbg!(prefix); this }) } @@ -377,11 +379,10 @@ impl AllocPtr { ptr: NonNull::new_unchecked(ptr.cast()), marker: PhantomData, }; - assert!(core::ptr::eq( + debug_assert!(core::ptr::eq( prefix.as_ptr().cast(), this.prefix() as *const _ )); - dbg!(prefix); this }) } @@ -399,11 +400,13 @@ impl AllocPtr { ) -> Option { let layout = Layout::of::>().concat(Layout::array::(prev_capacity)); let ptr = alloc.realloc( - dbg!(self.prefix() as *const AllocPrefix) + (self.prefix() as *const AllocPrefix) .cast_mut() .cast(), layout, - new_capacity, + Layout::of::>() + .concat(Layout::array::(new_capacity)) + .size, ); NonNull::new(ptr).map(|prefix| unsafe { prefix.cast::>().as_mut().capacity = AtomicUsize::new(new_capacity); @@ -412,11 +415,10 @@ impl AllocPtr { ptr: NonNull::new_unchecked(ptr.cast()), marker: PhantomData, }; - assert!(core::ptr::eq( + debug_assert!(core::ptr::eq( prefix.as_ptr().cast(), this.prefix() as *const _ )); - dbg!(prefix); this }) }