From b73b831892c3f982d2158f53faebeac468f612dc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 29 Oct 2020 10:02:59 -0500 Subject: [PATCH] Replace binaryen -ttf based fuzzing with wasm-smith (#2336) This commit removes the binaryen support for fuzzing from wasmtime, instead switching over to `wasm-smith`. In general it's great to have what fuzzing we can, but our binaryen support suffers from a few issues: * The Rust crate, binaryen-sys, seems largely unmaintained at this point. While we could likely take ownership and/or send PRs to update the crate it seems like the maintenance is largely on us at this point. * Currently the binaryen-sys crate doesn't support fuzzing anything beyond MVP wasm, but we're interested at least in features like bulk memory and reference types. Additionally we'll also be interested in features like module-linking. New features would require either implementation work in binaryen or the binaryen-sys crate to support. * We have 4-5 fuzz-bugs right now related to timeouts simply in generating a module for wasmtime to fuzz. One investigation along these lines in the past revealed a bug in binaryen itself, and in any case these bugs would otherwise need to get investigated, reported, and possibly fixed ourselves in upstream binaryen. Overall I'm not sure at this point if maintaining binaryen fuzzing is worth it with the advent of `wasm-smith` which has similar goals for wasm module generation, but is much more readily maintainable on our end. Additonally in this commit I've added a fuzzer for wasm-smith's `SwarmConfig`-based fuzzer which should expand the coverage of tested modules. Closes #2163 --- .github/workflows/main.yml | 2 +- Cargo.lock | 27 ++----------- crates/fuzzing/Cargo.toml | 2 +- crates/fuzzing/src/generators.rs | 43 --------------------- crates/fuzzing/src/generators/api.rs | 12 +++--- crates/fuzzing/src/oracles.rs | 14 +++---- fuzz/Cargo.toml | 18 +++------ fuzz/fuzz_targets/differential.rs | 5 ++- fuzz/fuzz_targets/instantiate-swarm.rs | 13 +++++++ fuzz/fuzz_targets/instantiate_translated.rs | 9 ----- 10 files changed, 41 insertions(+), 104 deletions(-) create mode 100644 fuzz/fuzz_targets/instantiate-swarm.rs delete mode 100644 fuzz/fuzz_targets/instantiate_translated.rs diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index aab84ac0be..3c3caeea5a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -146,7 +146,7 @@ jobs: - run: cargo install cargo-fuzz --vers "^0.8" - run: cargo fetch working-directory: ./fuzz - - run: cargo fuzz build --dev --features binaryen + - run: cargo fuzz build --dev rebuild_peephole_optimizers: name: Rebuild Peephole Optimizers diff --git a/Cargo.lock b/Cargo.lock index 0ad42fbdb8..77aff5b000 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -115,27 +115,6 @@ version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3441f0f7b02788e948e47f457ca01f1d7e6d92c693bc132c22b087d3141c03ff" -[[package]] -name = "binaryen" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a51ad23b3c7ab468d9daa948201921879ef0052e561c250fd0b326e6f000f2dd" -dependencies = [ - "binaryen-sys", -] - -[[package]] -name = "binaryen-sys" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df5829a7c89f7827e58866704e4dfdf48a635d73c6e5449c1a8a0ba5a319d28a" -dependencies = [ - "cc", - "cmake", - "heck", - "regex", -] - [[package]] name = "bincode" version = "1.3.1" @@ -2294,9 +2273,9 @@ dependencies = [ [[package]] name = "wasm-smith" -version = "0.1.5" +version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ff896bbe4adf62d6a909708c34db3ad94ce2103daa9673f64fe15e60ba70dad" +checksum = "f372c777fcc75bbad237aa0b14e380cfddb3680f42c0584f1f0681542f8559b7" dependencies = [ "arbitrary", "leb128", @@ -2497,10 +2476,10 @@ version = "0.19.0" dependencies = [ "anyhow", "arbitrary", - "binaryen", "env_logger", "log", "rayon", + "wasm-smith", "wasmparser 0.63.0", "wasmprinter", "wasmtime", diff --git a/crates/fuzzing/Cargo.toml b/crates/fuzzing/Cargo.toml index 75e2fceb6d..28dc3177d9 100644 --- a/crates/fuzzing/Cargo.toml +++ b/crates/fuzzing/Cargo.toml @@ -9,7 +9,6 @@ version = "0.19.0" [dependencies] anyhow = "1.0.22" arbitrary = { version = "0.4.1", features = ["derive"] } -binaryen = { version = "0.10.0", optional = true } env_logger = "0.7.1" log = "0.4.8" rayon = "1.2.1" @@ -17,6 +16,7 @@ wasmparser = "0.63.0" wasmprinter = "0.2.10" wasmtime = { path = "../wasmtime" } wasmtime-wast = { path = "../wast" } +wasm-smith = "0.1.9" [dev-dependencies] wat = "1.0.23" diff --git a/crates/fuzzing/src/generators.rs b/crates/fuzzing/src/generators.rs index 6641eddb66..d31eba04cb 100644 --- a/crates/fuzzing/src/generators.rs +++ b/crates/fuzzing/src/generators.rs @@ -8,55 +8,12 @@ //! wrapper over an external tool, such that the wrapper implements the //! `Arbitrary` trait for the wrapped external tool. -#[cfg(feature = "binaryen")] pub mod api; pub mod table_ops; use arbitrary::{Arbitrary, Unstructured}; -/// A Wasm test case generator that is powered by Binaryen's `wasm-opt -ttf`. -#[derive(Clone)] -#[cfg(feature = "binaryen")] -pub struct WasmOptTtf { - /// The raw, encoded Wasm bytes. - pub wasm: Vec, -} - -#[cfg(feature = "binaryen")] -impl std::fmt::Debug for WasmOptTtf { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!( - f, - "WasmOptTtf {{ wasm: wat::parse_str(r###\"\n{}\n\"###).unwrap() }}", - wasmprinter::print_bytes(&self.wasm).expect("valid wasm should always disassemble") - ) - } -} - -#[cfg(feature = "binaryen")] -impl Arbitrary for WasmOptTtf { - fn arbitrary(input: &mut arbitrary::Unstructured) -> arbitrary::Result { - crate::init_fuzzing(); - let seed: Vec = Arbitrary::arbitrary(input)?; - let module = binaryen::tools::translate_to_fuzz_mvp(&seed); - let wasm = module.write(); - Ok(WasmOptTtf { wasm }) - } - - fn arbitrary_take_rest(input: arbitrary::Unstructured) -> arbitrary::Result { - crate::init_fuzzing(); - let seed: Vec = Arbitrary::arbitrary_take_rest(input)?; - let module = binaryen::tools::translate_to_fuzz_mvp(&seed); - let wasm = module.write(); - Ok(WasmOptTtf { wasm }) - } - - fn size_hint(depth: usize) -> (usize, Option) { - as Arbitrary>::size_hint(depth) - } -} - /// A description of configuration options that we should do differential /// testing between. #[derive(Arbitrary, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/crates/fuzzing/src/generators/api.rs b/crates/fuzzing/src/generators/api.rs index b27fcaee59..d85f93170d 100644 --- a/crates/fuzzing/src/generators/api.rs +++ b/crates/fuzzing/src/generators/api.rs @@ -17,6 +17,7 @@ use arbitrary::{Arbitrary, Unstructured}; use std::collections::BTreeMap; use std::mem; +use wasm_smith::Module; use wasmparser::*; #[derive(Arbitrary, Debug)] @@ -31,7 +32,7 @@ struct Swarm { } /// A call to one of Wasmtime's public APIs. -#[derive(Arbitrary, Clone, Debug)] +#[derive(Arbitrary, Debug)] #[allow(missing_docs)] pub enum ApiCall { ConfigNew, @@ -39,7 +40,7 @@ pub enum ApiCall { ConfigInterruptable(bool), EngineNew, StoreNew, - ModuleNew { id: usize, wasm: super::WasmOptTtf }, + ModuleNew { id: usize, wasm: Module }, ModuleDrop { id: usize }, InstanceNew { id: usize, module: usize }, InstanceDrop { id: usize }, @@ -106,8 +107,9 @@ impl Arbitrary for ApiCalls { if swarm.module_new { choices.push(|input, scope| { let id = scope.next_id(); - let wasm = super::WasmOptTtf::arbitrary(input)?; - let predicted_rss = predict_rss(&wasm.wasm).unwrap_or(0); + let mut wasm = Module::arbitrary(input)?; + wasm.ensure_termination(1000); + let predicted_rss = predict_rss(&wasm.to_bytes()).unwrap_or(0); scope.modules.insert(id, predicted_rss); Ok(ModuleNew { id, wasm }) }); @@ -173,7 +175,7 @@ impl Arbitrary for ApiCalls { // We can generate arbitrary `WasmOptTtf` instances, which have // no upper bound on the number of bytes they consume. This sets // the upper bound to `None`. - ::size_hint(depth), + ::size_hint(depth), ) }) } diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index d0e7fc3e88..550ef77d15 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -119,9 +119,8 @@ pub fn compile(wasm: &[u8], strategy: Strategy) { /// exports. Modulo OOM, non-canonical NaNs, and usage of Wasm features that are /// or aren't enabled for different configs, we should get the same results when /// we call the exported functions for all of our different configs. -#[cfg(feature = "binaryen")] pub fn differential_execution( - ttf: &crate::generators::WasmOptTtf, + module: &wasm_smith::Module, configs: &[crate::generators::DifferentialConfig], ) { use std::collections::{HashMap, HashSet}; @@ -144,13 +143,14 @@ pub fn differential_execution( }; let mut export_func_results: HashMap, Trap>> = Default::default(); - log_wasm(&ttf.wasm); + let wasm = module.to_bytes(); + log_wasm(&wasm); for config in &configs { let engine = Engine::new(config); let store = Store::new(&engine); - let module = match Module::new(&engine, &ttf.wasm) { + let module = match Module::new(&engine, &wasm) { Ok(module) => module, // The module might rely on some feature that our config didn't // enable or something like that. @@ -278,7 +278,6 @@ pub fn differential_execution( } /// Invoke the given API calls. -#[cfg(feature = "binaryen")] pub fn make_api_calls(api: crate::generators::api::ApiCalls) { use crate::generators::api::ApiCall; use std::collections::HashMap; @@ -323,8 +322,9 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { ApiCall::ModuleNew { id, wasm } => { log::debug!("creating module: {}", id); - log_wasm(&wasm.wasm); - let module = match Module::new(engine.as_ref().unwrap(), &wasm.wasm) { + let wasm = wasm.to_bytes(); + log_wasm(&wasm); + let module = match Module::new(engine.as_ref().unwrap(), &wasm) { Ok(m) => m, Err(_) => continue, }; diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 62c322e69a..f295655755 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -31,26 +31,17 @@ path = "fuzz_targets/instantiate.rs" test = false doc = false -[[bin]] -name = "instantiate_translated" -path = "fuzz_targets/instantiate_translated.rs" -test = false -doc = false -required-features = ["binaryen"] - [[bin]] name = "api_calls" path = "fuzz_targets/api_calls.rs" test = false doc = false -required-features = ["binaryen"] [[bin]] name = "differential" path = "fuzz_targets/differential.rs" test = false doc = false -required-features = ["binaryen"] [[bin]] name = "spectests" @@ -99,15 +90,18 @@ test = false doc = false required-features = ["peepmatic-fuzzing"] -[features] -binaryen = ["wasmtime-fuzzing/binaryen"] - [[bin]] name = "instantiate-wasm-smith" path = "fuzz_targets/instantiate-wasm-smith.rs" test = false doc = false +[[bin]] +name = "instantiate-swarm" +path = "fuzz_targets/instantiate-swarm.rs" +test = false +doc = false + [[bin]] name = "instantiate-maybe-invalid" path = "fuzz_targets/instantiate-maybe-invalid.rs" diff --git a/fuzz/fuzz_targets/differential.rs b/fuzz/fuzz_targets/differential.rs index 5cf14f0523..9b92d7998f 100644 --- a/fuzz/fuzz_targets/differential.rs +++ b/fuzz/fuzz_targets/differential.rs @@ -6,8 +6,9 @@ use wasmtime_fuzzing::{generators, oracles}; fuzz_target!(|data: ( generators::DifferentialConfig, generators::DifferentialConfig, - generators::WasmOptTtf + wasm_smith::Module, )| { - let (lhs, rhs, wasm) = data; + let (lhs, rhs, mut wasm) = data; + wasm.ensure_termination(1000); oracles::differential_execution(&wasm, &[lhs, rhs]); }); diff --git a/fuzz/fuzz_targets/instantiate-swarm.rs b/fuzz/fuzz_targets/instantiate-swarm.rs new file mode 100644 index 0000000000..a3049ad998 --- /dev/null +++ b/fuzz/fuzz_targets/instantiate-swarm.rs @@ -0,0 +1,13 @@ +#![no_main] + +use libfuzzer_sys::fuzz_target; +use std::time::Duration; +use wasm_smith::{ConfiguredModule, SwarmConfig}; +use wasmtime::Strategy; +use wasmtime_fuzzing::oracles; + +fuzz_target!(|module: ConfiguredModule| { + let mut cfg = wasmtime_fuzzing::fuzz_default_config(Strategy::Auto).unwrap(); + cfg.wasm_multi_memory(true); + oracles::instantiate_with_config(&module.to_bytes(), cfg, Some(Duration::from_secs(20))); +}); diff --git a/fuzz/fuzz_targets/instantiate_translated.rs b/fuzz/fuzz_targets/instantiate_translated.rs deleted file mode 100644 index 4f98c3d2fb..0000000000 --- a/fuzz/fuzz_targets/instantiate_translated.rs +++ /dev/null @@ -1,9 +0,0 @@ -#![no_main] - -use libfuzzer_sys::fuzz_target; -use wasmtime::Strategy; -use wasmtime_fuzzing::{generators, oracles}; - -fuzz_target!(|data: generators::WasmOptTtf| { - oracles::instantiate(&data.wasm, Strategy::Auto); -});