diff --git a/crates/fuzzing/src/generators.rs b/crates/fuzzing/src/generators.rs index e6e4cc76be..6641eddb66 100644 --- a/crates/fuzzing/src/generators.rs +++ b/crates/fuzzing/src/generators.rs @@ -11,6 +11,8 @@ #[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`. diff --git a/crates/fuzzing/src/generators/table_ops.rs b/crates/fuzzing/src/generators/table_ops.rs new file mode 100644 index 0000000000..45aaa5b802 --- /dev/null +++ b/crates/fuzzing/src/generators/table_ops.rs @@ -0,0 +1,148 @@ +//! Generating series of `table.get` and `table.set` operations. + +use arbitrary::Arbitrary; +use std::fmt::Write; +use std::ops::Range; + +/// A description of a Wasm module that makes a series of `externref` table +/// operations. +#[derive(Arbitrary, Debug)] +pub struct TableOps { + num_params: u8, + table_size: u32, + ops: Vec, +} + +const NUM_PARAMS_RANGE: Range = 1..10; +const TABLE_SIZE_RANGE: Range = 1..100; +const MAX_OPS: usize = 1000; + +impl TableOps { + /// Get the number of parameters this module's "run" function takes. + pub fn num_params(&self) -> u8 { + let num_params = std::cmp::max(self.num_params, NUM_PARAMS_RANGE.start); + let num_params = std::cmp::min(num_params, NUM_PARAMS_RANGE.end); + num_params + } + + /// Get the size of the table that this module uses. + pub fn table_size(&self) -> u32 { + let table_size = std::cmp::max(self.table_size, TABLE_SIZE_RANGE.start); + let table_size = std::cmp::min(table_size, TABLE_SIZE_RANGE.end); + table_size + } + + /// Convert this into a WAT string. + /// + /// The module requires a single import: `(import "" "gc" (func))`. This + /// should be a function to trigger GC. + /// + /// The single export of the module is a function "run" that takes + /// `self.num_params()` parameters of type `externref`. + /// + /// The "run" function is guaranteed to terminate (no loops or recursive + /// calls), but is not guaranteed to avoid traps (might access out-of-bounds + /// of the table). + pub fn to_wat_string(&self) -> String { + let mut wat = "(module\n".to_string(); + + // Import the GC function. + wat.push_str(" (import \"\" \"gc\" (func))\n"); + + // Define our table. + wat.push_str(" (table $table "); + write!(&mut wat, "{}", self.table_size()).unwrap(); + wat.push_str(" externref)\n"); + + // Define the "run" function export. + wat.push_str(r#" (func (export "run") (param"#); + for _ in 0..self.num_params() { + wat.push_str(" externref"); + } + wat.push_str(")\n"); + for op in self.ops.iter().take(MAX_OPS) { + wat.push_str(" "); + op.to_wat_string(&mut wat); + wat.push('\n'); + } + wat.push_str(" )\n"); + + wat.push_str(")\n"); + wat + } +} + +#[derive(Arbitrary, Debug)] +pub(crate) enum TableOp { + // `(call 0)` + Gc, + // `(drop (table.get x))` + Get(u32), + // `(table.set x (local.get y))` + SetFromParam(u32, u8), + // `(table.set x (table.get y))` + SetFromGet(u32, u32), +} + +impl TableOp { + fn to_wat_string(&self, wat: &mut String) { + match self { + Self::Gc => { + wat.push_str("(call 0)"); + } + Self::Get(x) => { + wat.push_str("(drop (table.get $table (i32.const "); + write!(wat, "{}", x).unwrap(); + wat.push_str(")))"); + } + Self::SetFromParam(x, y) => { + wat.push_str("(table.set $table (i32.const "); + write!(wat, "{}", x).unwrap(); + wat.push_str(") (local.get "); + write!(wat, "{}", y).unwrap(); + wat.push_str("))"); + } + Self::SetFromGet(x, y) => { + wat.push_str("(table.set $table (i32.const "); + write!(wat, "{}", x).unwrap(); + wat.push_str(") (table.get $table (i32.const "); + write!(wat, "{}", y).unwrap(); + wat.push_str(")))"); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_wat_string() { + let ops = TableOps { + num_params: 2, + table_size: 10, + ops: vec![ + TableOp::Gc, + TableOp::Get(0), + TableOp::SetFromParam(1, 2), + TableOp::SetFromGet(3, 4), + ], + }; + + let expected = r#" +(module + (import "" "gc" (func)) + (table $table 10 externref) + (func (export "run") (param externref externref) + (call 0) + (drop (table.get $table (i32.const 0))) + (table.set $table (i32.const 1) (local.get 2)) + (table.set $table (i32.const 3) (table.get $table (i32.const 4))) + ) +) +"#; + let actual = ops.to_wat_string(); + assert_eq!(actual.trim(), expected.trim()); + } +} diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index 19fcc6b1f2..9a948638d1 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -13,12 +13,15 @@ pub mod dummy; use dummy::dummy_imports; +use std::cell::Cell; +use std::rc::Rc; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use wasmtime::*; use wasmtime_wast::WastContext; +static CNT: AtomicUsize = AtomicUsize::new(0); + fn log_wasm(wasm: &[u8]) { - static CNT: AtomicUsize = AtomicUsize::new(0); if !log::log_enabled!(log::Level::Debug) { return; } @@ -33,6 +36,16 @@ fn log_wasm(wasm: &[u8]) { } } +fn log_wat(wat: &str) { + if !log::log_enabled!(log::Level::Debug) { + return; + } + + let i = CNT.fetch_add(1, SeqCst); + let name = format!("testcase{}.wat", i); + std::fs::write(&name, wat).expect("failed to write wat file"); +} + /// Instantiate the Wasm buffer, and implicitly fail if we have an unexpected /// panic or segfault or anything else that can be detected "passively". /// @@ -400,3 +413,55 @@ pub fn spectest(config: crate::generators::Config, test: crate::generators::Spec .run_buffer(test.file, test.contents.as_bytes()) .unwrap(); } + +/// Execute a series of `table.get` and `table.set` operations. +pub fn table_ops(config: crate::generators::Config, ops: crate::generators::table_ops::TableOps) { + let _ = env_logger::try_init(); + + let num_dropped = Rc::new(Cell::new(0)); + + { + let mut config = config.to_wasmtime(); + config.wasm_reference_types(true); + let engine = Engine::new(&config); + let store = Store::new(&engine); + + let wat = ops.to_wat_string(); + log_wat(&wat); + let module = match Module::new(&engine, &wat) { + Ok(m) => m, + Err(_) => return, + }; + + // To avoid timeouts, limit the number of explicit GCs we perform per + // test case. + const MAX_GCS: usize = 5; + + let num_gcs = Cell::new(0); + let gc = Func::wrap(&store, move |caller: Caller| { + if num_gcs.get() < MAX_GCS { + caller.store().gc(); + num_gcs.set(num_gcs.get() + 1); + } + }); + + let instance = Instance::new(&store, &module, &[gc.into()]).unwrap(); + let run = instance.get_func("run").unwrap(); + + let args: Vec<_> = (0..ops.num_params()) + .map(|_| Val::ExternRef(Some(ExternRef::new(CountDrops(num_dropped.clone()))))) + .collect(); + let _ = run.call(&args); + } + + assert_eq!(num_dropped.get(), ops.num_params()); + return; + + struct CountDrops(Rc>); + + impl Drop for CountDrops { + fn drop(&mut self) { + self.0.set(self.0.get().checked_add(1).unwrap()); + } + } +} diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 48ef51f54c..a28808c563 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -901,18 +901,14 @@ impl StackMapRegistry { // Exact hit. Ok(i) => i, - Err(n) => { - // `Err(0)` means that the associated stack map would have been - // the first element in the array if this pc had an associated - // stack map, but this pc does not have an associated stack - // map. That doesn't make sense since every call and trap inside - // Wasm is a GC safepoint and should have a stack map, and the - // only way to have Wasm frames under this native frame is if we - // are at a call or a trap. - debug_assert!(n != 0); + // `Err(0)` means that the associated stack map would have been the + // first element in the array if this pc had an associated stack + // map, but this pc does not have an associated stack map. This can + // only happen inside a Wasm frame if there are no live refs at this + // pc. + Err(0) => return None, - n - 1 - } + Err(n) => n - 1, }; let stack_map = stack_maps.pc_to_stack_map[index].1.clone(); diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index da57248306..f2ce4f6e6f 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -52,8 +52,8 @@ fn instantiate( config.memory_creator.as_ref().map(|a| a as _), store.interrupts().clone(), host, - &**store.externref_activations_table() as *const VMExternRefActivationsTable as *mut _, - &**store.stack_map_registry() as *const StackMapRegistry as *mut _, + store.externref_activations_table() as *const VMExternRefActivationsTable as *mut _, + store.stack_map_registry() as *const StackMapRegistry as *mut _, )?; // After we've created the `InstanceHandle` we still need to run diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index ddec435af7..e47eff4792 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -813,8 +813,8 @@ pub(crate) struct StoreInner { instances: RefCell>, signal_handler: RefCell>>>, jit_code_ranges: RefCell>, - externref_activations_table: Rc, - stack_map_registry: Rc, + externref_activations_table: VMExternRefActivationsTable, + stack_map_registry: StackMapRegistry, } struct HostInfoKey(VMExternRef); @@ -854,8 +854,8 @@ impl Store { instances: RefCell::new(Vec::new()), signal_handler: RefCell::new(None), jit_code_ranges: RefCell::new(Vec::new()), - externref_activations_table: Rc::new(VMExternRefActivationsTable::new()), - stack_map_registry: Rc::new(StackMapRegistry::default()), + externref_activations_table: VMExternRefActivationsTable::new(), + stack_map_registry: StackMapRegistry::default(), }), } } @@ -1091,11 +1091,11 @@ impl Store { } } - pub(crate) fn externref_activations_table(&self) -> &Rc { + pub(crate) fn externref_activations_table(&self) -> &VMExternRefActivationsTable { &self.inner.externref_activations_table } - pub(crate) fn stack_map_registry(&self) -> &Rc { + pub(crate) fn stack_map_registry(&self) -> &StackMapRegistry { &self.inner.stack_map_registry } @@ -1106,8 +1106,8 @@ impl Store { // used with this store in `self.inner.stack_map_registry`. unsafe { wasmtime_runtime::gc( - &*self.inner.stack_map_registry, - &*self.inner.externref_activations_table, + &self.inner.stack_map_registry, + &self.inner.externref_activations_table, ); } } diff --git a/crates/wasmtime/src/trampoline/create_handle.rs b/crates/wasmtime/src/trampoline/create_handle.rs index db4d477905..e97773318d 100644 --- a/crates/wasmtime/src/trampoline/create_handle.rs +++ b/crates/wasmtime/src/trampoline/create_handle.rs @@ -47,8 +47,8 @@ pub(crate) fn create_handle( signatures.into_boxed_slice(), state, store.interrupts().clone(), - &**store.externref_activations_table() as *const VMExternRefActivationsTable as *mut _, - &**store.stack_map_registry() as *const StackMapRegistry as *mut _, + store.externref_activations_table() as *const VMExternRefActivationsTable as *mut _, + store.stack_map_registry() as *const StackMapRegistry as *mut _, )?; Ok(store.add_instance(handle)) } diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 5d99b616ba..0da950145c 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -57,6 +57,12 @@ path = "fuzz_targets/spectests.rs" test = false doc = false +[[bin]] +name = "table_ops" +path = "fuzz_targets/table_ops.rs" +test = false +doc = false + [[bin]] name = "peepmatic_simple_automata" path = "fuzz_targets/peepmatic_simple_automata.rs" diff --git a/fuzz/fuzz_targets/table_ops.rs b/fuzz/fuzz_targets/table_ops.rs new file mode 100755 index 0000000000..b17637157e --- /dev/null +++ b/fuzz/fuzz_targets/table_ops.rs @@ -0,0 +1,9 @@ +#![no_main] + +use libfuzzer_sys::fuzz_target; +use wasmtime_fuzzing::generators::{table_ops::TableOps, Config}; + +fuzz_target!(|pair: (Config, TableOps)| { + let (config, ops) = pair; + wasmtime_fuzzing::oracles::table_ops(config, ops); +}); diff --git a/tests/all/gc.rs b/tests/all/gc.rs index 09eacd93e4..b2a47d3104 100644 --- a/tests/all/gc.rs +++ b/tests/all/gc.rs @@ -47,12 +47,9 @@ fn smoke_test_gc() -> anyhow::Result<()> { "#, )?; - let do_gc = Func::wrap(&store, { - let store = store.clone(); - move || { - // Do a GC with `externref`s on the stack in Wasm frames. - store.gc(); - } + let do_gc = Func::wrap(&store, |caller: Caller| { + // Do a GC with `externref`s on the stack in Wasm frames. + caller.store().gc(); }); let instance = Instance::new(&store, &module, &[do_gc.into()])?; let func = instance.get_func("func").unwrap();