differential_wasmi fuzzing: reject all modules with FP ops for now.

NaN handling is nondeterministic, so this creates fuzzing failures that
we shouldn't care about. We should have decent coverage of weird FP
corner cases from the spec tests anyway.

Cargo.toml

@@ -96,3 +96,7 @@ harness = false
 
 [profile.dev.package.backtrace]
 debug = false # FIXME(#1813)
+
+[patch.crates-io.wasm-smith]
+git = "https://github.com/bytecodealliance/wasm-tools"
+rev = "8f08547b061ac9dd0f986f22809e4a62cb910165"

crates/fuzzing/Cargo.toml

@@ -16,7 +16,7 @@ wasmparser = "0.68.0"
 wasmprinter = "0.2.15"
 wasmtime = { path = "../wasmtime" }
 wasmtime-wast = { path = "../wast" }
-wasm-smith = "0.1.10"
+wasm-smith = "0.1.11"
 wasmi = "0.7.0"
 
 [dev-dependencies]

crates/fuzzing/src/oracles.rs

@@ -477,125 +477,113 @@ pub fn table_ops(config: crate::generators::Config, ops: crate::generators::tabl
     }
 }
 
-/// Do we accept the given wasm module for differential fuzzing between Wasmtime and wasmi? We want
-/// to return early for modules that don't meet our criteria so that the fuzzer learns that these
-/// are not interesting (as they will not lead to additional coverage).
-///
-/// If accepted, returns the name of the memory we will diff and the function we will call.
-fn differential_wasmi_accept_module(wasm: &[u8]) -> Option<(String, String)> {
-    // Find the names of the first memory and first export.
-    let mut memory_name = None;
-    let mut func_name = None;
-    let parser = wasmparser::Parser::new(0);
-    for payload in parser.parse_all(&wasm[..]) {
-        let payload = match payload {
-            Ok(p) => p,
-            Err(_) => return None,
-        };
-        match payload {
-            wasmparser::Payload::ExportSection(mut e) => {
-                for _ in 0..e.get_count() {
-                    let exp = match e.read() {
-                        Ok(exp) => exp,
-                        Err(_) => return None,
-                    };
-                    match exp.kind {
-                        wasmparser::ExternalKind::Memory => {
-                            memory_name = Some(exp.field.to_string());
-                        }
-                        wasmparser::ExternalKind::Function => {
-                            func_name = Some(exp.field.to_string());
-                        }
-                        _ => {}
-                    }
-                    if memory_name.is_some() && func_name.is_some() {
-                        break;
-                    }
-                }
-            }
-            wasmparser::Payload::MemorySection(mut m) => {
-                for _ in 0..m.get_count() {
-                    let mem = match m.read() {
-                        Ok(mem) => mem,
-                        Err(_) => return None,
-                    };
-                    match mem {
-                        wasmparser::MemoryType::M32 { limits, .. } => {
-                            if limits.initial > 100 {
-                                return None;
-                            }
-                            if limits.maximum.is_none() || limits.maximum.unwrap() > 100 {
-                                return None;
-                            }
-                        }
-                        wasmparser::MemoryType::M64 { .. } => {
-                            // Disallow 64-bit memories altogether.
-                            return None;
-                        }
-                    }
-                }
-            }
-            _ => {}
-        }
+/// Configuration options for wasm-smith such that generated modules always
+/// conform to certain specifications.
+#[derive(Default, Debug)]
+pub struct DifferentialWasmiModuleConfig;
+
+impl arbitrary::Arbitrary for DifferentialWasmiModuleConfig {
+    fn arbitrary(_: &mut arbitrary::Unstructured) -> arbitrary::Result<Self> {
+        Ok(Self)
     }
-    if !memory_name.is_some() || !func_name.is_some() {
-        return None;
+}
+
+impl wasm_smith::Config for DifferentialWasmiModuleConfig {
+    fn allow_start_export(&self) -> bool {
+        false
     }
 
-    let memory_name = memory_name.unwrap();
-    let func_name = func_name.unwrap();
-    Some((memory_name, func_name))
-}
+    fn min_funcs(&self) -> usize {
+        1
+    }
 
-/// Perform differential execution between Cranelift and wasmi, diffing the resulting memory image
-/// when execution terminates. This relies on the module-under-test to be instrumented to bound the
-/// execution time.
-pub fn differential_wasmi_execution(
-    module: &wasm_smith::Module,
-    config: &crate::generators::Config,
-) {
-    crate::init_fuzzing();
+    fn max_funcs(&self) -> usize {
+        1
+    }
 
-    let wasm = module.to_bytes();
+    fn min_memories(&self) -> u32 {
+        1
+    }
 
-    let (memory_name, func_name) = match differential_wasmi_accept_module(&wasm[..]) {
-        Some(pair) => pair,
-        None => return,
-    };
+    fn max_memories(&self) -> u32 {
+        1
+    }
 
-    // Use wasmi initialization sequence to validate requirements (teach fuzzer which exports we
-    // want) as well.
-    let wasmi_module = match wasmi::Module::from_buffer(&wasm[..]) {
-        Ok(m) => m,
-        Err(_) => return,
-    };
-    let wasmi_instance =
-        match wasmi::ModuleInstance::new(&wasmi_module, &wasmi::ImportsBuilder::default()) {
-            Ok(inst) => inst,
-            Err(_) => return,
-        };
-    if wasmi_instance.has_start() {
-        return;
+    fn max_imports(&self) -> usize {
+        0
+    }
+
+    fn min_exports(&self) -> usize {
+        2
+    }
+
+    fn max_memory_pages(&self) -> u32 {
+        1
+    }
+
+    fn memory_max_size_required(&self) -> bool {
+        true
     }
+}
+
+/// Perform differential execution between Cranelift and wasmi, diffing the
+/// resulting memory image when execution terminates. This relies on the
+/// module-under-test to be instrumented to bound the execution time. Invoke
+/// with a module generated by `wasm-smith` using the
+/// `DiferentialWasmiModuleConfig` configuration type for best results.
+///
+/// May return `None` if we early-out due to a rejected fuzz config; these
+/// should be rare if modules are generated appropriately.
+pub fn differential_wasmi_execution(wasm: &[u8], config: &crate::generators::Config) -> Option<()> {
+    crate::init_fuzzing();
+
+    // Instantiate wasmi module and instance.
+    let wasmi_module = wasmi::Module::from_buffer(&wasm[..]).ok()?;
+    let wasmi_instance =
+        wasmi::ModuleInstance::new(&wasmi_module, &wasmi::ImportsBuilder::default()).ok()?;
     let wasmi_instance = wasmi_instance.assert_no_start();
-    let wasmi_mem_export = wasmi_instance.export_by_name(&memory_name[..]).unwrap();
-    let wasmi_mem = wasmi_mem_export.as_memory().unwrap();
-    let wasmi_main_export = wasmi_instance.export_by_name(&func_name[..]).unwrap();
-    let wasmi_main = wasmi_main_export.as_func().unwrap();
-    let wasmi_val = match wasmi::FuncInstance::invoke(&wasmi_main, &[], &mut wasmi::NopExternals) {
-        Ok(Some(val)) => val,
-        _ => {
-            return;
-        }
-    };
 
-    let wasmtime_config = config.to_wasmtime();
+    // TODO(paritytech/wasmi#19): wasmi does not currently canonicalize NaNs. To avoid spurious
+    // fuzz failures, for now let's fuzz only integer Wasm programs.
+    if wasmi_module.deny_floating_point().is_err() {
+        return None;
+    }
+
+    // Instantiate wasmtime module and instance.
+    let mut wasmtime_config = config.to_wasmtime();
+    wasmtime_config.cranelift_nan_canonicalization(true);
     let wasmtime_engine = Engine::new(&wasmtime_config);
     let wasmtime_store = Store::new(&wasmtime_engine);
     let wasmtime_module =
         Module::new(&wasmtime_engine, &wasm).expect("Wasmtime can compile module");
     let wasmtime_instance = Instance::new(&wasmtime_store, &wasmtime_module, &[])
         .expect("Wasmtime can instantiate module");
+
+    // Introspect wasmtime module to find name of an exported function and of an
+    // exported memory. Stop when we have one of each. (According to the config
+    // above, there should be at most one of each.)
+    let (func_name, memory_name) = {
+        let mut func_name = None;
+        let mut memory_name = None;
+        for e in wasmtime_module.exports() {
+            match e.ty() {
+                wasmtime::ExternType::Func(..) => func_name = Some(e.name().to_string()),
+                wasmtime::ExternType::Memory(..) => memory_name = Some(e.name().to_string()),
+                _ => {}
+            }
+            if func_name.is_some() && memory_name.is_some() {
+                break;
+            }
+        }
+        (func_name?, memory_name?)
+    };
+
+    let wasmi_mem_export = wasmi_instance.export_by_name(&memory_name[..]).unwrap();
+    let wasmi_mem = wasmi_mem_export.as_memory().unwrap();
+    let wasmi_main_export = wasmi_instance.export_by_name(&func_name[..]).unwrap();
+    let wasmi_main = wasmi_main_export.as_func().unwrap();
+    let wasmi_val = wasmi::FuncInstance::invoke(&wasmi_main, &[], &mut wasmi::NopExternals).ok()?;
+
     let wasmtime_mem = wasmtime_instance
         .get_memory(&memory_name[..])
         .expect("memory export is present");
@@ -605,8 +593,7 @@ pub fn differential_wasmi_execution(
     let wasmtime_vals = wasmtime_main
         .call(&[])
         .expect("Wasmtime can execute function");
-    assert_eq!(wasmtime_vals.len(), 1, "function should return a value");
-    let wasmtime_val = &wasmtime_vals[0];
+    let wasmtime_val = wasmtime_vals.iter().next().cloned();
 
     debug!(
         "Successful execution: wasmi returned {:?}, wasmtime returned {:?}",
@@ -619,11 +606,12 @@ pub fn differential_wasmi_execution(
         }
     };
 
-    match (wasmi_val, wasmtime_val) {
-        (wasmi::RuntimeValue::I32(a), &Val::I32(b)) if a == b => {}
-        (wasmi::RuntimeValue::F32(a), &Val::F32(b)) if f32_equal(a.to_bits(), b) => {}
-        (wasmi::RuntimeValue::I64(a), &Val::I64(b)) if a == b => {}
-        (wasmi::RuntimeValue::F64(a), &Val::F64(b)) if f64_equal(a.to_bits(), b) => {}
+    match (wasmi_val, &wasmtime_val) {
+        (Some(wasmi::RuntimeValue::I32(a)), &Some(Val::I32(b))) if a == b => {}
+        (Some(wasmi::RuntimeValue::F32(a)), &Some(Val::F32(b))) if f32_equal(a.to_bits(), b) => {}
+        (Some(wasmi::RuntimeValue::I64(a)), &Some(Val::I64(b))) if a == b => {}
+        (Some(wasmi::RuntimeValue::F64(a)), &Some(Val::F64(b))) if f64_equal(a.to_bits(), b) => {}
+        (None, None) => {}
         _ => {
             show_wat();
             panic!(
@@ -633,7 +621,7 @@ pub fn differential_wasmi_execution(
         }
     }
 
-    if wasmi_mem.current_size().0 * 65536 != wasmtime_mem.data_size() {
+    if wasmi_mem.current_size().0 != wasmtime_mem.size() as usize {
         show_wat();
         panic!("resulting memories are not the same size");
     }
@@ -659,4 +647,6 @@ pub fn differential_wasmi_execution(
         show_wat();
         panic!("memory contents are not equal");
     }
+
+    Some(())
 }

fuzz/Cargo.toml

@@ -17,7 +17,7 @@ target-lexicon = "0.11"
 peepmatic-fuzzing = { path = "../cranelift/peepmatic/crates/fuzzing", optional = true }
 wasmtime = { path = "../crates/wasmtime" }
 wasmtime-fuzzing = { path = "../crates/fuzzing" }
-wasm-smith = "0.1.5"
+wasm-smith = "0.1.11"
 
 [[bin]]
 name = "compile"

fuzz/fuzz_targets/differential_wasmi.rs

@@ -3,8 +3,11 @@
 use libfuzzer_sys::fuzz_target;
 use wasmtime_fuzzing::{generators, oracles};
 
-fuzz_target!(|data: (generators::Config, wasm_smith::Module,)| {
+fuzz_target!(|data: (
+    generators::Config,
+    wasm_smith::ConfiguredModule<oracles::DifferentialWasmiModuleConfig>
+)| {
     let (config, mut wasm) = data;
     wasm.ensure_termination(1000);
-    oracles::differential_wasmi_execution(&wasm, &config);
+    oracles::differential_wasmi_execution(&wasm.to_bytes()[..], &config);
 });