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
paritytech · Sep 22, 2022 · 3e988a7 · 3e988a7
1 parent 72dd091
commit 3e988a7
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 71 deletions.
diff --git 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
@@ -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<u8, u8>) -> Vec<(Vec<u8>, Vec<u8>)> {
+	fn model_required_content(model: &BTreeMap<u8, u8>) -> Vec<(Vec<u8>, Vec<u8>)> {
+		model
+			.iter()
+			.filter_map(|(k, v)| if *v > 0 { Some((vec![*k], vec![*k])) } else { None })
+			.collect::<Vec<_>>()
+	}
+
+	fn model_optional_content(model: &BTreeMap<u8, u8>) -> Vec<(Vec<u8>, Vec<u8>)> {
 		model.iter().map(|(k, _)| (vec![*k], vec![*k])).collect::<Vec<_>>()
 	}
+
+	fn model_removed_content(_model: &BTreeMap<u8, u8>) -> Vec<Vec<u8>> {
+		Vec::new()
+	}
 }
 
 fuzz_target!(|entry: (Config, Vec<Action<Operation>>)| {

diff --git 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<u8, u8>,
+	removed: BTreeSet<u8>,
+}
+
 struct Simulator;
 
 impl DbSimulator for Simulator {
 	type Operation = (u8, Option<u8>);
-	type Model = BTreeMap<u8, u8>;
+	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<u8>), model: &mut Self::Model) {
+	fn apply_operation_on_model(operation: &(u8, Option<u8>), 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<u8, u8>) -> Vec<(Vec<u8>, Vec<u8>)> {
-		model.iter().map(|(k, v)| (vec![*k], vec![*v])).collect::<Vec<_>>()
+	fn model_required_content(model: &Model) -> Vec<(Vec<u8>, Vec<u8>)> {
+		model.current.iter().map(|(k, v)| (vec![*k], vec![*v])).collect::<Vec<_>>()
+	}
+
+	fn model_optional_content(model: &Model) -> Vec<(Vec<u8>, Vec<u8>)> {
+		Self::model_required_content(model)
+	}
+
+	fn model_removed_content(model: &Model) -> Vec<Vec<u8>> {
+		model.removed.iter().map(|k| vec![*k]).collect()
 	}
 }
 

diff --git 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<u8>, Vec<u8>>;
 
-	fn model_content(model: &Self::Model) -> Vec<(Vec<u8>, Vec<u8>)>;
+	fn model_required_content(model: &Self::Model) -> Vec<(Vec<u8>, Vec<u8>)>;
+
+	fn model_optional_content(model: &Self::Model) -> Vec<(Vec<u8>, Vec<u8>)>;
+
+	fn model_removed_content(model: &Self::Model) -> Vec<Vec<u8>>;
 
 	fn simulate(config: Config, actions: Vec<Action<Self::Operation>>) {
 		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<Result<(), String>> {
+		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<T> + 'a) -> T {
@@ -167,46 +210,21 @@ fn retry_operation<'a, T>(op: impl Fn() -> parity_db::Result<T> + 'a) -> T {
 	})
 }
 
-fn check_db_and_model_are_equals(
-	db: &parity_db::Db,
-	mut model_content: Vec<(Vec<u8>, Vec<u8>)>,
-	is_db_b_tree: bool,
-) -> parity_db::Result<Result<(), String>> {
-	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<T>(
+	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
 }