From: Emilie Burgun Date: Sun, 10 May 2026 13:07:08 +0000 (+0200) Subject: RawBlock: seal `base` and add Stack::index_dangling_or_frame X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=898b6b2b251dc84224fe06c6af1f2187f60d5d6c;p=scryer-prolog.git RawBlock: seal `base` and add Stack::index_dangling_or_frame Direct accesses to `base` are replaced with dedicated methods with explicit safety requirements. --- 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 fe57e91d..8c64e62d 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::() + } } } @@ -241,33 +247,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) { - self.buf.shrink(b); + self.buf.shift_back(b); } } diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index b48d3a1b..6cb74cab 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -1327,8 +1327,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..a0194f93 100644 --- a/src/offset_table.rs +++ b/src/offset_table.rs @@ -225,17 +225,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,7 +249,7 @@ impl SerialOffsetTable { }; let serial_tbl = mem::replace(self, empty_serial_tbl); - let num_tbl_entries = serial_tbl.block.size() / size_of::(); + let num_tbl_entries = serial_tbl.block.used_bytes() / size_of::(); let block = Arcu::new(serial_tbl.block, GlobalEpochCounterPool); let offset_locks: Vec> = (0..num_tbl_entries).map(|_| RwLock::new(())).collect(); @@ -283,7 +284,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 +293,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 +309,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 +328,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() diff --git a/src/raw_block.rs b/src/raw_block.rs index 8c4e4af5..9f5ecc90 100644 --- a/src/raw_block.rs +++ b/src/raw_block.rs @@ -14,7 +14,7 @@ pub trait RawBlockTraits { /// A block of memory with fast, lock-free appends. #[derive(Debug)] pub struct RawBlock { - pub base: *const u8, + base: *const u8, capacity: usize, ptr: UnsafeCell<*mut u8>, @@ -55,6 +55,9 @@ impl RawBlock { 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()) @@ -141,7 +144,8 @@ impl RawBlock { /// Moves `ptr` back to `new_size`. /// /// Note that this method does *not* deallocate what was placed in the [`RawBlock`]. - pub fn shrink(&mut self, new_size: usize) { + /// 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!( @@ -162,6 +166,46 @@ impl RawBlock { 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() + ); + 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 {