From a712d1476b373ed6295115d464f8bc357aba68f5 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 16 Dec 2020 19:40:55 +0100 Subject: [PATCH] Return error instead of debug assert With dirty storage we hit the assert. Returning an error permits to continue to catch if the invariant is broken for normal operation while being able to continue fuzzing with dirty storage. --- libraries/persistent_store/src/format.rs | 56 +++++++++---------- .../persistent_store/src/format/bitfield.rs | 29 ++++++---- libraries/persistent_store/src/store.rs | 36 +++++++----- 3 files changed, 67 insertions(+), 54 deletions(-) diff --git a/libraries/persistent_store/src/format.rs b/libraries/persistent_store/src/format.rs index 9b5631b..f575750 100644 --- a/libraries/persistent_store/src/format.rs +++ b/libraries/persistent_store/src/format.rs @@ -335,12 +335,12 @@ impl Format { } /// Builds the storage representation of an init info. - pub fn build_init(&self, init: InitInfo) -> WordSlice { + pub fn build_init(&self, init: InitInfo) -> StoreResult { let mut word = ERASED_WORD; - INIT_CYCLE.set(&mut word, init.cycle); - INIT_PREFIX.set(&mut word, init.prefix); - WORD_CHECKSUM.set(&mut word, 0); - word.as_slice() + INIT_CYCLE.set(&mut word, init.cycle)?; + INIT_PREFIX.set(&mut word, init.prefix)?; + WORD_CHECKSUM.set(&mut word, 0)?; + Ok(word.as_slice()) } /// Returns the storage index of the compact info of a page. @@ -368,36 +368,36 @@ impl Format { } /// Builds the storage representation of a compact info. - pub fn build_compact(&self, compact: CompactInfo) -> WordSlice { + pub fn build_compact(&self, compact: CompactInfo) -> StoreResult { let mut word = ERASED_WORD; - COMPACT_TAIL.set(&mut word, compact.tail); - WORD_CHECKSUM.set(&mut word, 0); - word.as_slice() + COMPACT_TAIL.set(&mut word, compact.tail)?; + WORD_CHECKSUM.set(&mut word, 0)?; + Ok(word.as_slice()) } /// Builds the storage representation of an internal entry. - pub fn build_internal(&self, internal: InternalEntry) -> WordSlice { + pub fn build_internal(&self, internal: InternalEntry) -> StoreResult { let mut word = ERASED_WORD; match internal { InternalEntry::Erase { page } => { - ID_ERASE.set(&mut word); - ERASE_PAGE.set(&mut word, page); + ID_ERASE.set(&mut word)?; + ERASE_PAGE.set(&mut word, page)?; } InternalEntry::Clear { min_key } => { - ID_CLEAR.set(&mut word); - CLEAR_MIN_KEY.set(&mut word, min_key); + ID_CLEAR.set(&mut word)?; + CLEAR_MIN_KEY.set(&mut word, min_key)?; } InternalEntry::Marker { count } => { - ID_MARKER.set(&mut word); - MARKER_COUNT.set(&mut word, count); + ID_MARKER.set(&mut word)?; + MARKER_COUNT.set(&mut word, count)?; } InternalEntry::Remove { key } => { - ID_REMOVE.set(&mut word); - REMOVE_KEY.set(&mut word, key); + ID_REMOVE.set(&mut word)?; + REMOVE_KEY.set(&mut word, key)?; } } - WORD_CHECKSUM.set(&mut word, 0); - word.as_slice() + WORD_CHECKSUM.set(&mut word, 0)?; + Ok(word.as_slice()) } /// Parses the first word of an entry from its storage representation. @@ -459,31 +459,31 @@ impl Format { } /// Builds the storage representation of a user entry. - pub fn build_user(&self, key: Nat, value: &[u8]) -> Vec { + pub fn build_user(&self, key: Nat, value: &[u8]) -> StoreResult> { let length = usize_to_nat(value.len()); let word_size = self.word_size(); let footer = self.bytes_to_words(length); let mut result = vec![0xff; ((1 + footer) * word_size) as usize]; result[word_size as usize..][..length as usize].copy_from_slice(value); let mut word = ERASED_WORD; - ID_HEADER.set(&mut word); + ID_HEADER.set(&mut word)?; if footer > 0 && is_erased(&result[(footer * word_size) as usize..]) { HEADER_FLIPPED.set(&mut word); *result.last_mut().unwrap() = 0x7f; } - HEADER_LENGTH.set(&mut word, length); - HEADER_KEY.set(&mut word, key); + HEADER_LENGTH.set(&mut word, length)?; + HEADER_KEY.set(&mut word, key)?; HEADER_CHECKSUM.set( &mut word, count_zeros(&result[(footer * word_size) as usize..]), - ); + )?; result[..word_size as usize].copy_from_slice(&word.as_slice()); - result + Ok(result) } /// Sets the padding bit in the first word of a user entry. - pub fn set_padding(&self, word: &mut Word) { - ID_PADDING.set(word); + pub fn set_padding(&self, word: &mut Word) -> StoreResult<()> { + ID_PADDING.set(word) } /// Sets the deleted bit in the first word of a user entry. diff --git a/libraries/persistent_store/src/format/bitfield.rs b/libraries/persistent_store/src/format/bitfield.rs index 2cffc4b..32c0ae5 100644 --- a/libraries/persistent_store/src/format/bitfield.rs +++ b/libraries/persistent_store/src/format/bitfield.rs @@ -42,15 +42,20 @@ impl Field { /// Sets the value of a bit field. /// - /// # Preconditions + /// # Errors /// /// - The value must fit in the bit field: `num_bits(value) < self.len`. /// - The value must only change bits from 1 to 0: `self.get(*word) & value == value`. - pub fn set(&self, word: &mut Word, value: Nat) { - debug_assert_eq!(value & self.mask(), value); + pub fn set(&self, word: &mut Word, value: Nat) -> StoreResult<()> { + if value & self.mask() != value { + return Err(StoreError::InvalidStorage); + } let mask = !(self.mask() << self.pos); word.0 &= mask | (value << self.pos); - debug_assert_eq!(self.get(*word), value); + if self.get(*word) != value { + return Err(StoreError::InvalidStorage); + } + Ok(()) } /// Returns a bit mask the length of the bit field. @@ -82,8 +87,8 @@ impl ConstField { } /// Sets the bit field to its value. - pub fn set(&self, word: &mut Word) { - self.field.set(word, self.value); + pub fn set(&self, word: &mut Word) -> StoreResult<()> { + self.field.set(word, self.value) } } @@ -135,15 +140,15 @@ impl Checksum { /// Sets the checksum to the external increment value. /// - /// # Preconditions + /// # Errors /// /// - The bits of the checksum bit field should be set to one: `self.field.get(*word) == /// self.field.mask()`. /// - The checksum value should fit in the checksum bit field: `num_bits(word.count_zeros() + /// value) < self.field.len`. - pub fn set(&self, word: &mut Word, value: Nat) { + pub fn set(&self, word: &mut Word, value: Nat) -> StoreResult<()> { debug_assert_eq!(self.field.get(*word), self.field.mask()); - self.field.set(word, word.0.count_zeros() + value); + self.field.set(word, word.0.count_zeros() + value) } } @@ -290,7 +295,7 @@ mod tests { assert_eq!(field.get(Word(0x000000f8)), 0x1f); assert_eq!(field.get(Word(0x0000ff37)), 6); let mut word = Word(0xffffffff); - field.set(&mut word, 3); + field.set(&mut word, 3).unwrap(); assert_eq!(word, Word(0xffffff1f)); } @@ -305,7 +310,7 @@ mod tests { assert!(field.check(Word(0x00000048))); assert!(field.check(Word(0x0000ff4f))); let mut word = Word(0xffffffff); - field.set(&mut word); + field.set(&mut word).unwrap(); assert_eq!(word, Word(0xffffff4f)); } @@ -333,7 +338,7 @@ mod tests { assert_eq!(field.get(Word(0x00ffff67)), Ok(4)); assert_eq!(field.get(Word(0x7fffff07)), Err(StoreError::InvalidStorage)); let mut word = Word(0x0fffffff); - field.set(&mut word, 4); + field.set(&mut word, 4).unwrap(); assert_eq!(word, Word(0x0fffff47)); } diff --git a/libraries/persistent_store/src/store.rs b/libraries/persistent_store/src/store.rs index bc4258a..f707a89 100644 --- a/libraries/persistent_store/src/store.rs +++ b/libraries/persistent_store/src/store.rs @@ -300,7 +300,9 @@ impl Store { self.reserve(self.format.transaction_capacity(updates))?; // Write the marker entry. let marker = self.tail()?; - let entry = self.format.build_internal(InternalEntry::Marker { count }); + let entry = self + .format + .build_internal(InternalEntry::Marker { count })?; self.write_slice(marker, &entry)?; self.init_page(marker, marker)?; // Write the updates. @@ -308,7 +310,7 @@ impl Store { for update in updates { let length = match *update { StoreUpdate::Insert { key, ref value } => { - let entry = self.format.build_user(usize_to_nat(key), value); + let entry = self.format.build_user(usize_to_nat(key), value)?; let word_size = self.format.word_size(); let footer = usize_to_nat(entry.len()) / word_size - 1; self.write_slice(tail, &entry[..(footer * word_size) as usize])?; @@ -317,7 +319,7 @@ impl Store { } StoreUpdate::Remove { key } => { let key = usize_to_nat(key); - let remove = self.format.build_internal(InternalEntry::Remove { key }); + let remove = self.format.build_internal(InternalEntry::Remove { key })?; self.write_slice(tail, &remove)?; 0 } @@ -337,7 +339,9 @@ impl Store { if min_key > self.format.max_key() { return Err(StoreError::InvalidArgument); } - let clear = self.format.build_internal(InternalEntry::Clear { min_key }); + let clear = self + .format + .build_internal(InternalEntry::Clear { min_key })?; // We always have one word available. We can't use `reserve` because this is internal // capacity, not user capacity. while self.immediate_capacity()? < 1 { @@ -403,7 +407,7 @@ impl Store { if key > self.format.max_key() || value_len > self.format.max_value_len() { return Err(StoreError::InvalidArgument); } - let entry = self.format.build_user(key, value); + let entry = self.format.build_user(key, value)?; let entry_len = usize_to_nat(entry.len()); self.reserve(entry_len / self.format.word_size())?; let tail = self.tail()?; @@ -469,7 +473,7 @@ impl Store { let init_info = self.format.build_init(InitInfo { cycle: 0, prefix: 0, - }); + })?; self.storage_write_slice(index, &init_info) } @@ -681,7 +685,9 @@ impl Store { } let tail = max(self.tail()?, head.next_page(&self.format)); let index = self.format.index_compact(head.page(&self.format)); - let compact_info = self.format.build_compact(CompactInfo { tail: tail - head }); + let compact_info = self + .format + .build_compact(CompactInfo { tail: tail - head })?; self.storage_write_slice(index, &compact_info)?; self.compact_copy() } @@ -721,7 +727,7 @@ impl Store { self.init_page(tail, tail + (length - 1))?; tail += length; } - let erase = self.format.build_internal(InternalEntry::Erase { page }); + let erase = self.format.build_internal(InternalEntry::Erase { page })?; self.write_slice(tail, &erase)?; self.init_page(tail, tail)?; self.compact_erase(tail) @@ -851,7 +857,7 @@ impl Store { let init_info = self.format.build_init(InitInfo { cycle: new_first.cycle(&self.format), prefix: new_first.word(&self.format), - }); + })?; self.storage_write_slice(index, &init_info)?; Ok(()) } @@ -859,7 +865,7 @@ impl Store { /// Sets the padding bit of a user header. fn set_padding(&mut self, pos: Position) -> StoreResult<()> { let mut word = Word::from_slice(self.read_word(pos)); - self.format.set_padding(&mut word); + self.format.set_padding(&mut word)?; self.write_slice(pos, &word.as_slice())?; Ok(()) } @@ -1195,10 +1201,12 @@ impl Store { let format = Format::new(storage).unwrap(); // Write the init info of the first page. let mut index = format.index_init(0); - let init_info = format.build_init(InitInfo { - cycle: usize_to_nat(cycle), - prefix: 0, - }); + let init_info = format + .build_init(InitInfo { + cycle: usize_to_nat(cycle), + prefix: 0, + }) + .unwrap(); storage.write_slice(index, &init_info).unwrap(); // Pad the first word of the page. This makes the store looks used, otherwise we may confuse // it with a partially initialized store.