From 98ef18a22a5646990b00a469c97876ee924410ff Mon Sep 17 00:00:00 2001 From: Conrad Watt Date: Tue, 1 Mar 2022 18:01:46 +0000 Subject: [PATCH] Fuzzing against verified fork of spec interpreter (#3843) * Revert "Remove spec interpreter fuzz target temporarily (#3399)" This reverts commit 25d3fa4d7b944b4341928a401c57feaaa0ce3c35. * add support for differential fuzzing against verified OCaml interpreter * formatting * comments * fix missing dep case * fix build error * fix unit tests? * restore previous differential_v8 max_table config * attempt: add OCaml deps * fix interpeter github repo * fix spec repo url * fix zarith package * fix unit test --- .github/workflows/main.yml | 2 +- crates/fuzzing/Cargo.toml | 5 +- crates/fuzzing/src/generators.rs | 3 ++ crates/fuzzing/src/oracles.rs | 16 +++--- crates/fuzzing/wasm-spec-interpreter/build.rs | 4 +- .../wasm-spec-interpreter/ocaml/Makefile | 9 +++- .../wasm-spec-interpreter/ocaml/README.md | 5 +- .../wasm-spec-interpreter/ocaml/interpret.ml | 54 +++++++++++-------- .../fuzzing/wasm-spec-interpreter/src/lib.rs | 5 +- .../wasm-spec-interpreter/src/with_library.rs | 34 +++++++----- .../src/without_library.rs | 4 +- .../wasm-spec-interpreter/tests/oob.wat | 3 +- fuzz/Cargo.toml | 6 +++ fuzz/fuzz_targets/differential_spec.rs | 47 ++++++++++++++++ fuzz/fuzz_targets/differential_v8.rs | 4 ++ 15 files changed, 142 insertions(+), 59 deletions(-) create mode 100644 fuzz/fuzz_targets/differential_spec.rs diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ad75d9006b..7fb800c420 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -181,7 +181,7 @@ jobs: toolchain: nightly-2021-12-15 - run: cargo install cargo-fuzz --vers "^0.11" # Install OCaml packages necessary for 'differential_spec' fuzz target. - - run: sudo apt install -y ocaml-nox ocamlbuild + - run: sudo apt install -y ocaml-nox ocamlbuild ocaml-findlib libzarith-ocaml-dev - run: cargo fetch working-directory: ./fuzz - run: cargo fuzz build --dev diff --git a/crates/fuzzing/Cargo.toml b/crates/fuzzing/Cargo.toml index 67663c70f8..b7b264bd4f 100644 --- a/crates/fuzzing/Cargo.toml +++ b/crates/fuzzing/Cargo.toml @@ -35,8 +35,7 @@ v8 = "0.33" [dev-dependencies] wat = "1.0.37" -# FIXME(#3251) should re-enable once spec interpreter won't time out # We only build the library containing the OCaml spec interpreter if the OCaml # toolchain is available--which is assumed here to be the case when fuzzing. -# [target.'cfg(fuzzing)'.dependencies] -# wasm-spec-interpreter = { path = "./wasm-spec-interpreter", features = ["build-libinterpret"] } +[target.'cfg(fuzzing)'.dependencies] +wasm-spec-interpreter = { path = "./wasm-spec-interpreter", features = ["build-libinterpret"] } diff --git a/crates/fuzzing/src/generators.rs b/crates/fuzzing/src/generators.rs index 4d641a6b21..b84f2d1bb9 100644 --- a/crates/fuzzing/src/generators.rs +++ b/crates/fuzzing/src/generators.rs @@ -272,6 +272,9 @@ impl Config { config.max_memory_pages = 1; config.memory_max_size_required = true; + // While reference types are disabled below, only allow one table + config.max_tables = 1; + // Don't allow any imports config.max_imports = 0; diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index edb5d27e81..85c6b3c60e 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -793,9 +793,11 @@ pub fn differential_spec_execution(wasm: &[u8], config: &generators::Config) -> // interfere, observable as an uncaught `SIGSEGV`--not even caught by // libFuzzer. By running Wasmtime second, its signal handlers are registered // most recently and they catch failures appropriately. - let spec_vals = wasm_spec_interpreter::interpret(wasm, vec![]); + // + // For now, execute with dummy (zeroed) function arguments. + let spec_vals = wasm_spec_interpreter::interpret(wasm, None); debug!("spec interpreter returned: {:?}", &spec_vals); - let wasmtime_vals = run_in_wasmtime(wasm, config, &[]); + let wasmtime_vals = run_in_wasmtime(wasm, config); debug!("Wasmtime returned: {:?}", wasmtime_vals); // Match a spec interpreter value against a Wasmtime value. Eventually this @@ -871,11 +873,7 @@ fn differential_store( /// Helper for instantiating and running a Wasm module in Wasmtime and returning /// its `Val` results. -fn run_in_wasmtime( - wasm: &[u8], - config: &generators::Config, - params: &[Val], -) -> anyhow::Result>> { +fn run_in_wasmtime(wasm: &[u8], config: &generators::Config) -> anyhow::Result>> { // Instantiate wasmtime module and instance. let (wasmtime_module, mut wasmtime_store) = differential_store(wasm, config); let wasmtime_module = match wasmtime_module { @@ -893,10 +891,12 @@ fn run_in_wasmtime( .get_func(&mut wasmtime_store, &func_name[..]) .expect("function export is present"); + let dummy_params = dummy::dummy_values(ty.params()); + // Execute the function and return the values. let mut results = vec![Val::I32(0); ty.results().len()]; wasmtime_main - .call(&mut wasmtime_store, params, &mut results) + .call(&mut wasmtime_store, &dummy_params, &mut results) .map(|()| Some(results)) } diff --git a/crates/fuzzing/wasm-spec-interpreter/build.rs b/crates/fuzzing/wasm-spec-interpreter/build.rs index 9bf0c0eb76..38bef61204 100644 --- a/crates/fuzzing/wasm-spec-interpreter/build.rs +++ b/crates/fuzzing/wasm-spec-interpreter/build.rs @@ -10,8 +10,8 @@ use std::{env, path::PathBuf, process::Command}; const LIB_NAME: &'static str = "interpret"; const OCAML_DIR: &'static str = "ocaml"; const SPEC_DIR: &'static str = "ocaml/spec"; -const SPEC_REPOSITORY: &'static str = "https://github.com/bytecodealliance/wasm-spec-mirror"; -const SPEC_REPOSITORY_BRANCH: &'static str = "fuzzing"; +const SPEC_REPOSITORY: &'static str = "https://github.com/conrad-watt/spec"; +const SPEC_REPOSITORY_BRANCH: &'static str = "wasmtime_fuzzing"; fn main() { if cfg!(feature = "build-libinterpret") { diff --git a/crates/fuzzing/wasm-spec-interpreter/ocaml/Makefile b/crates/fuzzing/wasm-spec-interpreter/ocaml/Makefile index de7b79d934..4f4ac7bbdd 100644 --- a/crates/fuzzing/wasm-spec-interpreter/ocaml/Makefile +++ b/crates/fuzzing/wasm-spec-interpreter/ocaml/Makefile @@ -11,14 +11,19 @@ SPEC_DIR := spec/interpreter SPEC_BUILD_DIR := $(SPEC_DIR)/_build SPEC_LIB := $(SPEC_BUILD_DIR)/wasm.cmxa +# A space-separated list of paths that the linker will use to search for libgmp. +# Override with `make LIBGMP_PATHS=...`. +LIBGMP_PATHS := /usr/lib /usr/lib/x86_64-linux-gnu + +PKGS = zarith # Build and package the static library, `libinterpret.a`. $(BUILD_DIR)/libinterpret.a: $(BUILD_DIR)/interpret.lib.o ar qs $@ $^ $(BUILD_DIR)/interpret.lib.o: $(SPEC_LIB) $(BUILD_DIR)/interpret.cmx - ocamlopt $(OCAML_FLAGS) -I $(SPEC_BUILD_DIR) -o $@ -output-complete-obj $^ + ocamlfind ocamlopt $(OCAML_FLAGS) -I $(SPEC_BUILD_DIR) -o $@ -output-complete-obj $^ -linkpkg $(PKGS:%=-package %) -cclib "$(LIBGMP_PATHS:%=-L%)" $(BUILD_DIR)/interpret.cmx: interpret.ml $(SPEC_BUILD_DIR) $(BUILD_DIR) - ocamlopt $(OCAML_FLAGS) -I $(SPEC_BUILD_DIR) -o $@ -c -impl $< + ocamlfind ocamlopt $(OCAML_FLAGS) -I $(SPEC_BUILD_DIR) -o $@ -c -impl $< -linkpkg $(PKGS:%=-package %) $(BUILD_DIR): mkdir -p $@ diff --git a/crates/fuzzing/wasm-spec-interpreter/ocaml/README.md b/crates/fuzzing/wasm-spec-interpreter/ocaml/README.md index a7f4d218dd..21cfb7c373 100644 --- a/crates/fuzzing/wasm-spec-interpreter/ocaml/README.md +++ b/crates/fuzzing/wasm-spec-interpreter/ocaml/README.md @@ -1,7 +1,10 @@ This directory contains the necessary parts for building a library with FFI access to the Wasm spec interpreter. Its major parts: - `spec`: the Wasm spec code as a Git submodule (you may need to retrieve it: - `git clone https://github.com/bytecodealliance/wasm-spec-mirror). + `git clone https://github.com/conrad-watt/spec/tree/wasmtime_fuzzing). - `interpret.ml`: a shim layer for calling the Wasm spec code and exposing it for FFI access - `Makefile`: the steps for gluing these pieces together into a static library + +Note: the makefile must be configured with the path to libgmp. See LIBGMP_PATHS +in the makefile. diff --git a/crates/fuzzing/wasm-spec-interpreter/ocaml/interpret.ml b/crates/fuzzing/wasm-spec-interpreter/ocaml/interpret.ml index ead9aad28a..cda7ea2032 100644 --- a/crates/fuzzing/wasm-spec-interpreter/ocaml/interpret.ml +++ b/crates/fuzzing/wasm-spec-interpreter/ocaml/interpret.ml @@ -7,6 +7,7 @@ understand this better, see: (* Here we access the WebAssembly specification interpreter; this must be linked in. *) open Wasm +open Wasm.WasmRef_Isa_m.WasmRef_Isa (** Enumerate the types of values we pass across the FFI boundary. This must match `Value` in `src/lib.rs` *) @@ -17,18 +18,18 @@ type ffi_value = | F64 of int64 (** Helper for converting the FFI values to their spec interpreter type. *) -let convert_to_wasm (v: ffi_value) : Values.value = match v with -| I32 n -> Values.Num (I32 n) -| I64 n -> Values.Num (I64 n) -| F32 n -> Values.Num (F32 (F32.of_bits n)) -| F64 n -> Values.Num (F64 (F64.of_bits n)) +let convert_to_wasm (v: ffi_value) : v = match v with +| I32 n -> ConstInt32 (I32_impl_abs n) +| I64 n -> ConstInt64 (I64_impl_abs n) +| F32 n -> ConstFloat32 (F32.of_bits n) +| F64 n -> ConstFloat64 (F64.of_bits n) (** Helper for converting the spec interpreter values to their FFI type. *) -let convert_from_wasm (v: Values.value) : ffi_value = match v with -| Values.Num (I32 n) -> I32 n -| Values.Num (I64 n) -> I64 n -| Values.Num (F32 n) -> F32 (F32.to_bits n) -| Values.Num (F64 n) -> F64 (F64.to_bits n) +let convert_from_wasm (v: v) : ffi_value = match v with +| (ConstInt32 (I32_impl_abs n)) -> I32 n +| (ConstInt64 (I64_impl_abs n)) -> I64 n +| (ConstFloat32 n) -> F32 (F32.to_bits n) +| (ConstFloat64 n) -> F64 (F64.to_bits n) | _ -> failwith "Unknown type" (** Parse the given WebAssembly module binary into an Ast.module_. At some point in the future this @@ -36,30 +37,37 @@ should also be able to parse the textual form (TODO). *) let parse bytes = (* Optionally, use Bytes.unsafe_to_string here to avoid the copy *) let bytes_as_str = Bytes.to_string bytes in - Decode.decode "default" bytes_as_str + (Decode.decode "default" bytes_as_str) (** Return true if an export is a function. *) let match_exported_func export = match export with -| (_, Instance.ExternFunc(func)) -> true +| Module_export_ext(_,Ext_func n,_) -> true | _ -> false (** Extract a function from its export or fail. *) let extract_exported_func export = match export with -| (_, Instance.ExternFunc(func)) -> func +| Module_export_ext(_,Ext_func n,_) -> n | _ -> failwith "" -(** Interpret the first exported function with the given parameters and return the result. *) -let interpret_exn module_bytes params = - let params' = List.map convert_to_wasm params in +(** Interpret the first exported function and return the result. Use provided +parameters if they exist, otherwise use default (zeroed) values. *) +let interpret_exn module_bytes opt_params = + let opt_params_ = Option.map (List.map convert_to_wasm) opt_params in let module_ = parse module_bytes in - let instance = Eval.init module_ [] in - let func = extract_exported_func (List.find match_exported_func instance.exports) in - let returns = Eval.invoke func params' in - let returns' = List.map convert_from_wasm returns in - returns' (* TODO eventually we should hash the memory state and return the hash *) + let m_isa = Ast_convert.convert_module (module_.it) in + let fuel = Z.of_string "4611686018427387904" in + let max_call_depth = Z.of_string "300" in + (match run_fuzz (nat_of_integer fuel) (nat_of_integer max_call_depth) (make_empty_store_m ()) m_isa [] opt_params_ () with + | (s', RValue vs_isa') -> List.map convert_from_wasm (List.rev vs_isa') + | (s', RTrap str) -> raise (Eval.Trap (Source.no_region, "(Isabelle) trap: " ^ str)) + | (s', (RCrash (Error_exhaustion str))) -> raise (Eval.Exhaustion (Source.no_region, "(Isabelle) call stack exhausted")) + | (s', (RCrash (Error_invalid str))) -> raise (Eval.Crash (Source.no_region, "(Isabelle) error: " ^ str)) + | (s', (RCrash (Error_invariant str))) -> raise (Eval.Crash (Source.no_region, "(Isabelle) error: " ^ str)) + (* TODO eventually we should hash the memory state and return the hash *) + ) -let interpret module_bytes params = - try Ok(interpret_exn module_bytes params) with +let interpret module_bytes opt_params = + try Ok(interpret_exn module_bytes opt_params) with | _ as e -> Error(Printexc.to_string e) let () = diff --git a/crates/fuzzing/wasm-spec-interpreter/src/lib.rs b/crates/fuzzing/wasm-spec-interpreter/src/lib.rs index 20440c500e..a2f46a3a9a 100644 --- a/crates/fuzzing/wasm-spec-interpreter/src/lib.rs +++ b/crates/fuzzing/wasm-spec-interpreter/src/lib.rs @@ -28,7 +28,6 @@ mod without_library; #[cfg(not(feature = "has-libinterpret"))] pub use without_library::*; -// FIXME(#3251) should re-enable once spec interpreter won't time out // If the user is fuzzing`, we expect the OCaml library to have been built. -// #[cfg(all(fuzzing, not(feature = "has-libinterpret")))] -// compile_error!("The OCaml library was not built."); +#[cfg(all(fuzzing, not(feature = "has-libinterpret")))] +compile_error!("The OCaml library was not built."); diff --git a/crates/fuzzing/wasm-spec-interpreter/src/with_library.rs b/crates/fuzzing/wasm-spec-interpreter/src/with_library.rs index c8d7fec42b..9dadce4866 100644 --- a/crates/fuzzing/wasm-spec-interpreter/src/with_library.rs +++ b/crates/fuzzing/wasm-spec-interpreter/src/with_library.rs @@ -3,7 +3,7 @@ //! # use wasm_spec_interpreter::{Value, interpret}; //! let module = wat::parse_file("tests/add.wat").unwrap(); //! let parameters = vec![Value::I32(42), Value::I32(1)]; -//! let results = interpret(&module, parameters).unwrap(); +//! let results = interpret(&module, Some(parameters)).unwrap(); //! assert_eq!(results, &[Value::I32(43)]); //! ``` use crate::Value; @@ -16,8 +16,9 @@ lazy_static! { } /// Interpret the first function in the passed WebAssembly module (in Wasm form, -/// currently, not WAT) with the given parameters. -pub fn interpret(module: &[u8], parameters: Vec) -> Result, String> { +/// currently, not WAT), optionally with the given parameters. If no parameters +/// are provided, the function is invoked with zeroed parameters. +pub fn interpret(module: &[u8], opt_parameters: Option>) -> Result, String> { // The OCaml runtime is not re-entrant // (https://ocaml.org/manual/intfc.html#ss:parallel-execution-long-running-c-code). // We need to make sure that only one Rust thread is executing at a time @@ -35,8 +36,9 @@ pub fn interpret(module: &[u8], parameters: Vec) -> Result, St let ocaml_runtime = unsafe { OCamlRuntime::recover_handle() }; // Parse and execute, returning results converted to Rust. let module = module.to_boxroot(ocaml_runtime); - let parameters = parameters.to_boxroot(ocaml_runtime); - let results = ocaml_bindings::interpret(ocaml_runtime, &module, ¶meters); + + let opt_parameters = opt_parameters.to_boxroot(ocaml_runtime); + let results = ocaml_bindings::interpret(ocaml_runtime, &module, &opt_parameters); results.to_rust(ocaml_runtime) } @@ -66,7 +68,7 @@ mod ocaml_bindings { // In Rust, this function becomes: // `pub fn interpret(_: &mut OCamlRuntime, ...: OCamlRef<...>) -> BoxRoot<...>;` ocaml! { - pub fn interpret(module: OCamlBytes, params: OCamlList) -> Result, String>; + pub fn interpret(module: OCamlBytes, params: Option>) -> Result, String>; } } @@ -77,22 +79,28 @@ mod tests { #[test] fn multiple() { let module = wat::parse_file("tests/add.wat").unwrap(); - let parameters = vec![Value::I32(42), Value::I32(1)]; - let results1 = interpret(&module, parameters.clone()).unwrap(); - let results2 = interpret(&module, parameters.clone()).unwrap(); + + let parameters1 = Some(vec![Value::I32(42), Value::I32(1)]); + let results1 = interpret(&module, parameters1.clone()).unwrap(); + + let parameters2 = Some(vec![Value::I32(1), Value::I32(42)]); + let results2 = interpret(&module, parameters2.clone()).unwrap(); + assert_eq!(results1, results2); - let results3 = interpret(&module, parameters).unwrap(); + + let parameters3 = Some(vec![Value::I32(20), Value::I32(23)]); + let results3 = interpret(&module, parameters3.clone()).unwrap(); + assert_eq!(results2, results3); } #[test] fn oob() { let module = wat::parse_file("tests/oob.wat").unwrap(); - let parameters = vec![]; - let results = interpret(&module, parameters); + let results = interpret(&module, None); assert_eq!( results, - Err("Error(_, \"out of bounds memory access\")".to_string()) + Err("Error(_, \"(Isabelle) trap: load\")".to_string()) ); } } diff --git a/crates/fuzzing/wasm-spec-interpreter/src/without_library.rs b/crates/fuzzing/wasm-spec-interpreter/src/without_library.rs index 899a592196..e932dc1c51 100644 --- a/crates/fuzzing/wasm-spec-interpreter/src/without_library.rs +++ b/crates/fuzzing/wasm-spec-interpreter/src/without_library.rs @@ -3,13 +3,13 @@ //! //! ```should_panic //! # use wasm_spec_interpreter::interpret; -//! let _ = interpret(&[], vec![]); +//! let _ = interpret(&[], Some(vec![])); //! ``` use crate::Value; #[allow(dead_code)] -pub fn interpret(_module: &[u8], _parameters: Vec) -> Result, String> { +pub fn interpret(_module: &[u8], _parameters: Option>) -> Result, String> { panic!( "wasm-spec-interpreter was built without its Rust-to-OCaml shim \ library; re-compile with the dependencies listed in its README.md." diff --git a/crates/fuzzing/wasm-spec-interpreter/tests/oob.wat b/crates/fuzzing/wasm-spec-interpreter/tests/oob.wat index 8cdbc6e978..e0fff3815b 100644 --- a/crates/fuzzing/wasm-spec-interpreter/tests/oob.wat +++ b/crates/fuzzing/wasm-spec-interpreter/tests/oob.wat @@ -2,4 +2,5 @@ (memory (;0;) 0 0) (func (export "oob") i32.const 42 - f32.load align=1)) + f32.load align=1 + return)) diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 2f3299a10a..73c8cf1e37 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -56,6 +56,12 @@ path = "fuzz_targets/differential.rs" test = false doc = false +[[bin]] +name = "differential_spec" +path = "fuzz_targets/differential_spec.rs" +test = false +doc = false + [[bin]] name = "differential_wasmi" path = "fuzz_targets/differential_wasmi.rs" diff --git a/fuzz/fuzz_targets/differential_spec.rs b/fuzz/fuzz_targets/differential_spec.rs new file mode 100644 index 0000000000..6ed13ff16c --- /dev/null +++ b/fuzz/fuzz_targets/differential_spec.rs @@ -0,0 +1,47 @@ +#![no_main] + +use libfuzzer_sys::arbitrary::{Result, Unstructured}; +use libfuzzer_sys::fuzz_target; +use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; +use wasmtime_fuzzing::{generators, oracles}; + +// Keep track of how many WebAssembly modules we actually executed (i.e. ran to +// completion) versus how many were tried. +static TRIED: AtomicUsize = AtomicUsize::new(0); +static EXECUTED: AtomicUsize = AtomicUsize::new(0); + +fuzz_target!(|data: &[u8]| { + // errors in `run` have to do with not enough input in `data`, which we + // ignore here since it doesn't affect how we'd like to fuzz. + drop(run(data)); +}); + +fn run(data: &[u8]) -> Result<()> { + let mut u = Unstructured::new(data); + let mut config: generators::Config = u.arbitrary()?; + config.set_differential_config(); + + // Enable features that the spec interpreter has implemented + config.module_config.config.multi_value_enabled = false; + + // TODO: this is a best-effort attempt to avoid errors caused by the + // generated module exporting no functions. + config.module_config.config.min_exports = 5; + config.module_config.config.max_exports = 5; + + let module = config.generate(&mut u, Some(1000))?; + let tried = TRIED.fetch_add(1, SeqCst); + let executed = match oracles::differential_spec_execution(&module.to_bytes(), &config) { + Some(_) => EXECUTED.fetch_add(1, SeqCst), + None => EXECUTED.load(SeqCst), + }; + if tried > 0 && tried % 1000 == 0 { + println!( + "=== Execution rate ({} executed modules / {} tried modules): {}% ===", + executed, + tried, + executed as f64 / tried as f64 * 100f64 + ) + } + Ok(()) +} diff --git a/fuzz/fuzz_targets/differential_v8.rs b/fuzz/fuzz_targets/differential_v8.rs index fa4c8b6df6..59fad11868 100644 --- a/fuzz/fuzz_targets/differential_v8.rs +++ b/fuzz/fuzz_targets/differential_v8.rs @@ -20,6 +20,10 @@ fn run(data: &[u8]) -> Result<()> { config.module_config.config.bulk_memory_enabled = true; config.module_config.config.reference_types_enabled = true; + // Allow multiple tables, as set_differential_config() assumes reference + // types are disabled and therefore sets max_tables to 1 + config.module_config.config.max_tables = 4; + let module = config.generate(&mut u, Some(1000))?; oracles::differential_v8_execution(&module.to_bytes(), &config); Ok(())