diff --git a/crates/fuzzing/src/generators/config.rs b/crates/fuzzing/src/generators/config.rs index 14463dfa78c0..6ee459a7af3f 100644 --- a/crates/fuzzing/src/generators/config.rs +++ b/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) diff --git a/crates/fuzzing/src/generators/module.rs b/crates/fuzzing/src/generators/module.rs index 2c608acb488c..a33feb1db332 100644 --- a/crates/fuzzing/src/generators/module.rs +++ b/crates/fuzzing/src/generators/module.rs @@ -17,21 +17,27 @@ impl<'a> Arbitrary<'a> for ModuleConfig { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { 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 }) } diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index 45c05858c988..98845a19c41e 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/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, 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 { 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::().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. diff --git a/crates/fuzzing/src/oracles/diff_spec.rs b/crates/fuzzing/src/oracles/diff_spec.rs index d545d24ce0b1..d90b8d32bda7 100644 --- a/crates/fuzzing/src/oracles/diff_spec.rs +++ b/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 { diff --git a/crates/fuzzing/src/oracles/diff_v8.rs b/crates/fuzzing/src/oracles/diff_v8.rs index f0e1a1a38cfb..d5d7ac0cb821 100644 --- a/crates/fuzzing/src/oracles/diff_v8.rs +++ b/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 { diff --git a/crates/fuzzing/src/oracles/diff_wasmi.rs b/crates/fuzzing/src/oracles/diff_wasmi.rs index efc9160c0b94..2a34b6f379cc 100644 --- a/crates/fuzzing/src/oracles/diff_wasmi.rs +++ b/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::() { + 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::() + .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::() { + Some(wasmi::Error::Trap(trap)) => trap, + Some(_) => return false, + None => match err.downcast_ref::() { + Some(trap) => trap, + None => return false, + }, + }; + match trap.kind() { + wasmi::TrapKind::StackOverflow => true, + _ => false, + } } } diff --git a/crates/fuzzing/src/oracles/diff_wasmtime.rs b/crates/fuzzing/src/oracles/diff_wasmtime.rs index 3c86138f9e56..b3fa796d9d9f 100644 --- a/crates/fuzzing/src/oracles/diff_wasmtime.rs +++ b/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::().unwrap(); + fn assert_error_match(&self, trap: &Trap, err: &Error) { + let trap2 = err + .downcast_ref::() + .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::() { + Some(trap) => trap.trap_code() == Some(TrapCode::StackOverflow), + None => false, + } + } } /// A wrapper around a Wasmtime instance. diff --git a/crates/fuzzing/src/oracles/engine.rs b/crates/fuzzing/src/oracles/engine.rs index bcb76b49403c..b69ac532a69d 100644 --- a/crates/fuzzing/src/oracles/engine.rs +++ b/crates/fuzzing/src/oracles/engine.rs @@ -13,7 +13,7 @@ pub fn choose( u: &mut Unstructured<'_>, existing_config: &Config, allowed: &[&str], -) -> arbitrary::Result> { +) -> arbitrary::Result>> { // Filter out any engines that cannot match the `existing_config` or are not // `allowed`. let mut engines: Vec> = vec![]; @@ -54,13 +54,16 @@ pub fn choose( } } + if engines.is_empty() { + return Ok(None); + } + // Use the input of the fuzzer to pick an engine that we'll be fuzzing // Wasmtime against. - assert!(!engines.is_empty()); let index: usize = u.int_in_range(0..=engines.len() - 1)?; let engine = engines.swap_remove(index); log::debug!("selected engine: {}", engine.name()); - Ok(engine) + Ok(Some(engine)) } /// Provide a way to instantiate Wasm modules. @@ -73,7 +76,11 @@ pub trait DiffEngine { /// Tests that the wasmtime-originating `trap` matches the error this engine /// generated. - fn assert_error_match(&self, trap: &Trap, err: Error); + fn assert_error_match(&self, trap: &Trap, err: &Error); + + /// Returns whether the error specified from this engine might be stack + /// overflow. + fn is_stack_overflow(&self, err: &Error) -> bool; } /// Provide a way to evaluate Wasm functions--a Wasm instance implemented by a diff --git a/fuzz/fuzz_targets/differential.rs b/fuzz/fuzz_targets/differential.rs index 0d24eb94b0f4..bb726e323635 100644 --- a/fuzz/fuzz_targets/differential.rs +++ b/fuzz/fuzz_targets/differential.rs @@ -86,8 +86,13 @@ fn run(data: &[u8]) -> Result<()> { }; log_wasm(&wasm); - // Choose a left-hand side Wasm engine. - let mut lhs = engine::choose(&mut u, &config, unsafe { &ALLOWED_ENGINES })?; + // Choose a left-hand side Wasm engine. If no engine could be chosen then + // that means the configuration selected above doesn't match any allowed + // engine (configured via an env var) so the test case is thrown out. + let mut lhs = match engine::choose(&mut u, &config, unsafe { &ALLOWED_ENGINES })? { + Some(engine) => engine, + None => return Ok(()), + }; let lhs_instance = lhs.instantiate(&wasm); STATS.bump_engine(lhs.name()); @@ -101,7 +106,7 @@ fn run(data: &[u8]) -> Result<()> { (Ok(l), Ok(r)) => (l, r), (Err(l), Err(r)) => { let err = r.downcast::().expect("not a trap"); - lhs.assert_error_match(&err, l); + lhs.assert_error_match(&err, &l); return Ok(()); } (l, r) => panic!( @@ -112,7 +117,7 @@ fn run(data: &[u8]) -> Result<()> { }; // Call each exported function with different sets of arguments. - for (name, signature) in rhs_instance.exported_functions() { + 'outer: for (name, signature) in rhs_instance.exported_functions() { let mut invocations = 0; loop { let arguments = signature @@ -123,8 +128,9 @@ fn run(data: &[u8]) -> Result<()> { .results() .map(|t| DiffValueType::try_from(t).unwrap()) .collect::>(); - differential( + let ok = differential( lhs_instance.as_mut(), + lhs.as_ref(), &mut rhs_instance, &name, &arguments, @@ -132,12 +138,20 @@ fn run(data: &[u8]) -> Result<()> { ) .expect("failed to run differential evaluation"); + invocations += 1; + STATS.total_invocations.fetch_add(1, SeqCst); + + // If this differential execution has resulted in the two instances + // diverging in state we can't keep executing so don't execute any + // more functions. + if !ok { + break 'outer; + } + // We evaluate the same function with different arguments until we // hit a predetermined limit or we run out of unstructured data--it // does not make sense to re-evaluate the same arguments over and // over. - invocations += 1; - STATS.total_invocations.fetch_add(1, SeqCst); if invocations > NUM_INVOCATIONS || u.is_empty() { break; }