diff --git a/src/atom_table.rs b/src/atom_table.rs index 7f828654..155aeb9a 100644 --- a/src/atom_table.rs +++ b/src/atom_table.rs @@ -306,8 +306,7 @@ impl Atom { AtomTableRef::try_map(atom_table.inner.read(), |buf| unsafe { let ptr = buf .block - .base - .add(self.flat_index() as usize - STRINGS.len()); + .get_unchecked(self.flat_index() as usize - STRINGS.len()); // TODO use std::ptr::from_raw_parts instead when feature ptr_metadata is stable rust-lang/rust#81513 let atom_data = &*(std::ptr::slice_from_raw_parts(ptr, 0) as *const AtomData); let len = atom_data.header.len(); @@ -520,12 +519,13 @@ impl AtomTable { } }; - let ptr_base = block_epoch.block.base.addr(); + // SAFETY: `len_ptr` was obtained from `block_epoch.block.alloc()` + let len_offset = block_epoch.block.get_offset(len_ptr); write_to_ptr(string, len_ptr); let atom = AtomCell::new() - .with_name((STRINGS.len() + len_ptr.addr() - ptr_base) as u64) + .with_name((STRINGS.len() + len_offset) as u64) .with_arity(0) .with_f(false) .with_m(false) diff --git a/src/machine/stack.rs b/src/machine/stack.rs index c4bed865..15944356 100644 --- a/src/machine/stack.rs +++ b/src/machine/stack.rs @@ -89,14 +89,20 @@ impl Index for Stack { #[inline] fn index(&self, index: usize) -> &Self::Output { - unsafe { &*self.buf.base.add(index).cast() } + unsafe { + let ptr = self.buf.get_unchecked(index); + &*ptr.cast::() + } } } impl IndexMut for Stack { #[inline] fn index_mut(&mut self, index: usize) -> &mut Self::Output { - unsafe { &mut *self.buf.base.add(index).cast_mut().cast() } + unsafe { + let ptr = self.buf.get_unchecked(index); + &mut *ptr.cast_mut().cast::() + } } } @@ -183,7 +189,7 @@ impl Stack { let frame_size = AndFrame::size_of(num_cells); unsafe { - let e = (*self.buf.ptr.get_mut()).addr() - self.buf.base.addr(); + let e = self.buf.used_bytes(); let new_ptr = self.alloc(frame_size)?; let mut offset = prelude_size::(); @@ -208,14 +214,14 @@ impl Stack { } pub(crate) fn top(&self) -> usize { - unsafe { (*self.buf.ptr.get()).addr() - self.buf.base.addr() } + self.buf.used_bytes() } pub(crate) fn allocate_or_frame(&mut self, num_cells: usize) -> Result { let frame_size = OrFrame::size_of(num_cells); unsafe { - let b = (*self.buf.ptr.get_mut()).addr() - self.buf.base.addr(); + let b = self.buf.used_bytes(); let new_ptr = self.alloc(frame_size)?; let mut offset = prelude_size::(); @@ -239,37 +245,57 @@ impl Stack { } } + fn get_raw(&self, index: usize) -> *const u8 { + debug_assert!(index < self.buf.used_bytes()); + + unsafe { self.buf.get_unchecked(index) } + } + #[inline(always)] pub(crate) fn index_and_frame(&self, e: usize) -> &AndFrame { - unsafe { &*self.buf.base.add(e).cast() } + let ptr = self.get_raw(e); + + unsafe { &*ptr.cast::() } } #[inline(always)] pub(crate) fn index_and_frame_mut(&mut self, e: usize) -> &mut AndFrame { - unsafe { - // This is doing alignment wrong - let ptr = self.buf.base.add(e); - &mut *(ptr as *mut AndFrame) - } + let ptr = self.get_raw(e); + + unsafe { &mut *ptr.cast_mut().cast::() } } #[inline(always)] pub(crate) fn index_or_frame(&self, b: usize) -> &OrFrame { - unsafe { &*self.buf.base.add(b).cast() } + let ptr = self.get_raw(b); + + unsafe { &*ptr.cast::() } } #[inline(always)] pub(crate) fn index_or_frame_mut(&mut self, b: usize) -> &mut OrFrame { - unsafe { &mut *self.buf.base.add(b).cast_mut().cast() } + let ptr = self.get_raw(b); + + unsafe { &mut *ptr.cast_mut().cast::() } + } + + /// # Safety + /// + /// The stack must contain a valid OrFrame at [`self.top()`](Self::top), + /// which can only be achieved by allocating it in the first place and later truncating the stack. + /// + /// No allocation must have been done since the last call to [`truncate()`](Self::truncate). + #[inline(always)] + pub(crate) unsafe fn index_dangling_or_frame(&self) -> &OrFrame { + unsafe { + let ptr = self.buf.get_unchecked(self.top()); + &*ptr.cast::() + } } #[inline(always)] pub(crate) fn truncate(&mut self, b: usize) { - let base = unsafe { self.buf.base.add(b) }; - - if base < (*self.buf.ptr.get_mut()) { - *self.buf.ptr.get_mut() = base.cast_mut(); - } + self.buf.shift_back(b); } } diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index a319c08a..0e36d1ad 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -1303,8 +1303,10 @@ impl Machine { // the last clause of the retract // helper to delay deallocation of its // environment frame. - let clause_b = self.machine_st.stack.top(); - self.machine_st.stack.index_or_frame(clause_b).prelude.biip as usize + unsafe { + self.machine_st.stack.index_dangling_or_frame().prelude.biip + as usize + } }; return ( diff --git a/src/offset_table.rs b/src/offset_table.rs index c6fee00d..f09a3583 100644 --- a/src/offset_table.rs +++ b/src/offset_table.rs @@ -12,8 +12,8 @@ use parking_lot::{Mutex, RwLock}; use crate::machine::heap::AllocError; use crate::machine::machine_indices::IndexPtr; -use crate::raw_block::RawBlock; use crate::raw_block::RawBlockTraits; +use crate::raw_block::{RawBlock, RawBlockConcurrent}; use ordered_float::OrderedFloat; @@ -93,8 +93,9 @@ impl OffsetTableImpl { // this shouldn't be able to fail let raw_block = Arc::try_unwrap(table.block.replace(RawBlock::empty_block())).unwrap(); - self.0 = - InnerOffsetTableImpl::Serial(SerialOffsetTable { block: raw_block }); + self.0 = InnerOffsetTableImpl::Serial(SerialOffsetTable { + block: raw_block.into(), + }); Ok(()) } Err(table_arc) => { @@ -130,7 +131,7 @@ struct SerialOffsetTable { #[derive(Debug)] pub struct ConcurrentOffsetTable { - block: Arcu, GlobalEpochCounterPool>, + block: Arcu, GlobalEpochCounterPool>, growth_lock: RwLock<()>, offset_locks: RwLock>>, } @@ -225,17 +226,18 @@ impl SerialOffsetTable { } ptr::write(ptr as *mut T, value); - ptr.addr() - self.block.base.addr() + // SAFETY: `ptr` was obtained from `self.block.alloc()` + self.block.get_offset(ptr) } #[inline] unsafe fn lookup(&self, offset: usize) -> &T { - &*self.block.base.add(offset).cast::() + &*self.block.get_unchecked(offset).cast::() } #[inline] unsafe fn lookup_mut(&mut self, offset: usize) -> &mut T { - &mut *self.block.base.add(offset).cast::().cast_mut() + &mut *self.block.get_unchecked(offset).cast::().cast_mut() } #[allow(clippy::wrong_self_convention)] @@ -248,8 +250,9 @@ impl SerialOffsetTable { }; let serial_tbl = mem::replace(self, empty_serial_tbl); - let num_tbl_entries = serial_tbl.block.size() / size_of::(); - let block = Arcu::new(serial_tbl.block, GlobalEpochCounterPool); + let num_tbl_entries = serial_tbl.block.used_bytes() / size_of::(); + let raw_block: RawBlock = serial_tbl.block.into(); + let block = Arcu::new(raw_block, GlobalEpochCounterPool); let offset_locks: Vec> = (0..num_tbl_entries).map(|_| RwLock::new(())).collect(); @@ -283,7 +286,7 @@ impl ConcurrentOffsetTable { } } - let new_tbl_sz = block_epoch.size() / size_of::(); + let new_tbl_sz = block_epoch.used_bytes() / size_of::(); let mut offset_locks = self.offset_locks.write(); offset_locks.resize_with(new_tbl_sz, || RwLock::new(())); @@ -292,7 +295,8 @@ impl ConcurrentOffsetTable { ptr::write(ptr as *mut T, value); } - let value = ptr.addr() - block_epoch.base.addr(); + // SAFETY: `ptr` was obtained from `block_epoch.alloc()` + let value = unsafe { block_epoch.get_offset(ptr) }; // AtomTable would have to update the index table at this point // explicit drop to ensure we don't accidentally drop it early @@ -307,7 +311,7 @@ impl ConcurrentOffsetTable { let inner_offset_lock = outer_offset_lock[offset / size_of::()].read(); let rcu_ref = RcuRef::try_map(self.block.read(), |raw_block| unsafe { - raw_block.base.add(offset).cast::().as_ref() + raw_block.get_unchecked(offset).cast::().as_ref() }) .expect("offset valid"); @@ -326,8 +330,7 @@ impl ConcurrentOffsetTable { let rcu_ref = RcuRef::try_map(self.block.read(), |raw_block| unsafe { raw_block - .base - .add(offset) + .get_unchecked(offset) .cast_mut() .cast::>() .as_ref() @@ -402,6 +405,8 @@ impl F64Table { // which breaks the invariant indirection_tbl is meant to enforce. // Since this branch is never invoked, it does no harm, but that // that will eventually change. + // + // Note: may be indirectly fixed by the use of the new RawBlockConcurrency trait. { let indirection_tbl = concurrent_tbl.indirection_tbl.lock(); @@ -472,7 +477,9 @@ impl F64Table { .unwrap(); *self = Self::Serial(SerialF64Table { indirection_tbl: indirection_tbl.into_inner(), - offset_tbl: SerialOffsetTable { block: raw_block }, + offset_tbl: SerialOffsetTable { + block: raw_block.into(), + }, }); Ok(()) diff --git a/src/raw_block.rs b/src/raw_block.rs index 64346883..521bb72b 100644 --- a/src/raw_block.rs +++ b/src/raw_block.rs @@ -1,32 +1,156 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use core::marker::PhantomData; use std::alloc; -use std::cell::UnsafeCell; +use std::cell::Cell; use std::ptr; +use std::sync::atomic::{AtomicPtr, Ordering}; + +trait PtrCellTrait: std::fmt::Debug { + fn new(val: *mut u8) -> Self; + + fn get(&self) -> *mut u8; + + /// Modifies the wrapped value. + fn set(&mut self, val: *mut u8); + + /// Performs an atomic compare-and-swap on the wrapped value. + /// + /// If the compare succeeded and `cb` returns `Some(new_ptr)`, stored `new_ptr` and returns `Ok(old_ptr). + /// + /// If `cb` returns `None`, returns `Err(old_ptr)`. + /// + /// May retry multiple times if the comparison fails. + fn try_update(&self, cb: impl Fn(*mut u8) -> Option<*mut u8>) -> Result<*mut u8, *mut u8>; +} + +impl PtrCellTrait for Cell<*mut u8> { + fn new(val: *mut u8) -> Self { + Cell::new(val) + } + + #[inline(always)] + fn get(&self) -> *mut u8 { + Cell::get(self) + } + + #[inline(always)] + fn set(&mut self, val: *mut u8) { + Cell::set(self, val) + } + + #[inline(always)] + fn try_update(&self, cb: impl Fn(*mut u8) -> Option<*mut u8>) -> Result<*mut u8, *mut u8> { + let val = Cell::get(self); + if let Some(new_val) = cb(val) { + Cell::set(self, new_val); + Ok(val) + } else { + Err(val) + } + } +} + +impl PtrCellTrait for AtomicPtr { + fn new(val: *mut u8) -> Self { + AtomicPtr::new(val) + } + + #[inline(always)] + fn get(&self) -> *mut u8 { + self.load(Ordering::Acquire) + } + + #[inline(always)] + fn set(&mut self, val: *mut u8) { + *self.get_mut() = val; + } + + #[inline] + fn try_update(&self, cb: impl Fn(*mut u8) -> Option<*mut u8>) -> Result<*mut u8, *mut u8> { + let mut prev = PtrCellTrait::get(self); + + while let Some(next) = cb(prev) { + match self.compare_exchange_weak(prev, next, Ordering::Relaxed, Ordering::Acquire) { + x @ Ok(_) => return x, + Err(next_prev) => prev = next_prev, + } + } + + Err(prev) + + // TODO: replace with the following once 1.95 is the msrv: + // AtomicPtr::try_update(self, Ordering::Relaxed, Ordering::Acquire, cb) + } +} + +/// Allows the choice of implementation for the mutable pointer in [`RawBlock`]. +/// Can be one of: +/// - [`RawBlockSerial`] (using a [`Cell<*mut u8>`]) +/// - [`RawBlockConcurrent`] (using a [`AtomicPtr`]) +pub(crate) trait RawBlockConcurrency { + #[allow(private_bounds)] + type PtrCell: PtrCellTrait; +} + +#[derive(Debug, Clone, Copy)] +pub struct RawBlockSerial(); +#[derive(Debug, Clone, Copy)] +pub struct RawBlockConcurrent(); + +impl RawBlockConcurrency for RawBlockSerial { + type PtrCell = Cell<*mut u8>; +} + +impl RawBlockConcurrency for RawBlockConcurrent { + type PtrCell = AtomicPtr; +} use crate::machine::heap::AllocError; pub trait RawBlockTraits { + /// ## Safety + /// + /// Must be non-zero. fn init_size() -> usize; + + /// ## Safety + /// + /// Must be constant. + /// + /// Must respect the invariants of [`std::alloc::Layout::from_size_align()`], namely: + /// - must not be zero + /// - must be a power of two + /// - must not overflow `usize` fn align() -> usize; } +/// A block of memory with fast, lock-free appends. +/// +/// ## Invariants +/// +/// - `base.is_null()` iff `capacity == 0` +/// - if `!base.is_null()`, then `ptr.get()` is in the same allocation as `base`. #[derive(Debug)] -pub struct RawBlock { - pub base: *const u8, - pub top: *const u8, - pub ptr: UnsafeCell<*mut u8>, +pub struct RawBlock { + base: *const u8, + capacity: usize, + + ptr: C::PtrCell, _marker: PhantomData, + _c_marker: PhantomData, } -impl RawBlock { +impl RawBlock { #[inline] pub fn empty_block() -> Self { RawBlock { base: ptr::null(), - top: ptr::null(), - ptr: UnsafeCell::new(ptr::null_mut()), + capacity: 0, + ptr: C::PtrCell::new(ptr::null_mut()), _marker: PhantomData, + _c_marker: PhantomData, } } @@ -41,90 +165,253 @@ impl RawBlock { Ok(block) } + /// ## Safety + /// + /// Assumes that the object has not been initialized before (ie. `self.base.is_null()`) + /// and assumes that `cap > 0`. unsafe fn init_at_size(&mut self, cap: usize) -> Result<(), AllocError> { - let layout = alloc::Layout::from_size_align_unchecked(cap, T::align()); - let new_base = alloc::alloc(layout).cast_const(); + debug_assert!(cap > 0); + debug_assert!(self.base.is_null()); + + // SAFETY: + // - Guaranteed by caller: `cap > 0` + // - Guaranteed by caller: `T::align()` respects the invariants of `Layout::from_size_align` + let new_base = unsafe { + let layout = alloc::Layout::from_size_align_unchecked(cap, T::align()); + alloc::alloc(layout).cast_const() + }; + if new_base.is_null() { return Err(AllocError); } + self.base = new_base; - self.top = self.base.add(cap); - *self.ptr.get_mut() = self.base.cast_mut(); + self.capacity = cap; + self.ptr.set(self.base.cast_mut()); Ok(()) } + /// ## Safety + /// + /// Invalidates all pointers previously obtained by [`RawBlock::get()`] or [`RawBlock::alloc()`]. pub unsafe fn grow(&mut self) -> Result<(), AllocError> { - if self.base.is_null() { - self.init_at_size(T::init_size()) - } else { - let size = self.size(); - let layout = alloc::Layout::from_size_align_unchecked(size, T::align()); + self.debug_check_invariants(); + + if self.base.is_null() { + // SAFETY: + // - Guaranteed by caller: `T::init_size() > 0` + // - Asserted: `self.base.is_null()` + unsafe { self.init_at_size(T::init_size()) } + } else { + let size = self.capacity(); + let used_bytes = self.used_bytes(); + // SAFETY: + // - Guaranteed by caller: `T::align()` respects the invariants of `Layout::from_size_align` + // - Asserted: `!self.base.is_null()` + // - Invariant: `self.base.is_null()` iff `self.capacity() == 0` + // - Thus `self.capacity() > 0` + let new_base = unsafe { + let layout = alloc::Layout::from_size_align_unchecked(size, T::align()); + + alloc::realloc(self.base.cast_mut(), layout, size * 2).cast_const() + }; - let new_base = alloc::realloc(self.base.cast_mut(), layout, size * 2).cast_const(); if new_base.is_null() { Err(AllocError) } else { self.base = new_base; - self.top = self.base.add(size * 2); - *self.ptr.get_mut() = self.base.add(size).cast_mut(); + self.capacity = size * 2; + // SAFETY: + // - Invariant: `used_bytes < size` + // - Definition: `new_base` has allocation size `2 * size` + let new_ptr = unsafe { self.base.add(used_bytes).cast_mut() }; + self.ptr.set(new_ptr); + + self.debug_check_invariants(); Ok(()) } } } pub unsafe fn grow_new(&self) -> Result { + self.debug_check_invariants(); if self.base.is_null() { Self::new() } else { let mut new_block = Self::empty_block(); - new_block.init_at_size(self.size() * 2)?; - let allocated = (*self.ptr.get()).addr() - self.base.addr(); - self.base.copy_to(new_block.base.cast_mut(), allocated); - *new_block.ptr.get_mut() = new_block.base.add(allocated).cast_mut(); + // SAFETY: + // - Asserted: !self.base.is_null() + // - Invariant: self.base.is_null() iff self.capacity == 0 + // - Thus self.capacity > 0 + // - Definition: `new_block` was not yet initialized + unsafe { + new_block.init_at_size(self.capacity() * 2)?; + } + + let used_bytes = self.used_bytes(); + + // SAFETY: + // - Definition: `self.base` contains `self.allocated()` bytes + // - Invariant: `self.used_bytes() < self.allocated()` + unsafe { + self.base.copy_to(new_block.base.cast_mut(), used_bytes); + new_block.ptr.set(new_block.base.add(used_bytes).cast_mut()); + } + + new_block.debug_check_invariants(); + Ok(new_block) } } - #[inline] - pub fn size(&self) -> usize { - self.top.addr() - self.base.addr() + #[inline(always)] + fn debug_check_invariants(&self) { + if cfg!(debug_assertions) { + assert!( + self.ptr.get().cast_const() >= self.base, + "self.ptr = {:?} < {:?} = self.base", + self.ptr.get(), + self.base + ); + + assert!(self.used_bytes() <= self.capacity()); + } } - #[inline(always)] - unsafe fn free_space(&self) -> usize { - debug_assert!( - *self.ptr.get() as *const _ >= self.base, - "self.ptr = {:?} < {:?} = self.base", - *self.ptr.get(), - self.base - ); + #[inline] + pub fn capacity(&self) -> usize { + self.capacity + } - self.top.addr() - (*self.ptr.get()).addr() + #[inline] + pub fn used_bytes(&self) -> usize { + // SAFETY: + // - Invariant: `ptr` is in the same allocation as `base` + unsafe { self.ptr.get().offset_from(self.base) as usize } } pub unsafe fn alloc(&self, size: usize) -> *mut u8 { - let aligned_size = size.next_multiple_of(size); - if self.free_space() >= aligned_size { - let ptr = *self.ptr.get(); - *self.ptr.get() = ptr.add(aligned_size) as *mut _; - ptr - } else { - ptr::null_mut() + self.debug_check_invariants(); + + let aligned_size = size.next_multiple_of(T::align()); + + match self.ptr.try_update(|ptr| { + // SAFETY: + // - Invariant: `ptr` is in the same allocation as `base` + let free_bytes = unsafe { self.capacity() - ptr.offset_from(self.base) as usize }; + + if free_bytes >= aligned_size { + Some(unsafe { ptr.add(aligned_size) }) + } else { + // Not enough space: don't allocate and return a null pointer + None + } + }) { + Ok(ptr) => ptr, + Err(_) => ptr::null_mut(), } } + + /// Moves `ptr` back to `new_size`. + /// + /// Note that this method does *not* deallocate what was placed in the [`RawBlock`]. + /// Pointers to data past `new_size` remain valid until the next call to [`RawBlock::alloc()`]. + pub fn shift_back(&mut self, new_size: usize) { + self.debug_check_invariants(); + + assert!( + new_size <= self.used_bytes(), + "Shrink cannot grow: new_size = {:?} > allocated = {:?}", + new_size, + self.used_bytes() + ); + + // SAFETY: + // - Asserted: new_size <= self.capacity + // - Definition: self.base := alloc(self.capacity) + let new_ptr = unsafe { self.base.add(new_size) }; + + debug_assert!(new_ptr as usize <= self.ptr.get() as usize,); + + self.ptr.set(new_ptr.cast_mut()); + + self.debug_check_invariants(); + } + + /// Returns a pointer at a given `offset` within the block of memory. + /// + /// Panics if that range of bytes wasn't allocated yet with [`RawBlock::alloc()`]. + pub fn get(&self, offset: usize) -> *const u8 { + assert!(offset < self.used_bytes()); + + // SAFETY: Asserted. + unsafe { self.get_unchecked(offset) } + } + + /// Returns a pointer at a given `offset` within the block of memory. + /// + /// ## Safety + /// + /// Assumes that `offset < self.capacity()`. + #[inline] + pub unsafe fn get_unchecked(&self, offset: usize) -> *const u8 { + debug_assert!( + offset < self.capacity(), + "offset out of bounds: offset is {:?} but {:?} bytes are available", + offset, + self.used_bytes() + ); + + // SAFETY: Guaranteed by caller. + unsafe { self.base.add(offset) } + } + + /// ## Safety + /// + /// `ptr` is a valid pointer be obtained from [`RawBlock::get()`] or [`RawBlock::alloc()`]. + #[inline] + pub unsafe fn get_offset(&self, ptr: *const u8) -> usize { + // SAFETY: + // - Guaranteed by caller: `ptr` is still valid + // - Guranteed by caller: `ptr` was obtained from `get()` or `alloc()` + // - get() and alloc() return pointers in the same allocation as `self.base` + // - All functions modifying `self.base` invalidate pointers in their contract + // - Thus `ptr` and `self.base` originate from the same allocation + unsafe { ptr.offset_from(self.base) as usize } + } } -impl Drop for RawBlock { +impl Drop for RawBlock { fn drop(&mut self) { if !self.base.is_null() { unsafe { - let layout = alloc::Layout::from_size_align_unchecked(self.size(), T::align()); + let layout = alloc::Layout::from_size_align_unchecked(self.capacity(), T::align()); alloc::dealloc(self.base as *mut _, layout); } - - self.top = ptr::null(); - self.base = ptr::null(); - *self.ptr.get_mut() = ptr::null_mut(); + } + } +} + +impl From> for RawBlock { + fn from(other: RawBlock) -> Self { + Self { + base: other.base, + capacity: other.capacity, + ptr: PtrCellTrait::new(other.ptr.get()), + _marker: PhantomData, + _c_marker: PhantomData, + } + } +} + +impl From> for RawBlock { + fn from(other: RawBlock) -> Self { + Self { + base: other.base, + capacity: other.capacity, + ptr: PtrCellTrait::new(other.ptr.get()), + _marker: PhantomData, + _c_marker: PhantomData, } } }