Skip to content

Commit

Permalink
Fix bug and add debug impls
Browse files Browse the repository at this point in the history
  • Loading branch information
diondokter committed Feb 7, 2024
1 parent a319eb5 commit 1cbef99
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 26 deletions.
31 changes: 22 additions & 9 deletions fuzz/fuzz_targets/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,29 @@ use libfuzzer_sys::arbitrary::Arbitrary;
use libfuzzer_sys::fuzz_target;
use rand::{Rng, SeedableRng};
use sequential_storage::{
cache::{CacheImpl, NoCache, PagePointerCache, PageStateCache},
mock_flash::{MockFlashBase, MockFlashError, WriteCountCheck},
Error,
};
use std::{collections::VecDeque, ops::Range};
const MAX_VALUE_SIZE: usize = u8::MAX as usize;

fuzz_target!(|data: Input| fuzz(data));
const PAGES: usize = 4;
const WORD_SIZE: usize = 4;
const WORDS_PER_PAGE: usize = 256;

fuzz_target!(|data: Input| match data.cache_type {
CacheType::NoCache => fuzz(data, NoCache::new()),
CacheType::PageStateCache => fuzz(data, PageStateCache::<PAGES>::new()),
CacheType::PagePointerCache => fuzz(data, PagePointerCache::<PAGES>::new()),
});

#[derive(Arbitrary, Debug, Clone)]
struct Input {
seed: u64,
fuel: u16,
ops: Vec<Op>,
cache_type: CacheType,
}

#[derive(Arbitrary, Debug, Clone)]
Expand All @@ -34,23 +44,24 @@ struct PushOp {
value_len: u8,
}

#[derive(Arbitrary, Debug, Clone)]
enum CacheType {
NoCache,
PageStateCache,
PagePointerCache,
}

#[repr(align(4))]
struct AlignedBuf([u8; MAX_VALUE_SIZE + 1]);

fn fuzz(ops: Input) {
const PAGES: usize = 4;
const WORD_SIZE: usize = 4;
const WORDS_PER_PAGE: usize = 256;

fn fuzz(ops: Input, mut cache: impl CacheImpl) {
let mut flash = MockFlashBase::<PAGES, WORD_SIZE, WORDS_PER_PAGE>::new(
WriteCountCheck::Twice,
Some(ops.fuel as u32),
true,
);
const FLASH_RANGE: Range<u32> = 0x000..0x1000;

let mut cache = sequential_storage::cache::NoCache::new();

let mut order = VecDeque::new();
let mut buf = AlignedBuf([0; MAX_VALUE_SIZE + 1]);

Expand All @@ -67,7 +78,9 @@ fn fuzz(ops: Input) {
#[cfg(fuzzing_repro)]
eprintln!("{}", flash.print_items());
#[cfg(fuzzing_repro)]
eprintln!("=== OP: {op:?} ===");
eprintln!("{:?}", cache);
#[cfg(fuzzing_repro)]
eprintln!("\n=== OP: {op:?} ===\n");

match &mut op {
Op::Push(op) => {
Expand Down
65 changes: 59 additions & 6 deletions src/cache.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Module implementing all things cache related

use core::{num::NonZeroU32, ops::Range};
use core::{fmt::Debug, num::NonZeroU32, ops::Range};

use embedded_storage_async::nor_flash::NorFlash;

Expand All @@ -10,7 +10,7 @@ use crate::{

/// Trait implemented by all cache types
#[allow(private_bounds)]
pub trait CacheImpl: PrivateCacheImpl {}
pub trait CacheImpl: PrivateCacheImpl + Debug {}

impl<T: CacheImpl> CacheImpl for &mut T {}

Expand Down Expand Up @@ -45,6 +45,7 @@ impl<PSC: PageStatesCache, PPC: PagePointersCache> PrivateCacheImpl for Cache<PS
///
/// This type of cache doesn't have to be kept around and may be constructed on every api call.
/// You could simply pass `&mut NoCache::new()` every time.
#[derive(Debug)]
pub struct NoCache(Cache<UncachedPageSates, UncachedPagePointers>);

impl NoCache {
Expand Down Expand Up @@ -76,6 +77,7 @@ impl CacheImpl for NoCache {}
/// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌`
///
/// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point.
#[derive(Debug)]
pub struct PageStateCache<const PAGE_COUNT: usize>(
Cache<CachedPageStates<PAGE_COUNT>, UncachedPagePointers>,
);
Expand Down Expand Up @@ -109,6 +111,7 @@ impl<const PAGE_COUNT: usize> CacheImpl for PageStateCache<PAGE_COUNT> {}
/// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌`
///
/// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point.
#[derive(Debug)]
pub struct PagePointerCache<const PAGE_COUNT: usize>(
Cache<CachedPageStates<PAGE_COUNT>, CachedPagePointers<PAGE_COUNT>>,
);
Expand Down Expand Up @@ -238,17 +241,36 @@ impl<PSC: PageStatesCache, PPC: PagePointersCache> Cache<PSC, PPC> {
}
}

pub(crate) trait PageStatesCache {
pub(crate) trait PageStatesCache: Debug {
fn get_page_state(&self, page_index: usize) -> Option<PageState>;
fn notice_page_state(&mut self, page_index: usize, new_state: PageState);
fn invalidate_cache_state(&mut self);
}

#[derive(Debug)]
pub(crate) struct CachedPageStates<const PAGE_COUNT: usize> {
pages: [Option<PageState>; PAGE_COUNT],
}

impl<const PAGE_COUNT: usize> Debug for CachedPageStates<PAGE_COUNT> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "[")?;
for (i, val) in self.pages.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}

if let Some(val) = val {
write!(f, "{val:?}")?;
} else {
write!(f, "?")?;
}
}
write!(f, "]")?;

Ok(())
}
}

impl<const PAGE_COUNT: usize> CachedPageStates<PAGE_COUNT> {
pub const fn new() -> Self {
Self {
Expand Down Expand Up @@ -284,7 +306,7 @@ impl PageStatesCache for UncachedPageSates {
fn invalidate_cache_state(&mut self) {}
}

pub(crate) trait PagePointersCache {
pub(crate) trait PagePointersCache: Debug {
fn first_item_after_erased(&self, page_index: usize) -> Option<u32>;
fn first_item_after_written(&self, page_index: usize) -> Option<u32>;

Expand All @@ -307,12 +329,43 @@ pub(crate) trait PagePointersCache {

// Use NoneZeroU32 because we never store 0's in here (because of the first page marker)
// and so Option can make use of the niche so we save bytes
#[derive(Debug)]
pub(crate) struct CachedPagePointers<const PAGE_COUNT: usize> {
after_erased_pointers: [Option<NonZeroU32>; PAGE_COUNT],
after_written_pointers: [Option<NonZeroU32>; PAGE_COUNT],
}

impl<const PAGE_COUNT: usize> Debug for CachedPagePointers<PAGE_COUNT> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{{ after_erased_pointers: [")?;
for (i, val) in self.after_erased_pointers.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}

if let Some(val) = val {
write!(f, "{:?}", val.get())?;
} else {
write!(f, "?")?;
}
}
write!(f, "], after_written_pointers: [")?;
for (i, val) in self.after_written_pointers.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}

if let Some(val) = val {
write!(f, "{:?}", val.get())?;
} else {
write!(f, "?")?;
}
}
write!(f, "] }}")?;

Ok(())
}
}

impl<const PAGE_COUNT: usize> CachedPagePointers<PAGE_COUNT> {
pub const fn new() -> Self {
Self {
Expand Down
15 changes: 4 additions & 11 deletions src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,7 @@ impl ItemHeader {
}
}

async fn write<S: NorFlash>(
&self,
flash: &mut S,
flash_range: Range<u32>,
cache: &mut Cache<impl PageStatesCache, impl PagePointersCache>,
address: u32,
) -> Result<(), Error<S::Error>> {
async fn write<S: NorFlash>(&self, flash: &mut S, address: u32) -> Result<(), Error<S::Error>> {
let mut buffer = AlignedBuf([0xFF; MAX_WORD_SIZE]);

buffer[Self::DATA_CRC_FIELD]
Expand All @@ -153,8 +147,6 @@ impl ItemHeader {
buffer[Self::LENGTH_CRC_FIELD]
.copy_from_slice(&crc16(&self.length.to_le_bytes()).to_le_bytes());

cache.notice_item_written::<S>(flash_range, address, self);

flash
.write(
address,
Expand All @@ -177,7 +169,7 @@ impl ItemHeader {
) -> Result<Self, Error<S::Error>> {
self.crc = None;
cache.notice_item_erased::<S>(flash_range.clone(), address, &self);
self.write(flash, flash_range, cache, address).await?;
self.write(flash, address).await?;
Ok(self)
}

Expand Down Expand Up @@ -240,7 +232,8 @@ impl<'d> Item<'d> {
data: &[u8],
address: u32,
) -> Result<(), Error<S::Error>> {
header.write(flash, flash_range, cache, address).await?;
cache.notice_item_written::<S>(flash_range, address, &header);
header.write(flash, address).await?;

let (data_block, data_left) = data.split_at(round_down_to_alignment_usize::<S>(data.len()));

Expand Down

0 comments on commit 1cbef99

Please sign in to comment.