address incremental-serializer fuzzer failures

fuzz/fuzz_targets/deserialize_br_rand_tree.rs

@@ -11,7 +11,7 @@ fuzz_target!(|data: &[u8]| {
     let mut allocator = Allocator::new();
     let mut unstructured = arbitrary::Unstructured::new(data);
 
-    let program = make_tree::make_tree(&mut allocator, &mut unstructured);
+    let (program, _) = make_tree::make_tree(&mut allocator, &mut unstructured);
 
     let b1 = node_to_bytes_backrefs(&allocator, program).unwrap();
 

fuzz/fuzz_targets/incremental_serializer.rs

@@ -1,16 +1,18 @@
 #![no_main]
 
 mod make_tree;
+mod node_eq;
 
-use clvmr::serde::{node_from_bytes_backrefs, node_to_bytes, Serializer};
+use clvmr::serde::{node_from_bytes_backrefs, Serializer};
 use clvmr::{Allocator, NodePtr, SExp};
 use make_tree::make_tree_limits;
+use std::collections::HashMap;
 
 use libfuzzer_sys::fuzz_target;
 
 enum TreeOp {
     SExp(NodePtr),
-    Cons,
+    Cons(NodePtr),
 }
 
 // returns the new root (with a sentinel) as well as the sub-tree under the
@@ -30,6 +32,7 @@ fn insert_sentinel(
     let mut copy = Vec::new();
     let mut ops = vec![TreeOp::SExp(root)];
     let mut subtree: Option<NodePtr> = None;
+    let mut copied_nodes = HashMap::<NodePtr, NodePtr>::new();
 
     while let Some(op) = ops.pop() {
         match op {
@@ -44,22 +47,30 @@ fn insert_sentinel(
                     node_idx -= 1;
                     continue;
                 }
-                node_idx -= 1;
                 match a.sexp(node) {
                     SExp::Atom => {
+                        node_idx -= 1;
                         copy.push(node);
                     }
                     SExp::Pair(left, right) => {
-                        ops.push(TreeOp::Cons);
-                        ops.push(TreeOp::SExp(left));
-                        ops.push(TreeOp::SExp(right));
+                        if let Some(copied_node) = copied_nodes.get(&node) {
+                            copy.push(*copied_node);
+                        }
+                        else {
+                            node_idx -= 1;
+                            ops.push(TreeOp::Cons(node));
+                            ops.push(TreeOp::SExp(left));
+                            ops.push(TreeOp::SExp(right));
+                        }
                     }
                 }
             }
-            TreeOp::Cons => {
+            TreeOp::Cons(node) => {
                 let left = copy.pop().unwrap();
                 let right = copy.pop().unwrap();
-                copy.push(a.new_pair(left, right).unwrap());
+                let new_node = a.new_pair(left, right).unwrap();
+                copy.push(new_node);
+                copied_nodes.insert(node, new_node);
             }
         }
     }
@@ -81,22 +92,21 @@ fuzz_target!(|data: &[u8]| {
     let mut allocator = Allocator::new();
 
     // since we copy the tree, we must limit the number of pairs created, to not
-    // exceed the limit of the Allocator
-    let program = make_tree_limits(&mut allocator, &mut unstructured, 10_000_000, 10_000_000);
+    // exceed the limit of the Allocator. Since we run this test for every node
+    // in the resulting tree, a tree being too large causes the fuzzer to
+    // time-out.
+    let (program, node_count) = make_tree_limits(&mut allocator, &mut unstructured, 600_000, false);
 
     // this just needs to be a unique NodePtr, that won't appear in the tree
     let sentinel = allocator.new_pair(NodePtr::NIL, NodePtr::NIL).unwrap();
 
     let checkpoint = allocator.checkpoint();
     // count up intil we've used every node as the sentinel/cut-point
-    let mut node_idx = 0;
+    let node_idx = unstructured.int_in_range(0..=node_count).unwrap_or(5) as i32;
 
     // try to put the sentinel in all positions, to get full coverage
-    while let Some((first_step, second_step)) =
-        insert_sentinel(&mut allocator, program, node_idx, sentinel)
+    if let Some((first_step, second_step)) = insert_sentinel(&mut allocator, program, node_idx, sentinel)
     {
-        node_idx += 1;
-
         let mut ser = Serializer::new(Some(sentinel));
         let (done, _) = ser.add(&allocator, first_step).unwrap();
         assert!(!done);
@@ -106,11 +116,7 @@ fuzz_target!(|data: &[u8]| {
         // now, make sure that we deserialize to the exact same structure, by
         // comparing the uncompressed form
         let roundtrip = node_from_bytes_backrefs(&mut allocator, ser.get_ref()).unwrap();
-        let b1 = node_to_bytes(&allocator, roundtrip).unwrap();
-
-        let b2 = node_to_bytes(&allocator, program).unwrap();
-
-        assert_eq!(&hex::encode(&b1), &hex::encode(&b2));
+        assert!(node_eq::node_eq(&allocator, program, roundtrip));
 
         // free the memory used by the last iteration from the allocator,
         // otherwise we'll exceed the Allocator limits eventually

fuzz/fuzz_targets/make_tree.rs

@@ -17,21 +17,24 @@ enum NodeType {
 }
 
 #[allow(dead_code)]
-pub fn make_tree(a: &mut Allocator, unstructured: &mut Unstructured) -> NodePtr {
-    make_tree_limits(a, unstructured, 60_000_000, 60_000_000)
+pub fn make_tree(a: &mut Allocator, unstructured: &mut Unstructured) -> (NodePtr, u32) {
+    make_tree_limits(a, unstructured, 600_000, true)
 }
 
+/// returns an arbitrary CLVM tree structure and the number of (unique) nodes
+/// it's made up of. That's both pairs and atoms.
 pub fn make_tree_limits(
     a: &mut Allocator,
     unstructured: &mut Unstructured,
-    mut max_pairs: i64,
-    mut max_atoms: i64,
-) -> NodePtr {
+    mut max_nodes: i64,
+    reuse_nodes: bool,
+) -> (NodePtr, u32) {
     let mut previous_nodes = Vec::<NodePtr>::new();
     let mut value_stack = Vec::<NodePtr>::new();
     let mut op_stack = vec![Op::SubTree];
     // the number of Op::SubTree items on the op_stack
     let mut sub_trees: i64 = 1;
+    let mut counter = 0;
 
     while let Some(op) = op_stack.pop() {
         match op {
@@ -43,6 +46,7 @@ pub fn make_tree_limits(
                 } else {
                     a.new_pair(right, left).expect("out of memory (pair)")
                 };
+                counter += 1;
                 value_stack.push(pair);
                 previous_nodes.push(pair);
             }
@@ -55,15 +59,15 @@ pub fn make_tree_limits(
                         Err(..) => value_stack.push(NodePtr::NIL),
                         Ok(NodeType::Pair) => {
                             if sub_trees > unstructured.len() as i64
-                                || max_pairs <= 0
-                                || max_atoms <= 0
+                                || max_nodes <= 0
                             {
                                 // there isn't much entropy left, don't grow the
                                 // tree anymore
                                 value_stack.push(
+                                    if reuse_nodes {
                                     *unstructured
                                         .choose(&previous_nodes)
-                                        .unwrap_or(&NodePtr::NIL),
+                                        .unwrap_or(&NodePtr::NIL) } else { NodePtr::NIL }
                                 );
                             } else {
                                 // swap left and right arbitrarily, to avoid
@@ -74,11 +78,11 @@ pub fn make_tree_limits(
                                 op_stack.push(Op::SubTree);
                                 op_stack.push(Op::SubTree);
                                 sub_trees += 2;
-                                max_pairs -= 1;
-                                max_atoms -= 2;
+                                max_nodes -= 2;
                             }
                         }
                         Ok(NodeType::Bytes) => {
+                            counter += 1;
                             value_stack.push(match unstructured.arbitrary::<Vec<u8>>() {
                                 Err(..) => NodePtr::NIL,
                                 Ok(val) => {
@@ -89,6 +93,7 @@ pub fn make_tree_limits(
                             });
                         }
                         Ok(NodeType::U8) => {
+                            counter += 1;
                             value_stack.push(match unstructured.arbitrary::<u8>() {
                                 Err(..) => NodePtr::NIL,
                                 Ok(val) => a
@@ -97,6 +102,7 @@ pub fn make_tree_limits(
                             });
                         }
                         Ok(NodeType::U16) => {
+                            counter += 1;
                             value_stack.push(match unstructured.arbitrary::<u16>() {
                                 Err(..) => NodePtr::NIL,
                                 Ok(val) => a
@@ -105,16 +111,18 @@ pub fn make_tree_limits(
                             });
                         }
                         Ok(NodeType::U32) => {
+                            counter += 1;
                             value_stack.push(match unstructured.arbitrary::<u32>() {
                                 Err(..) => NodePtr::NIL,
                                 Ok(val) => a.new_number(val.into()).expect("out of memory (atom)"),
                             });
                         }
                         Ok(NodeType::Previous) => {
                             value_stack.push(
+                                if reuse_nodes {
                                 *unstructured
                                     .choose(&previous_nodes)
-                                    .unwrap_or(&NodePtr::NIL),
+                                    .unwrap_or(&NodePtr::NIL) } else { NodePtr::NIL }
                             );
                         }
                     }
@@ -123,5 +131,5 @@ pub fn make_tree_limits(
         }
     }
     assert_eq!(value_stack.len(), 1);
-    *value_stack.last().expect("internal error, empty stack")
+    (*value_stack.last().expect("internal error, empty stack"), counter)
 }

fuzz/fuzz_targets/node_eq.rs

@@ -1,12 +1,17 @@
 use clvmr::{Allocator, NodePtr, SExp};
+use std::collections::HashSet;
 
 /// compare two CLVM trees. Returns true if they are identical, false otherwise
 pub fn node_eq(allocator: &Allocator, lhs: NodePtr, rhs: NodePtr) -> bool {
     let mut stack = vec![(lhs, rhs)];
+    let mut visited = HashSet::<NodePtr>::new();
 
     while let Some((l, r)) = stack.pop() {
         match (allocator.sexp(l), allocator.sexp(r)) {
             (SExp::Pair(ll, lr), SExp::Pair(rl, rr)) => {
+                if !visited.insert(l) {
+                    continue;
+                }
                 stack.push((lr, rr));
                 stack.push((ll, rl));
             }

fuzz/fuzz_targets/object_cache.rs

@@ -5,8 +5,8 @@ mod make_tree;
 use clvmr::serde::{node_to_bytes, serialized_length, treehash, ObjectCache};
 use clvmr::{Allocator, NodePtr, SExp};
 use libfuzzer_sys::fuzz_target;
-
-use fuzzing_utils::{tree_hash, visit_tree};
+use std::collections::HashSet;
+use fuzzing_utils::tree_hash;
 
 enum Op {
     Cons,
@@ -43,19 +43,46 @@ fn compute_serialized_len(a: &Allocator, n: NodePtr) -> u64 {
     *stack.last().expect("internal error, empty stack")
 }
 
+fn pick_node(
+    a: &Allocator,
+    root: NodePtr,
+    mut node_idx: i32,
+) -> NodePtr {
+    let mut stack = vec![root];
+    let mut seen_node = HashSet::<NodePtr>::new();
+
+    while let Some(node) = stack.pop() {
+        if node_idx == 0 {
+            return node;
+        }
+        if !seen_node.insert(node) {
+            continue;
+        }
+        node_idx -= 1;
+        if let SExp::Pair(left, right) = a.sexp(node) {
+            stack.push(left);
+            stack.push(right);
+        }
+    }
+    NodePtr::NIL
+}
+
 fuzz_target!(|data: &[u8]| {
     let mut unstructured = arbitrary::Unstructured::new(data);
     let mut allocator = Allocator::new();
-    let program = make_tree::make_tree(&mut allocator, &mut unstructured);
+    let (tree, node_count) = make_tree::make_tree_limits(&mut allocator, &mut unstructured, 10_000, true);
 
     let mut hash_cache = ObjectCache::new(treehash);
     let mut length_cache = ObjectCache::new(serialized_length);
-    visit_tree(&allocator, program, |a, node| {
-        let expect_hash = tree_hash(a, node);
-        let expect_len = compute_serialized_len(a, node);
-        let computed_hash = hash_cache.get_or_calculate(a, &node, None).unwrap();
-        let computed_len = length_cache.get_or_calculate(a, &node, None).unwrap();
-        assert_eq!(computed_hash, &expect_hash);
-        assert_eq!(computed_len, &expect_len);
-    });
+
+    let node_idx = unstructured.int_in_range(0..=node_count).unwrap_or(5) as i32;
+
+    let node = pick_node(&allocator, tree, node_idx);
+
+    let expect_hash = tree_hash(&allocator, node);
+    let expect_len = compute_serialized_len(&allocator, node);
+    let computed_hash = hash_cache.get_or_calculate(&allocator, &node, None).unwrap();
+    let computed_len = length_cache.get_or_calculate(&allocator, &node, None).unwrap();
+    assert_eq!(computed_hash, &expect_hash);
+    assert_eq!(computed_len, &expect_len);
 });

fuzz/fuzz_targets/serializer.rs

@@ -14,7 +14,7 @@ use libfuzzer_sys::fuzz_target;
 fuzz_target!(|data: &[u8]| {
     let mut unstructured = arbitrary::Unstructured::new(data);
     let mut allocator = Allocator::new();
-    let program = make_tree::make_tree(&mut allocator, &mut unstructured);
+    let (program, _) = make_tree::make_tree(&mut allocator, &mut unstructured);
 
     let b1 = node_to_bytes_backrefs(&allocator, program).unwrap();