From d7a2bf94b86168c508989b52330a562a625ad1d3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 1 Sep 2022 18:15:57 -0700 Subject: [PATCH 1/5] Improve wasmi differential fuzzer * Support modules with a `start` function * Implement trap-matching to ensure that wasmi and Wasmtime both report the same flavor of trap. --- crates/fuzzing/src/generators/config.rs | 6 --- crates/fuzzing/src/oracles/diff_wasmi.rs | 67 ++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/crates/fuzzing/src/generators/config.rs b/crates/fuzzing/src/generators/config.rs index 14463dfa78c0..8e9011fc1147 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; diff --git a/crates/fuzzing/src/oracles/diff_wasmi.rs b/crates/fuzzing/src/oracles/diff_wasmi.rs index efc9160c0b94..cd95a14bf986 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,69 @@ 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)); + // 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::() { + Ok(wasmi::Error::Trap(trap)) => trap, + + // Out-of-bounds data segments turn into this category which + // Wasmtime reports as a `MemoryOutOfBounds`. + Ok(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. + Ok(wasmi::Error::Instantiation(msg)) => { + log::debug!("ignoring wasmi instantiation error: {msg}"); + return; + } + + Ok(other) => panic!("unexpected wasmi error: {}", other), + + Err(err) => err.downcast::().unwrap(), + }; + 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!(), + } } } From faa062887aaece15e45b0c5206c2620f9618cb4c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 1 Sep 2022 18:16:31 -0700 Subject: [PATCH 2/5] Support differential fuzzing where no engines match Locally I was attempting to run against just one wasm engine with `ALLOWED_ENGINES=wasmi` but the fuzzer quickly panicked because the generated test case didn't match wasmi's configuration. This commit updates engine-selection in the differential fuzzer to return `None` if no engine is applicable, throwing out the test case. This won't be hit at all with oss-fuzz-based runs but for local runs it'll be useful to have. --- crates/fuzzing/src/oracles/engine.rs | 9 ++++++--- fuzz/fuzz_targets/differential.rs | 9 +++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/fuzzing/src/oracles/engine.rs b/crates/fuzzing/src/oracles/engine.rs index bcb76b49403c..9637e31d8184 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. diff --git a/fuzz/fuzz_targets/differential.rs b/fuzz/fuzz_targets/differential.rs index 0d24eb94b0f4..eab9d03f0071 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()); From d8faf0f35cdf5b2d1cb0c1629b83432f9f852f04 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 1 Sep 2022 18:27:04 -0700 Subject: [PATCH 3/5] Improve proposal support in differential fuzzer * De-prioritize unstable wasm proposals such as multi-memory and memory64 by making them more unlikely with `Unstructured::ratio`. * Allow fuzzing multi-table (reference types) and multi-memory by avoiding setting their maximums to 1 in `set_differential_config`. * Update selection of the pooling strategy to unconditionally support the selected module config rather than the other way around. --- crates/fuzzing/src/generators/config.rs | 20 ++++++++------------ crates/fuzzing/src/generators/module.rs | 16 +++++++++++----- crates/fuzzing/src/oracles/diff_v8.rs | 4 ++++ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/crates/fuzzing/src/generators/config.rs b/crates/fuzzing/src/generators/config.rs index 8e9011fc1147..f3cff99eeb79 100644 --- a/crates/fuzzing/src/generators/config.rs +++ b/crates/fuzzing/src/generators/config.rs @@ -48,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; @@ -59,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; @@ -80,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; @@ -323,7 +321,7 @@ impl<'a> Arbitrary<'a> for Config { if let InstanceAllocationStrategy::Pooling { instance_limits: limits, .. - } = &config.wasmtime.strategy + } = &mut config.wasmtime.strategy { // If the pooling allocator is used, do not allow shared memory to // be created. FIXME: see @@ -345,14 +343,12 @@ impl<'a> Arbitrary<'a> for Config { } }; + // Force this pooling allocator to always be able to accomodate the + // module that may be generated. 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; + limits.memories = cfg.max_memories as u32; + limits.tables = cfg.max_tables as u32; + limits.memory_pages = cfg.max_memory_pages; } 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/diff_v8.rs b/crates/fuzzing/src/oracles/diff_v8.rs index f0e1a1a38cfb..4a390689c2c1 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()))), }) From c2bb4e1b43533cc6b705620841c08dc1e84f3c9f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 1 Sep 2022 20:05:56 -0700 Subject: [PATCH 4/5] Improve handling of traps in differential fuzzing This commit fixes an issue found via local fuzzing where engines were reporting different results but the underlying reason for this was that one engine was hitting stack overflow before the other. To fix the underlying issue I updated the execution to check for stack overflow and, if hit, it discards the entire fuzz test case from then on. The rationale behind this is that each engine can have unique limits for stack overflow. One test case I was looking at for example would stack overflow at less than 1000 frames with epoch interruption enabled but would stack overflow at more than 1000 frames with it disabled. This means that the state after the trap started to diverge and it looked like the engines produced different results. While I was at it I also improved the "function call returned a trap" case to compare traps to make sure the same trap reason popped out. --- crates/fuzzing/src/oracles.rs | 35 +++++++++++++++------ crates/fuzzing/src/oracles/diff_spec.rs | 8 ++++- crates/fuzzing/src/oracles/diff_v8.rs | 6 +++- crates/fuzzing/src/oracles/diff_wasmi.rs | 31 +++++++++++++----- crates/fuzzing/src/oracles/diff_wasmtime.rs | 15 +++++++-- crates/fuzzing/src/oracles/engine.rs | 6 +++- fuzz/fuzz_targets/differential.rs | 19 ++++++++--- 7 files changed, 93 insertions(+), 27 deletions(-) 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 4a390689c2c1..d5d7ac0cb821 100644 --- a/crates/fuzzing/src/oracles/diff_v8.rs +++ b/crates/fuzzing/src/oracles/diff_v8.rs @@ -82,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| { @@ -152,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 cd95a14bf986..2a34b6f379cc 100644 --- a/crates/fuzzing/src/oracles/diff_wasmi.rs +++ b/crates/fuzzing/src/oracles/diff_wasmi.rs @@ -56,15 +56,15 @@ impl DiffEngine for WasmiEngine { Ok(Box::new(WasmiInstance { module, instance })) } - fn assert_error_match(&self, trap: &Trap, err: Error) { + 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::() { - Ok(wasmi::Error::Trap(trap)) => 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`. - Ok(wasmi::Error::Memory(msg)) => { + Some(wasmi::Error::Memory(msg)) => { assert_eq!( trap.trap_code(), Some(TrapCode::MemoryOutOfBounds), @@ -76,14 +76,16 @@ impl DiffEngine for WasmiEngine { // 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. - Ok(wasmi::Error::Instantiation(msg)) => { + Some(wasmi::Error::Instantiation(msg)) => { log::debug!("ignoring wasmi instantiation error: {msg}"); return; } - Ok(other) => panic!("unexpected wasmi error: {}", other), + Some(other) => panic!("unexpected wasmi error: {}", other), - Err(err) => err.downcast::().unwrap(), + None => err + .downcast_ref::() + .expect(&format!("not a trap: {:?}", err)), }; match wasmi.kind() { wasmi::TrapKind::StackOverflow => { @@ -116,6 +118,21 @@ impl DiffEngine for WasmiEngine { 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, + } + } } /// A wrapper for `wasmi` Wasm instances. 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 9637e31d8184..b69ac532a69d 100644 --- a/crates/fuzzing/src/oracles/engine.rs +++ b/crates/fuzzing/src/oracles/engine.rs @@ -76,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 eab9d03f0071..bb726e323635 100644 --- a/fuzz/fuzz_targets/differential.rs +++ b/fuzz/fuzz_targets/differential.rs @@ -106,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!( @@ -117,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 @@ -128,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, @@ -137,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; } From 6898c183d91f3828104071b0890c96152cc33fe8 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 2 Sep 2022 07:03:39 -0700 Subject: [PATCH 5/5] Fix fuzzer tests --- crates/fuzzing/src/generators/config.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/fuzzing/src/generators/config.rs b/crates/fuzzing/src/generators/config.rs index f3cff99eeb79..6ee459a7af3f 100644 --- a/crates/fuzzing/src/generators/config.rs +++ b/crates/fuzzing/src/generators/config.rs @@ -323,14 +323,20 @@ impl<'a> Arbitrary<'a> for Config { .. } = &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); @@ -343,12 +349,10 @@ impl<'a> Arbitrary<'a> for Config { } }; - // Force this pooling allocator to always be able to accomodate the + // Force this pooling allocator to always be able to accommodate the // module that may be generated. - let cfg = &mut config.module_config.config; limits.memories = cfg.max_memories as u32; limits.tables = cfg.max_tables as u32; - limits.memory_pages = cfg.max_memory_pages; } Ok(config)