From 3ff8319b1d96bd9cdc97ae4769a57784bb884460 Mon Sep 17 00:00:00 2001 From: Matt Gleason Date: Tue, 11 Aug 2026 17:08:41 -0400 Subject: [PATCH 1/4] SFT-7482: first pass at arena allocator refactor --- arena/src/lib.rs | 137 ++++++++++++++++++++++++++++++++----------- urtypes/src/value.rs | 30 ++++++++++ 2 files changed, 134 insertions(+), 33 deletions(-) diff --git a/arena/src/lib.rs b/arena/src/lib.rs index 9e06cac..bde4c6b 100644 --- a/arena/src/lib.rs +++ b/arena/src/lib.rs @@ -26,20 +26,29 @@ #![no_std] -use core::{cell::RefCell, mem::MaybeUninit}; +use core::{ + cell::{Cell, UnsafeCell}, + mem::MaybeUninit, +}; pub mod boxed; /// An arena of objects of type `T`. pub struct Arena { - storage: RefCell>, + /// Each slot is written at most once. `UnsafeCell` permits initializing a + /// new slot through `&self` without creating a mutable reference to the + /// complete backing array, which would invalidate references to earlier + /// slots. + storage: UnsafeCell<[MaybeUninit; N]>, + len: Cell, } impl Arena { /// Construct a new arena. pub const fn new() -> Self { Self { - storage: RefCell::new(Chunk::new()), + storage: UnsafeCell::new([const { MaybeUninit::uninit() }; N]), + len: Cell::new(0), } } @@ -48,48 +57,110 @@ impl Arena { /// /// If there's not enough space left in the arena, then the item is /// returned as-is. - #[allow(clippy::mut_from_ref)] // Sound: interior mutability via RefCell + /// + /// Values allocated directly are not dropped when the arena is dropped. + /// Use [`boxed::Box`] when the value needs drop glue. + /// + /// # Safety invariants + /// + /// - `len` increases monotonically, so no two successful allocations use + /// the same slot. + /// - A mutable reference is created only for the newly initialized slot. + /// Later allocations access other slots only through raw pointers and + /// never create a mutable reference to the complete backing array. + /// - The returned reference is tied to the arena, so safe code cannot + /// outlive the arena or reset a slot while that reference exists. pub fn alloc(&self, item: T) -> Result<&mut T, T> { - let mut storage = self.storage.borrow_mut(); - let len = storage.len(); - storage.push(item)?; - Ok(unsafe { &mut *storage.as_mut_ptr().add(len) }) + let slot = self.len.get(); + if slot == N { + return Err(item); + } + + let ptr = self.slot_ptr(slot); + // SAFETY: `slot < N`, the slot is uninitialized, and the monotonic + // allocation index ensures no other reference can point at it. + unsafe { ptr.write(item) }; + + self.len.set(slot + 1); + + // SAFETY: the slot was initialized above and is uniquely owned by + // this allocation for the lifetime of the arena. + Ok(unsafe { &mut *ptr }) } -} -struct Chunk { - buffer: [MaybeUninit; N], - len: usize, + fn slot_ptr(&self, slot: usize) -> *mut T { + // SAFETY: callers ensure `slot < N`. Casting the raw pointer avoids + // creating a reference to the whole array. + unsafe { + self.storage + .get() + .cast::>() + .add(slot) + .cast::() + } + } } -impl Chunk { - const ELEM: MaybeUninit = MaybeUninit::uninit(); - const INIT: [MaybeUninit; N] = [Self::ELEM; N]; +#[cfg(test)] +mod tests { + use core::cell::Cell; - pub const fn new() -> Self { - Self { - buffer: Self::INIT, - len: 0, - } + use super::{boxed::Box, Arena}; + + #[test] + fn allocates_distinct_slots_up_to_capacity() { + let arena: Arena = Arena::new(); + + let first = arena.alloc(11).unwrap(); + let second = arena.alloc(22).unwrap(); + + assert_eq!((*first, *second), (11, 22)); + assert_eq!(arena.alloc(33), Err(33)); } - pub const fn len(&self) -> usize { - self.len + #[test] + fn earlier_allocation_remains_usable_after_later_allocation() { + let arena: Arena = Arena::new(); + + let first = arena.alloc(11).unwrap(); + let second = arena.alloc(22).unwrap(); + + // This is the arena's intended contract: successful allocations own + // distinct, stable slots for as long as the arena is alive. Before + // the allocator redesign, Miri reports UB when `first` is used here. + *first += 1; + *second += 1; + + assert_eq!((*first, *second), (12, 23)); } - pub fn push(&mut self, item: T) -> Result<(), T> { - if self.len < N { - unsafe { - *self.buffer.get_unchecked_mut(self.len) = MaybeUninit::new(item); - self.len += 1; + #[test] + fn counts_zero_sized_allocations_toward_capacity() { + let arena: Arena<(), 2> = Arena::new(); + + let first = arena.alloc(()).unwrap(); + let second = arena.alloc(()).unwrap(); + + assert_eq!((*first, *second), ((), ())); + assert_eq!(arena.alloc(()), Err(())); + } + + #[test] + fn boxed_value_is_dropped_once() { + #[derive(Debug)] + struct DropCounter<'a>(&'a Cell); + + impl Drop for DropCounter<'_> { + fn drop(&mut self) { + self.0.set(self.0.get() + 1); } - Ok(()) - } else { - Err(item) } - } - pub fn as_mut_ptr(&mut self) -> *mut T { - self.buffer.as_mut_ptr() as *mut T + let drops = Cell::new(0); + let arena: Arena = Arena::new(); + let value = Box::new_in(DropCounter(&drops), &arena).unwrap(); + + drop(value); + assert_eq!(drops.get(), 1); } } diff --git a/urtypes/src/value.rs b/urtypes/src/value.rs index f08cff3..c76f792 100644 --- a/urtypes/src/value.rs +++ b/urtypes/src/value.rs @@ -239,6 +239,36 @@ mod tests { assert!(matches!(decoded, Terminal::WitnessScriptHash(_))); } + #[test] + fn test_decode_output_descriptor_retains_nested_arena_nodes() { + // sh(sh(wsh(raw(0x42)))) allocates three nested `Terminal`s into the + // decode arena. The returned descriptor must retain and safely + // traverse every node after later allocations. + const CBOR: &[u8] = &[ + 0xd9, 0x01, 0x90, // script-hash + 0xd9, 0x01, 0x90, // script-hash + 0xd9, 0x01, 0x91, // witness-script-hash + 0xd9, 0x01, 0x98, // raw-script + 0x41, 0x42, // byte string: 0x42 + ]; + + let arena: TerminalContext<3> = TerminalContext::new(); + let decoded = decode_output_descriptor("crypto-output", CBOR, &arena).unwrap(); + + // Before the arena redesign, Miri reports UB while following this + // earliest arena-backed box after later decode allocations. + let Terminal::ScriptHash(first) = decoded else { + panic!("expected outer script-hash"); + }; + let Terminal::ScriptHash(second) = &*first else { + panic!("expected nested script-hash"); + }; + let Terminal::WitnessScriptHash(third) = &**second else { + panic!("expected witness-script-hash"); + }; + assert!(matches!(&**third, Terminal::RawScript(&[0x42]))); + } + #[test] fn test_decode_output_descriptor_rejects_output_descriptor_alias() { // BCR-2023-010 `output-descriptor` uses a different CBOR shape (tag From 55b9545f70c69a6098846dc68ff934953a50c3bd Mon Sep 17 00:00:00 2001 From: Matt Gleason Date: Wed, 12 Aug 2026 16:23:52 -0400 Subject: [PATCH 2/4] SFT-7482: removed excessive comments --- arena/src/lib.rs | 7 ------- urtypes/src/value.rs | 5 ----- 2 files changed, 12 deletions(-) diff --git a/arena/src/lib.rs b/arena/src/lib.rs index bde4c6b..a588199 100644 --- a/arena/src/lib.rs +++ b/arena/src/lib.rs @@ -35,10 +35,6 @@ pub mod boxed; /// An arena of objects of type `T`. pub struct Arena { - /// Each slot is written at most once. `UnsafeCell` permits initializing a - /// new slot through `&self` without creating a mutable reference to the - /// complete backing array, which would invalidate references to earlier - /// slots. storage: UnsafeCell<[MaybeUninit; N]>, len: Cell, } @@ -125,9 +121,6 @@ mod tests { let first = arena.alloc(11).unwrap(); let second = arena.alloc(22).unwrap(); - // This is the arena's intended contract: successful allocations own - // distinct, stable slots for as long as the arena is alive. Before - // the allocator redesign, Miri reports UB when `first` is used here. *first += 1; *second += 1; diff --git a/urtypes/src/value.rs b/urtypes/src/value.rs index c76f792..ea2533e 100644 --- a/urtypes/src/value.rs +++ b/urtypes/src/value.rs @@ -241,9 +241,6 @@ mod tests { #[test] fn test_decode_output_descriptor_retains_nested_arena_nodes() { - // sh(sh(wsh(raw(0x42)))) allocates three nested `Terminal`s into the - // decode arena. The returned descriptor must retain and safely - // traverse every node after later allocations. const CBOR: &[u8] = &[ 0xd9, 0x01, 0x90, // script-hash 0xd9, 0x01, 0x90, // script-hash @@ -255,8 +252,6 @@ mod tests { let arena: TerminalContext<3> = TerminalContext::new(); let decoded = decode_output_descriptor("crypto-output", CBOR, &arena).unwrap(); - // Before the arena redesign, Miri reports UB while following this - // earliest arena-backed box after later decode allocations. let Terminal::ScriptHash(first) = decoded else { panic!("expected outer script-hash"); }; From 5cd80a924b823fefcafdce6c77b86241f5d32fd5 Mon Sep 17 00:00:00 2001 From: Matt Gleason Date: Wed, 12 Aug 2026 16:32:20 -0400 Subject: [PATCH 3/4] SFT-7482: suppressed clippy error with justification from safety invariants --- arena/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/arena/src/lib.rs b/arena/src/lib.rs index a588199..8d3a5d9 100644 --- a/arena/src/lib.rs +++ b/arena/src/lib.rs @@ -66,6 +66,7 @@ impl Arena { /// never create a mutable reference to the complete backing array. /// - The returned reference is tied to the arena, so safe code cannot /// outlive the arena or reset a slot while that reference exists. + #[allow(clippy::mut_from_ref)] // SAFETY: see the invariants below. pub fn alloc(&self, item: T) -> Result<&mut T, T> { let slot = self.len.get(); if slot == N { From 4c30f0c663779e59db44a3ccbe2e8a3dab40c10f Mon Sep 17 00:00:00 2001 From: Matt Gleason Date: Tue, 8 Sep 2026 17:19:16 -0400 Subject: [PATCH 4/4] SFT-7482: handled PR feedback, fixed stable rust target compilation --- arena/src/lib.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/arena/src/lib.rs b/arena/src/lib.rs index 8d3a5d9..f362193 100644 --- a/arena/src/lib.rs +++ b/arena/src/lib.rs @@ -42,8 +42,12 @@ pub struct Arena { impl Arena { /// Construct a new arena. pub const fn new() -> Self { + // SAFETY: an array of `MaybeUninit` may contain uninitialized + // elements regardless of `T`. + let storage = unsafe { MaybeUninit::<[MaybeUninit; N]>::uninit().assume_init() }; + Self { - storage: UnsafeCell::new([const { MaybeUninit::uninit() }; N]), + storage: UnsafeCell::new(storage), len: Cell::new(0), } } @@ -69,7 +73,7 @@ impl Arena { #[allow(clippy::mut_from_ref)] // SAFETY: see the invariants below. pub fn alloc(&self, item: T) -> Result<&mut T, T> { let slot = self.len.get(); - if slot == N { + if slot >= N { return Err(item); } @@ -86,6 +90,15 @@ impl Arena { } fn slot_ptr(&self, slot: usize) -> *mut T { + // This should compile out if invariants hold up, and if not, + // crashing is preferable to undefined behavior. + assert!( + slot < N, + "Arena allocation slot out of bounds: slot = {}, N = {}", + slot, + N + ); + // SAFETY: callers ensure `slot < N`. Casting the raw pointer avoids // creating a reference to the whole array. unsafe {