Skip to content

Commit

Permalink
Return error instead of debug assert (google#363)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ia0
ia0 authored Aug 10, 2021
1 parent 42050f9 commit 5c7df89
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 54 deletions.
56 changes: 28 additions & 28 deletions libraries/persistent_store/src/format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<WordSlice> {
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.
Expand Down Expand Up @@ -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<WordSlice> {
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<WordSlice> {
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.
Expand Down Expand Up @@ -459,31 +459,31 @@ impl Format {
}

/// Builds the storage representation of a user entry.
pub fn build_user(&self, key: Nat, value: &[u8]) -> Vec<u8> {
pub fn build_user(&self, key: Nat, value: &[u8]) -> StoreResult<Vec<u8>> {
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.
Expand Down
29 changes: 17 additions & 12 deletions libraries/persistent_store/src/format/bitfield.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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));
}

Expand All @@ -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));
}

Expand Down Expand Up @@ -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));
}

Expand Down
36 changes: 22 additions & 14 deletions libraries/persistent_store/src/store.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,17 @@ impl<S: Storage> Store<S> {
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.
let mut tail = marker + 1;
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])?;
Expand All @@ -287,7 +289,7 @@ impl<S: Storage> Store<S> {
}
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
}
Expand All @@ -307,7 +309,9 @@ impl<S: Storage> Store<S> {
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 {
Expand Down Expand Up @@ -373,7 +377,7 @@ impl<S: Storage> Store<S> {
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()?;
Expand Down Expand Up @@ -437,7 +441,7 @@ impl<S: Storage> Store<S> {
let init_info = self.format.build_init(InitInfo {
cycle: 0,
prefix: 0,
});
})?;
self.storage_write_slice(index, &init_info)
}

Expand Down Expand Up @@ -646,7 +650,9 @@ impl<S: Storage> Store<S> {
}
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()
}
Expand Down Expand Up @@ -680,7 +686,7 @@ impl<S: Storage> Store<S> {
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)
Expand Down Expand Up @@ -792,15 +798,15 @@ impl<S: Storage> Store<S> {
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(())
}

/// 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(())
}
Expand Down Expand Up @@ -1110,10 +1116,12 @@ impl Store<BufferStorage> {
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.
Expand Down

0 comments on commit 5c7df89

Please sign in to comment.