From 3e988a786fe1287d1e404c1993d410bf49c5d146 Mon Sep 17 00:00:00 2001 From: Thomas Tanon Date: Thu, 22 Sep 2022 12:16:41 +0200 Subject: [PATCH] Improves fuzzers (#123) * Fixes ref-counted fuzzer Values which reference number is 0 or below might be still present or not * Fuzzer: Checks that removed values are not in the database anymore --- fuzz/Cargo.toml | 3 + fuzz/fuzz_targets/refcounted_model.rs | 18 +++- fuzz/fuzz_targets/simple_model.rs | 30 ++++-- fuzz/src/lib.rs | 136 +++++++++++++++----------- 4 files changed, 116 insertions(+), 71 deletions(-) diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index d87c960d..e732cf4e 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -14,6 +14,9 @@ libfuzzer-sys = "0.4" parity-db = { path = "..", features = ["instrumentation"] } tempfile = "3" +[profile.release] +debug = true + [workspace] members = ["."] diff --git a/fuzz/fuzz_targets/refcounted_model.rs b/fuzz/fuzz_targets/refcounted_model.rs index 3b25b575..fddd86be 100644 --- a/fuzz/fuzz_targets/refcounted_model.rs +++ b/fuzz/fuzz_targets/refcounted_model.rs @@ -52,10 +52,7 @@ impl DbSimulator for Simulator { Operation::Dereference(k) => if let Entry::Occupied(mut e) = model.entry(k) { let counter = e.get_mut(); - *counter -= 1; - if *counter == 0 { - e.remove_entry(); - } + *counter = counter.saturating_sub(1); }, Operation::Reference(k) => if let Entry::Occupied(mut e) = model.entry(k) { @@ -72,9 +69,20 @@ impl DbSimulator for Simulator { } } - fn model_content(model: &BTreeMap) -> Vec<(Vec, Vec)> { + fn model_required_content(model: &BTreeMap) -> Vec<(Vec, Vec)> { + model + .iter() + .filter_map(|(k, v)| if *v > 0 { Some((vec![*k], vec![*k])) } else { None }) + .collect::>() + } + + fn model_optional_content(model: &BTreeMap) -> Vec<(Vec, Vec)> { model.iter().map(|(k, _)| (vec![*k], vec![*k])).collect::>() } + + fn model_removed_content(_model: &BTreeMap) -> Vec> { + Vec::new() + } } fuzz_target!(|entry: (Config, Vec>)| { diff --git a/fuzz/fuzz_targets/simple_model.rs b/fuzz/fuzz_targets/simple_model.rs index c76f08e2..ff933858 100644 --- a/fuzz/fuzz_targets/simple_model.rs +++ b/fuzz/fuzz_targets/simple_model.rs @@ -8,15 +8,21 @@ use libfuzzer_sys::fuzz_target; use parity_db_fuzz::*; use std::{ - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, BTreeSet, HashMap}, path::Path, }; +#[derive(Default)] +struct Model { + current: BTreeMap, + removed: BTreeSet, +} + struct Simulator; impl DbSimulator for Simulator { type Operation = (u8, Option); - type Model = BTreeMap; + type Model = Model; fn build_options(config: &Config, path: &Path) -> parity_db::Options { parity_db::Options { @@ -34,12 +40,14 @@ impl DbSimulator for Simulator { } } - fn apply_operation_on_model(operation: &(u8, Option), model: &mut Self::Model) { + fn apply_operation_on_model(operation: &(u8, Option), model: &mut Model) { let (k, v) = operation; if let Some(v) = *v { - model.insert(*k, v); + model.current.insert(*k, v); + model.removed.remove(k); } else { - model.remove(k); + model.current.remove(k); + model.removed.insert(*k); } } @@ -52,8 +60,16 @@ impl DbSimulator for Simulator { } } - fn model_content(model: &BTreeMap) -> Vec<(Vec, Vec)> { - model.iter().map(|(k, v)| (vec![*k], vec![*v])).collect::>() + fn model_required_content(model: &Model) -> Vec<(Vec, Vec)> { + model.current.iter().map(|(k, v)| (vec![*k], vec![*v])).collect::>() + } + + fn model_optional_content(model: &Model) -> Vec<(Vec, Vec)> { + Self::model_required_content(model) + } + + fn model_removed_content(model: &Model) -> Vec> { + model.removed.iter().map(|k| vec![*k]).collect() } } diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index e5f5c849..f963fa51 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -2,7 +2,7 @@ // This file is dual-licensed as Apache-2.0 or MIT. use arbitrary::Arbitrary; -use std::{fmt::Debug, path::Path}; +use std::{cmp::Ordering, fmt::Debug, path::Path}; use tempfile::tempdir; #[derive(Arbitrary, Debug, Clone, Copy)] @@ -45,7 +45,11 @@ pub trait DbSimulator { fn map_operation(operation: &Self::Operation) -> parity_db::Operation, Vec>; - fn model_content(model: &Self::Model) -> Vec<(Vec, Vec)>; + fn model_required_content(model: &Self::Model) -> Vec<(Vec, Vec)>; + + fn model_optional_content(model: &Self::Model) -> Vec<(Vec, Vec)>; + + fn model_removed_content(model: &Self::Model) -> Vec>; fn simulate(config: Config, actions: Vec>) { let dir = tempdir().unwrap(); @@ -79,11 +83,7 @@ pub trait DbSimulator { Self::apply_operation_on_model(o, &mut model); } retry_operation(|| { - check_db_and_model_are_equals( - &db, - Self::model_content(&model), - config.btree_index, - ) + Self::check_db_and_model_are_equals(&db, &model, config.btree_index) }) .unwrap(); } @@ -94,11 +94,8 @@ pub trait DbSimulator { parity_db::Db::open_or_create(&options) } .and_then(|db| { - match check_db_and_model_are_equals( - &db, - Self::model_content(&model), - config.btree_index, - )? { + match Self::check_db_and_model_are_equals(&db, &model, config.btree_index)? + { Ok(()) => Ok(db), Err(_) => Err(parity_db::Error::Corruption("Instrumented failure".into())), @@ -137,10 +134,7 @@ pub trait DbSimulator { // We look for the matching model let mut model = Self::Model::default(); for action in actions { - if check_db_and_model_are_equals(&db, Self::model_content(&model), is_db_b_tree) - .unwrap() - .is_ok() - { + if Self::check_db_and_model_are_equals(&db, &model, is_db_b_tree).unwrap().is_ok() { return (db, model) //We found it! } @@ -150,13 +144,62 @@ pub trait DbSimulator { } } } - if let Err(e) = - check_db_and_model_are_equals(&db, Self::model_content(&model), is_db_b_tree).unwrap() - { + if let Err(e) = Self::check_db_and_model_are_equals(&db, &model, is_db_b_tree).unwrap() { panic!("Not able to recover to a proper state when coming back from an instrumented failure: {}", e) } (db, model) } + + fn check_db_and_model_are_equals( + db: &parity_db::Db, + model: &Self::Model, + is_db_b_tree: bool, + ) -> parity_db::Result> { + for (k, v) in Self::model_required_content(model) { + if db.get(0, &k)?.as_ref() != Some(&v) { + return Ok(Err(format!("The value {:?} for key {:?} is not in the database", k, v))) + } + } + for k in Self::model_removed_content(model) { + if db.get(0, &k)?.is_some() { + return Ok(Err(format!("The key {:?} should not be in the database anymore", k))) + } + } + + if is_db_b_tree { + let mut model_content = Self::model_optional_content(model); + + // We check the BTree forward iterator + let mut db_iter = db.iter(0)?; + db_iter.seek_to_first()?; + let mut db_content = Vec::new(); + while let Some(e) = db_iter.next()? { + db_content.push(e); + } + if !is_slice_included_in_sorted(&db_content, &model_content, |a, b| a.cmp(b)) { + return Ok(Err(format!( + "The forward iterator for the db gives {:?} and not {:?}", + db_content, model_content + ))) + } + + // We check the BTree backward iterator + model_content.reverse(); + let mut db_iter = db.iter(0)?; + db_iter.seek_to_last()?; + let mut db_content = Vec::new(); + while let Some(e) = db_iter.prev()? { + db_content.push(e); + } + if !is_slice_included_in_sorted(&db_content, &model_content, |a, b| b.cmp(a)) { + return Ok(Err(format!( + "The backward iterator for the db gives {:?} and not {:?}", + db_content, model_content + ))) + } + } + Ok(Ok(())) + } } fn retry_operation<'a, T>(op: impl Fn() -> parity_db::Result + 'a) -> T { @@ -167,46 +210,21 @@ fn retry_operation<'a, T>(op: impl Fn() -> parity_db::Result + 'a) -> T { }) } -fn check_db_and_model_are_equals( - db: &parity_db::Db, - mut model_content: Vec<(Vec, Vec)>, - is_db_b_tree: bool, -) -> parity_db::Result> { - for (k, v) in &model_content { - if db.get(0, k)?.as_ref() != Some(v) { - return Ok(Err(format!("The value {:?} for key {:?} is not in the database", k, v))) - } - } - - if is_db_b_tree { - // We check the BTree forward iterator - let mut db_iter = db.iter(0)?; - db_iter.seek_to_first()?; - let mut db_content = Vec::new(); - while let Some(e) = db_iter.next()? { - db_content.push(e); - } - if db_content != model_content { - return Ok(Err(format!( - "The forward iterator for the db gives {:?} and not {:?}", - db_content, model_content - ))) - } - - // We check the BTree backward iterator - model_content.reverse(); - let mut db_iter = db.iter(0)?; - db_iter.seek_to_last()?; - let mut db_content = Vec::new(); - while let Some(e) = db_iter.prev()? { - db_content.push(e); - } - if db_content != model_content { - return Ok(Err(format!( - "The backward iterator for the db gives {:?} and not {:?}", - db_content, model_content - ))) +fn is_slice_included_in_sorted( + small: &[T], + large: &[T], + cmp: impl Fn(&T, &T) -> Ordering, +) -> bool { + let mut large = large.iter(); + for se in small { + loop { + let le = if let Some(le) = large.next() { le } else { return false }; + match cmp(se, le) { + Ordering::Less => return false, + Ordering::Greater => continue, + Ordering::Equal => break, + } } } - Ok(Ok(())) + true }