Various improvements to differential fuzzing

crates/fuzzing/src/generators/config.rs

@@ -36,12 +36,6 @@ impl Config {
     pub fn set_differential_config(&mut self) {
         let config = &mut self.module_config.config;
 
-        // Disable the start function for now.
-        //
-        // TODO: should probably allow this after testing it works with the new
-        // differential setup in all engines.
-        config.allow_start_export = false;
-
         // Make it more likely that there are types available to generate a
         // function with.
         config.min_types = 1;
@@ -54,7 +48,6 @@ impl Config {
         // Allow a memory to be generated, but don't let it get too large.
         // Additionally require the maximum size to guarantee that the growth
         // behavior is consistent across engines.
-        config.max_memories = 1;
         config.max_memory_pages = 10;
         config.memory_max_size_required = true;
 
@@ -65,7 +58,6 @@ impl Config {
         //
         // Note that while reference types are disabled below, only allow one
         // table.
-        config.max_tables = 1;
         config.max_table_elements = 1_000;
         config.table_max_size_required = true;
 
@@ -86,10 +78,10 @@ impl Config {
         } = &mut self.wasmtime.strategy
         {
             // One single-page memory
-            limits.memories = 1;
+            limits.memories = config.max_memories as u32;
             limits.memory_pages = 10;
 
-            limits.tables = 1;
+            limits.tables = config.max_tables as u32;
             limits.table_elements = 1_000;
 
             limits.size = 1_000_000;
@@ -329,16 +321,22 @@ impl<'a> Arbitrary<'a> for Config {
         if let InstanceAllocationStrategy::Pooling {
             instance_limits: limits,
             ..
-        } = &config.wasmtime.strategy
+        } = &mut config.wasmtime.strategy
         {
+            let cfg = &mut config.module_config.config;
             // If the pooling allocator is used, do not allow shared memory to
             // be created. FIXME: see
             // https://github.com/bytecodealliance/wasmtime/issues/4244.
-            config.module_config.config.threads_enabled = false;
+            cfg.threads_enabled = false;
 
             // Force the use of a normal memory config when using the pooling allocator and
             // limit the static memory maximum to be the same as the pooling allocator's memory
             // page limit.
+            if cfg.max_memory_pages < limits.memory_pages {
+                limits.memory_pages = cfg.max_memory_pages;
+            } else {
+                cfg.max_memory_pages = limits.memory_pages;
+            }
             config.wasmtime.memory_config = match config.wasmtime.memory_config {
                 MemoryConfig::Normal(mut config) => {
                     config.static_memory_maximum_size = Some(limits.memory_pages * 0x10000);
@@ -351,14 +349,10 @@ impl<'a> Arbitrary<'a> for Config {
                 }
             };
 
-            let cfg = &mut config.module_config.config;
-            cfg.max_memories = limits.memories as usize;
-            cfg.max_tables = limits.tables as usize;
-            cfg.max_memory_pages = limits.memory_pages;
-
-            // Force no aliases in any generated modules as they might count against the
-            // import limits above.
-            cfg.max_aliases = 0;
+            // Force this pooling allocator to always be able to accommodate the
+            // module that may be generated.
+            limits.memories = cfg.max_memories as u32;
+            limits.tables = cfg.max_tables as u32;
         }
 
         Ok(config)

crates/fuzzing/src/generators/module.rs

@@ -17,21 +17,27 @@ impl<'a> Arbitrary<'a> for ModuleConfig {
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<ModuleConfig> {
         let mut config = SwarmConfig::arbitrary(u)?;
 
-        // Allow multi-memory by default.
-        config.max_memories = config.max_memories.max(2);
+        // Allow multi-memory but make it unlikely
+        if u.ratio(1, 20)? {
+            config.max_memories = config.max_memories.max(2);
+        } else {
+            config.max_memories = 1;
+        }
 
         // Allow multi-table by default.
-        config.max_tables = config.max_tables.max(4);
+        if config.reference_types_enabled {
+            config.max_tables = config.max_tables.max(4);
+        }
 
         // Allow enabling some various wasm proposals by default. Note that
         // these are all unconditionally turned off even with
         // `SwarmConfig::arbitrary`.
-        config.memory64_enabled = u.arbitrary()?;
+        config.memory64_enabled = u.ratio(1, 20)?;
 
         // Allow the threads proposal if memory64 is not already enabled. FIXME:
         // to allow threads and memory64 to coexist, see
         // https://github.com/bytecodealliance/wasmtime/issues/4267.
-        config.threads_enabled = !config.memory64_enabled && u.arbitrary()?;
+        config.threads_enabled = !config.memory64_enabled && u.ratio(1, 20)?;
 
         Ok(ModuleConfig { config })
     }

crates/fuzzing/src/oracles.rs

@@ -19,7 +19,7 @@ pub mod engine;
 mod stacks;
 
 use self::diff_wasmtime::WasmtimeInstance;
-use self::engine::DiffInstance;
+use self::engine::{DiffEngine, DiffInstance};
 use crate::generators::{self, DiffValue, DiffValueType};
 use arbitrary::Arbitrary;
 pub use stacks::check_stacks;
@@ -330,24 +330,28 @@ pub fn instantiate_with_dummy(store: &mut Store<StoreLimits>, module: &Module) -
 /// Evaluate the function identified by `name` in two different engine
 /// instances--`lhs` and `rhs`.
 ///
+/// Returns `Ok(true)` if more evaluations can happen or `Ok(false)` if the
+/// instances may have drifted apart and no more evaluations can happen.
+///
 /// # Panics
 ///
 /// This will panic if the evaluation is different between engines (e.g.,
 /// results are different, hashed instance is different, one side traps, etc.).
 pub fn differential(
     lhs: &mut dyn DiffInstance,
+    lhs_engine: &dyn DiffEngine,
     rhs: &mut WasmtimeInstance,
     name: &str,
     args: &[DiffValue],
     result_tys: &[DiffValueType],
-) -> anyhow::Result<()> {
+) -> anyhow::Result<bool> {
     log::debug!("Evaluating: `{}` with {:?}", name, args);
     let lhs_results = match lhs.evaluate(name, args, result_tys) {
         Ok(Some(results)) => Ok(results),
         Err(e) => Err(e),
         // this engine couldn't execute this type signature, so discard this
         // execution by returning success.
-        Ok(None) => return Ok(()),
+        Ok(None) => return Ok(true),
     };
     log::debug!(" -> results on {}: {:?}", lhs.name(), &lhs_results);
 
@@ -360,11 +364,24 @@ pub fn differential(
     match (lhs_results, rhs_results) {
         // If the evaluation succeeds, we compare the results.
         (Ok(lhs_results), Ok(rhs_results)) => assert_eq!(lhs_results, rhs_results),
-        // Both sides failed--this is an acceptable result (e.g., both sides
-        // trap at a divide by zero). We could compare the error strings perhaps
-        // (since the `lhs` and `rhs` could be failing for different reasons)
-        // but this seems good enough for now.
-        (Err(_), Err(_)) => {}
+
+        // Both sides failed. If either one hits a stack overflow then that's an
+        // engine defined limit which means we can no longer compare the state
+        // of the two instances, so `false` is returned and nothing else is
+        // compared.
+        //
+        // Otherwise, though, the same error should have popped out and this
+        // falls through to checking the intermediate state otherwise.
+        (Err(lhs), Err(rhs)) => {
+            let err = rhs.downcast::<Trap>().expect("not a trap");
+            let poisoned = err.trap_code() == Some(TrapCode::StackOverflow)
+                || lhs_engine.is_stack_overflow(&lhs);
+
+            if poisoned {
+                return Ok(false);
+            }
+            lhs_engine.assert_error_match(&err, &lhs);
+        }
         // A real bug is found if only one side fails.
         (Ok(_), Err(_)) => panic!("only the `rhs` ({}) failed for this input", rhs.name()),
         (Err(_), Ok(_)) => panic!("only the `lhs` ({}) failed for this input", lhs.name()),
@@ -392,7 +409,7 @@ pub fn differential(
         panic!("memories have differing values");
     }
 
-    Ok(())
+    Ok(true)
 }
 
 /// Invoke the given API calls.

crates/fuzzing/src/oracles/diff_spec.rs

@@ -59,10 +59,16 @@ impl DiffEngine for SpecInterpreter {
         }))
     }
 
-    fn assert_error_match(&self, trap: &Trap, err: Error) {
+    fn assert_error_match(&self, trap: &Trap, err: &Error) {
         // TODO: implement this for the spec interpreter
         drop((trap, err));
     }
+
+    fn is_stack_overflow(&self, err: &Error) -> bool {
+        // TODO: implement this for the spec interpreter
+        drop(err);
+        false
+    }
 }
 
 struct SpecInstance {

crates/fuzzing/src/oracles/diff_v8.rs

@@ -35,6 +35,10 @@ impl V8Engine {
             bail!("memory64 not enabled by default in v8");
         }
 
+        if config.config.max_memories > 1 {
+            bail!("multi-memory not enabled by default in v8");
+        }
+
         Ok(Self {
             isolate: Rc::new(RefCell::new(v8::Isolate::new(Default::default()))),
         })
@@ -78,7 +82,7 @@ impl DiffEngine for V8Engine {
         }))
     }
 
-    fn assert_error_match(&self, wasmtime: &Trap, err: Error) {
+    fn assert_error_match(&self, wasmtime: &Trap, err: &Error) {
         let v8 = err.to_string();
         let wasmtime_msg = wasmtime.to_string();
         let verify_wasmtime = |msg: &str| {
@@ -148,6 +152,10 @@ impl DiffEngine for V8Engine {
 
         verify_wasmtime("not possibly present in an error, just panic please");
     }
+
+    fn is_stack_overflow(&self, err: &Error) -> bool {
+        err.to_string().contains("Maximum call stack size exceeded")
+    }
 }
 
 struct V8Instance {

crates/fuzzing/src/oracles/diff_wasmi.rs

@@ -3,7 +3,7 @@
 use crate::generators::{DiffValue, DiffValueType, ModuleConfig};
 use crate::oracles::engine::{DiffEngine, DiffInstance};
 use anyhow::{bail, Context, Error, Result};
-use wasmtime::Trap;
+use wasmtime::{Trap, TrapCode};
 
 /// A wrapper for `wasmi` as a [`DiffEngine`].
 pub struct WasmiEngine;
@@ -36,6 +36,9 @@ impl WasmiEngine {
         if config.config.threads_enabled {
             bail!("wasmi does not support threads");
         }
+        if config.config.max_memories > 1 {
+            bail!("wasmi does not support multi-memory");
+        }
         Ok(Self)
     }
 }
@@ -49,13 +52,86 @@ impl DiffEngine for WasmiEngine {
         let module = wasmi::Module::from_buffer(wasm).context("unable to validate Wasm module")?;
         let instance = wasmi::ModuleInstance::new(&module, &wasmi::ImportsBuilder::default())
             .context("unable to instantiate module in wasmi")?;
-        let instance = instance.assert_no_start();
+        let instance = instance.run_start(&mut wasmi::NopExternals)?;
         Ok(Box::new(WasmiInstance { module, instance }))
     }
 
-    fn assert_error_match(&self, trap: &Trap, err: Error) {
-        // TODO: should implement this for `wasmi`
-        drop((trap, err));
+    fn assert_error_match(&self, trap: &Trap, err: &Error) {
+        // Acquire a `wasmi::Trap` from the wasmi error which we'll use to
+        // assert that it has the same kind of trap as the wasmtime-based trap.
+        let wasmi = match err.downcast_ref::<wasmi::Error>() {
+            Some(wasmi::Error::Trap(trap)) => trap,
+
+            // Out-of-bounds data segments turn into this category which
+            // Wasmtime reports as a `MemoryOutOfBounds`.
+            Some(wasmi::Error::Memory(msg)) => {
+                assert_eq!(
+                    trap.trap_code(),
+                    Some(TrapCode::MemoryOutOfBounds),
+                    "wasmtime error did not match wasmi: {msg}"
+                );
+                return;
+            }
+
+            // Ignore this for now, looks like "elements segment does not fit"
+            // falls into this category and to avoid doing string matching this
+            // is just ignored.
+            Some(wasmi::Error::Instantiation(msg)) => {
+                log::debug!("ignoring wasmi instantiation error: {msg}");
+                return;
+            }
+
+            Some(other) => panic!("unexpected wasmi error: {}", other),
+
+            None => err
+                .downcast_ref::<wasmi::Trap>()
+                .expect(&format!("not a trap: {:?}", err)),
+        };
+        match wasmi.kind() {
+            wasmi::TrapKind::StackOverflow => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::StackOverflow))
+            }
+            wasmi::TrapKind::MemoryAccessOutOfBounds => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::MemoryOutOfBounds))
+            }
+            wasmi::TrapKind::Unreachable => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::UnreachableCodeReached))
+            }
+            wasmi::TrapKind::TableAccessOutOfBounds => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::TableOutOfBounds))
+            }
+            wasmi::TrapKind::ElemUninitialized => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::IndirectCallToNull))
+            }
+            wasmi::TrapKind::DivisionByZero => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::IntegerDivisionByZero))
+            }
+            wasmi::TrapKind::IntegerOverflow => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::IntegerOverflow))
+            }
+            wasmi::TrapKind::InvalidConversionToInt => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::BadConversionToInteger))
+            }
+            wasmi::TrapKind::UnexpectedSignature => {
+                assert_eq!(trap.trap_code(), Some(TrapCode::BadSignature))
+            }
+            wasmi::TrapKind::Host(_) => unreachable!(),
+        }
+    }
+
+    fn is_stack_overflow(&self, err: &Error) -> bool {
+        let trap = match err.downcast_ref::<wasmi::Error>() {
+            Some(wasmi::Error::Trap(trap)) => trap,
+            Some(_) => return false,
+            None => match err.downcast_ref::<wasmi::Trap>() {
+                Some(trap) => trap,
+                None => return false,
+            },
+        };
+        match trap.kind() {
+            wasmi::TrapKind::StackOverflow => true,
+            _ => false,
+        }
     }
 }
 

crates/fuzzing/src/oracles/diff_wasmtime.rs

@@ -5,7 +5,7 @@ use crate::oracles::dummy;
 use crate::oracles::engine::DiffInstance;
 use crate::oracles::{compile_module, engine::DiffEngine, StoreLimits};
 use anyhow::{Context, Error, Result};
-use wasmtime::{Extern, FuncType, Instance, Module, Store, Trap, Val};
+use wasmtime::{Extern, FuncType, Instance, Module, Store, Trap, TrapCode, Val};
 
 /// A wrapper for using Wasmtime as a [`DiffEngine`].
 pub struct WasmtimeEngine {
@@ -34,8 +34,10 @@ impl DiffEngine for WasmtimeEngine {
         Ok(Box::new(instance))
     }
 
-    fn assert_error_match(&self, trap: &Trap, err: Error) {
-        let trap2 = err.downcast::<Trap>().unwrap();
+    fn assert_error_match(&self, trap: &Trap, err: &Error) {
+        let trap2 = err
+            .downcast_ref::<Trap>()
+            .expect(&format!("not a trap: {:?}", err));
         assert_eq!(
             trap.trap_code(),
             trap2.trap_code(),
@@ -44,6 +46,13 @@ impl DiffEngine for WasmtimeEngine {
             trap2
         );
     }
+
+    fn is_stack_overflow(&self, err: &Error) -> bool {
+        match err.downcast_ref::<Trap>() {
+            Some(trap) => trap.trap_code() == Some(TrapCode::StackOverflow),
+            None => false,
+        }
+    }
 }
 
 /// A wrapper around a Wasmtime instance.