diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d024ebf..17dd86f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,6 +52,10 @@ jobs: - name: Build run: cargo build --no-default-features --features ${{matrix.features}} + - name: Test + if: matrix.features == 'std' + run: cargo test --features ${{matrix.features}} + - name: Examples if: matrix.features == 'os' run: cargo build --release --examples --features ${{matrix.features}},nix,log,examples @@ -79,3 +83,32 @@ jobs: if: ${{ !env.ACT && matrix.features == 'os' }} with: platform-name: cross-platform + + run_miri: + runs-on: ubuntu-latest + strategy: + matrix: + features: ['std'] + + steps: + - name: Rust + uses: dtolnay/rust-toolchain@nightly + with: + components: miri + + - name: Install libdbus + run: sudo apt-get install -y libdbus-1-dev + + - name: Install libavahi-client + run: sudo apt-get install -y libavahi-client-dev + + - name: Checkout + uses: actions/checkout@v3 + + - name: Miri Setup + if: matrix.features == 'std' + run: cargo miri setup + + - name: Miri Test + if: matrix.features == 'std' + run: cargo miri test --features ${{matrix.features}} diff --git a/Cargo.toml b/Cargo.toml index e14173d..c4d7a5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -163,7 +163,7 @@ rustcrypto = ["rs-matter/rustcrypto"] os = ["backtrace", "rs-matter/os", "rustcrypto", "embassy-time/std"] backtrace = ["std", "rs-matter/backtrace"] async-io-mini = ["std", "edge-nal-std/async-io-mini"] -std = ["alloc", "rs-matter/std", "edge-nal-std"] +std = ["alloc", "rs-matter/std", "edge-nal-std", "critical-section/std"] alloc = ["embedded-svc/alloc"] examples = ["log", "os", "nix", "embassy-time-queue-utils/generic-queue-64", "zeroconf"] @@ -191,6 +191,7 @@ bitflags = "2" nix = { version = "0.27", features = ["net"], optional = true } [dev-dependencies] +critical-section = "1.0" static_cell = "2.1" futures-lite = "1" async-compat = "0.2" diff --git a/src/bump.rs b/src/bump.rs index e767753..516b878 100644 --- a/src/bump.rs +++ b/src/bump.rs @@ -7,7 +7,7 @@ //! `rustc` not being very intelligent w.r.t. stack usage in async functions. use core::marker::PhantomData; -use core::mem::MaybeUninit; +use core::mem::{self, MaybeUninit}; use core::pin::Pin; use core::ptr::NonNull; @@ -102,38 +102,37 @@ impl Bump { T: Sized, { self.inner.lock(|inner| { - let mut inner = inner.borrow_mut(); - - let size = core::mem::size_of_val(&object); + // SAFETY: + // The idea is to have a large chunk of memory allocated on the stack, + // with this function one can reserve a chunk of that memory for an object + // of type T. + // + // To reserve the memory, it will move the offset forward by the size required + // for T, and return a **mutable** reference to it. + // + // Given that it returns a mutable reference to it, there cannot be any other + // references to that memory location. This is ensured by the offset. + + let size = mem::size_of_val(&object); + let mut inner = inner.borrow_mut(); let offset = inner.offset; - let memory = unsafe { inner.memory.assume_init_mut() }; info!( "BUMP[{}]: {}b (U:{}b/F:{}b)", location, size, offset, - memory.len() - offset + inner.len() - offset ); - let remaining = &mut memory[offset..]; - let remaining_len = remaining.len(); - - let (t_buf, r_buf) = align_min::(remaining, 1); + let value = inner.allocate::(); - // Safety: We just allocated the memory and it's properly aligned - let ptr = unsafe { - let ptr = t_buf.as_ptr() as *mut T; - ptr.write(object); - - NonNull::new_unchecked(ptr) - }; - - inner.offset += remaining_len - r_buf.len(); + value.write(object); BumpBox { - ptr, + // SAFETY: The code above wrote to the memory location -> it can not be null + ptr: unsafe { NonNull::new_unchecked(value.as_mut_ptr()) }, _allocator: PhantomData, } }) @@ -143,7 +142,7 @@ impl Bump { /// A box-like container that uses bump allocation pub struct BumpBox<'a, T> { ptr: NonNull, - _allocator: core::marker::PhantomData<&'a ()>, + _allocator: PhantomData<&'a ()>, } impl BumpBox<'_, T> { @@ -182,7 +181,22 @@ impl Drop for BumpBox<'_, T> { } struct Inner { - memory: MaybeUninit<[u8; N]>, + // It is uncertain whether a [MaybeUninit; N] initialized with [const { MaybeUninit::uninit() }; N] + // would never be temporarily allocated on the stack and then moved to the final destination. + // + // In addition to that, the const { MaybeUninit::uninit() } sometimes results in bad optimizations like + // https://stackoverflow.com/questions/79513440. + // With the 1.77 compiler target, this issue would still be present, and const expressions only got + // stabilized in 1.79 -> not available. + // + // To avoid this, the entire array is wrapped in a MaybeUninit, which should prevent the compiler from + // trying to eagerly initialize the array on the stack. + // + // Technically it is enough to have MaybeUninit<[u8; N]> and then transmute it to [MaybeUninit; N] + // (which is safe to do), but this would require a transmute that is easy to get wrong. + // Using MaybeUninit<[MaybeUninit; N]> does not have any downsides and one can just use assume_init + // or assume_init_mut to get the [MaybeUninit; N] directly. + memory: MaybeUninit<[MaybeUninit; N]>, offset: usize, } @@ -200,10 +214,40 @@ impl Inner { offset: 0, }) } + + fn len(&self) -> usize { + N + } + + /// Allocate space for an object of type `T` + /// + /// # Panics + /// + /// If there is not enough memory left in the bump allocator to + /// allocate the requested objects. + fn allocate(&mut self) -> &mut MaybeUninit { + // SAFETY: This is safe because the type we are claiming to have initialized, + // is a bunch of `MaybeUninit`s, which do not require initialization. + let data: &mut [MaybeUninit; N] = unsafe { self.memory.assume_init_mut() }; + + // We can only use the memory from the current offset onwards, because + // the previous memory might be in use by previously allocated objects. + let remaining = &mut data[self.offset..]; + let remaining_len = remaining.len(); + // The t_buf will be where the caller can place their objects, + // and r_buf should be the remaining unused memory. + let (t_buf, r_buf) = align_min::(remaining, 1); + self.offset += remaining_len - r_buf.len(); + + &mut t_buf[0] + } } -fn align_min(buf: &mut [u8], count: usize) -> (&mut [MaybeUninit], &mut [u8]) { - if count == 0 || core::mem::size_of::() == 0 { +fn align_min( + buf: &mut [MaybeUninit], + count: usize, +) -> (&mut [MaybeUninit], &mut [MaybeUninit]) { + if count == 0 || mem::size_of::() == 0 { return (&mut [], buf); } @@ -215,7 +259,7 @@ fn align_min(buf: &mut [u8], count: usize) -> (&mut [MaybeUninit], &mut [u // Shrink `t_buf` to the number of requested items (count) let t_buf = &mut t_buf[..count]; let t_leading_buf0_len = t_leading_buf0.len(); - let t_buf_size = core::mem::size_of_val(t_buf); + let t_buf_size = mem::size_of_val(t_buf); let (buf0, remaining_buf) = buf.split_at_mut(t_leading_buf0_len + t_buf_size); @@ -226,3 +270,39 @@ fn align_min(buf: &mut [u8], count: usize) -> (&mut [MaybeUninit], &mut [u (t_buf, remaining_buf) } + +#[cfg(all(test, feature = "std"))] +mod tests { + use super::*; + + use alloc::vec::Vec; + use rs_matter::utils::sync::blocking::raw::StdRawMutex; + + const BUMP_SIZE: usize = 1024; + const DEFAULT_VALUE: u32 = 0xDEADBEEF; + + #[test] + fn test_one_concurrent_borrow() { + static BUMP: Bump = Bump::new(); + + for _ in 0..(BUMP_SIZE / mem::size_of_val(&DEFAULT_VALUE)) { + let b1 = BUMP.alloc(DEFAULT_VALUE, "test1"); + + assert_eq!(*b1, DEFAULT_VALUE); + } + } + + #[test] + fn test_multiple_concurrent_borrow() { + static BUMP: Bump = Bump::new(); + + let mut all_boxes = Vec::new(); + for i in 0..(BUMP_SIZE / mem::size_of::()) { + all_boxes.push(alloc!(BUMP, i)); + } + + for (i, b) in all_boxes.into_iter().enumerate() { + assert_eq!(*b, i); + } + } +}