diff --git a/crates/fuzzing/src/generators/table_ops.rs b/crates/fuzzing/src/generators/table_ops.rs index 0bf3e9ce5c3b..f8104d6f1a2c 100644 --- a/crates/fuzzing/src/generators/table_ops.rs +++ b/crates/fuzzing/src/generators/table_ops.rs @@ -3,8 +3,8 @@ use arbitrary::Arbitrary; use std::ops::Range; use wasm_encoder::{ - CodeSection, EntityType, Export, ExportSection, Function, FunctionSection, ImportSection, - Instruction, Module, TableSection, TableType, TypeSection, ValType, + CodeSection, EntityType, Export, ExportSection, Function, FunctionSection, GlobalSection, + ImportSection, Instruction, Module, TableSection, TableType, TypeSection, ValType, }; /// A description of a Wasm module that makes a series of `externref` table @@ -12,11 +12,13 @@ use wasm_encoder::{ #[derive(Arbitrary, Debug)] pub struct TableOps { num_params: u8, + num_globals: u8, table_size: u32, ops: Vec, } const NUM_PARAMS_RANGE: Range = 1..10; +const NUM_GLOBALS_RANGE: Range = 1..10; const TABLE_SIZE_RANGE: Range = 1..100; const MAX_OPS: usize = 100; @@ -28,6 +30,13 @@ impl TableOps { num_params } + /// Get the number of globals this module has. + pub fn num_globals(&self) -> u8 { + let num_globals = std::cmp::max(self.num_globals, NUM_GLOBALS_RANGE.start); + let num_globals = std::cmp::min(num_globals, NUM_GLOBALS_RANGE.end); + num_globals + } + /// 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); @@ -98,6 +107,18 @@ impl TableOps { maximum: None, }); + // Define our globals. + let mut globals = GlobalSection::new(); + for _ in 0..self.num_globals() { + globals.global( + wasm_encoder::GlobalType { + val_type: wasm_encoder::ValType::ExternRef, + mutable: true, + }, + Instruction::RefNull(wasm_encoder::ValType::ExternRef), + ); + } + // Define the "run" function export. let mut functions = FunctionSection::new(); functions.function(1); @@ -111,7 +132,12 @@ impl TableOps { func.instruction(Instruction::Loop(wasm_encoder::BlockType::Empty)); for op in self.ops.iter().take(MAX_OPS) { - op.insert(&mut func, self.num_params() as u32, self.table_size()); + op.insert( + &mut func, + self.num_params() as u32, + self.table_size(), + self.num_globals() as u32, + ); } func.instruction(Instruction::Br(0)); func.instruction(Instruction::End); @@ -125,6 +151,7 @@ impl TableOps { .section(&imports) .section(&functions) .section(&tables) + .section(&globals) .section(&exports) .section(&code); @@ -132,32 +159,56 @@ impl TableOps { } } -#[derive(Arbitrary, Debug)] +#[derive(Arbitrary, Copy, Clone, Debug)] pub(crate) enum TableOp { // `call $gc; drop; drop; drop;` Gc, + // `(drop (table.get x))` Get(i32), + + // `(drop (global.get i))` + GetGlobal(u32), + // `(table.set x (local.get y))` SetFromParam(i32, u32), + // `(table.set x (table.get y))` SetFromGet(i32, i32), + // `call $make_refs; table.set x; table.set y; table.set z` SetFromMake(i32, i32, i32), + + // `(global.set x (local.get y))` + SetGlobalFromParam(u32, u32), + + // `(global.set x (table.get y))` + SetGlobalFromGet(u32, i32), + + // `call $make_refs; global.set x; global.set y; global.set z` + SetGlobalFromMake(u32, u32, u32), + // `call $make_refs; drop; drop; drop;` Make, + // `local.get x; local.get y; local.get z; call $take_refs` TakeFromParams(u32, u32, u32), + // `table.get x; table.get y; table.get z; call $take_refs` TakeFromGet(i32, i32, i32), + + // `global.get x; global.get y; global.get z; call $take_refs` + TakeFromGlobalGet(u32, u32, u32), + // `call $make_refs; call $take_refs` TakeFromMake, + // `call $gc; call $take_refs` TakeFromGc, } impl TableOp { - fn insert(&self, func: &mut Function, num_params: u32, table_size: u32) { + fn insert(self, func: &mut Function, num_params: u32, table_size: u32, num_globals: u32) { assert!(num_params > 0); assert!(table_size > 0); @@ -165,49 +216,53 @@ impl TableOp { // but still rare. let table_mod = table_size as i32 + 1; + let gc_func_idx = 0; + let take_refs_func_idx = 1; + let make_refs_func_idx = 2; + match self { Self::Gc => { - func.instruction(Instruction::Call(0)); + func.instruction(Instruction::Call(gc_func_idx)); func.instruction(Instruction::Drop); func.instruction(Instruction::Drop); func.instruction(Instruction::Drop); } Self::Get(x) => { - func.instruction(Instruction::I32Const(*x % table_mod)); + func.instruction(Instruction::I32Const(x % table_mod)); func.instruction(Instruction::TableGet { table: 0 }); func.instruction(Instruction::Drop); } Self::SetFromParam(x, y) => { - func.instruction(Instruction::I32Const(*x % table_mod)); - func.instruction(Instruction::LocalGet(*y % num_params)); + func.instruction(Instruction::I32Const(x % table_mod)); + func.instruction(Instruction::LocalGet(y % num_params)); func.instruction(Instruction::TableSet { table: 0 }); } Self::SetFromGet(x, y) => { - func.instruction(Instruction::I32Const(*x % table_mod)); - func.instruction(Instruction::I32Const(*y % table_mod)); + func.instruction(Instruction::I32Const(x % table_mod)); + func.instruction(Instruction::I32Const(y % table_mod)); func.instruction(Instruction::TableGet { table: 0 }); func.instruction(Instruction::TableSet { table: 0 }); } Self::SetFromMake(x, y, z) => { - func.instruction(Instruction::Call(2)); + func.instruction(Instruction::Call(make_refs_func_idx)); func.instruction(Instruction::LocalSet(num_params)); - func.instruction(Instruction::I32Const(*x % table_mod)); + func.instruction(Instruction::I32Const(x % table_mod)); func.instruction(Instruction::LocalGet(num_params)); func.instruction(Instruction::TableSet { table: 0 }); func.instruction(Instruction::LocalSet(num_params)); - func.instruction(Instruction::I32Const(*y % table_mod)); + func.instruction(Instruction::I32Const(y % table_mod)); func.instruction(Instruction::LocalGet(num_params)); func.instruction(Instruction::TableSet { table: 0 }); func.instruction(Instruction::LocalSet(num_params)); - func.instruction(Instruction::I32Const(*z % table_mod)); + func.instruction(Instruction::I32Const(z % table_mod)); func.instruction(Instruction::LocalGet(num_params)); func.instruction(Instruction::TableSet { table: 0 }); } TableOp::Make => { - func.instruction(Instruction::Call(2)); + func.instruction(Instruction::Call(make_refs_func_idx)); func.instruction(Instruction::Drop); func.instruction(Instruction::Drop); func.instruction(Instruction::Drop); @@ -216,27 +271,52 @@ impl TableOp { func.instruction(Instruction::LocalGet(x % num_params)); func.instruction(Instruction::LocalGet(y % num_params)); func.instruction(Instruction::LocalGet(z % num_params)); - func.instruction(Instruction::Call(1)); + func.instruction(Instruction::Call(take_refs_func_idx)); } TableOp::TakeFromGet(x, y, z) => { - func.instruction(Instruction::I32Const(*x % table_mod)); + func.instruction(Instruction::I32Const(x % table_mod)); func.instruction(Instruction::TableGet { table: 0 }); - func.instruction(Instruction::I32Const(*y % table_mod)); + func.instruction(Instruction::I32Const(y % table_mod)); func.instruction(Instruction::TableGet { table: 0 }); - func.instruction(Instruction::I32Const(*z % table_mod)); + func.instruction(Instruction::I32Const(z % table_mod)); func.instruction(Instruction::TableGet { table: 0 }); - func.instruction(Instruction::Call(1)); + func.instruction(Instruction::Call(take_refs_func_idx)); } TableOp::TakeFromMake => { - func.instruction(Instruction::Call(2)); - func.instruction(Instruction::Call(1)); + func.instruction(Instruction::Call(make_refs_func_idx)); + func.instruction(Instruction::Call(take_refs_func_idx)); } Self::TakeFromGc => { - func.instruction(Instruction::Call(0)); - func.instruction(Instruction::Call(1)); + func.instruction(Instruction::Call(gc_func_idx)); + func.instruction(Instruction::Call(take_refs_func_idx)); + } + TableOp::GetGlobal(x) => { + func.instruction(Instruction::GlobalGet(x % num_globals)); + func.instruction(Instruction::Drop); + } + TableOp::SetGlobalFromParam(global, param) => { + func.instruction(Instruction::LocalGet(param % num_params)); + func.instruction(Instruction::GlobalSet(global % num_globals)); + } + TableOp::SetGlobalFromGet(global, x) => { + func.instruction(Instruction::I32Const(x)); + func.instruction(Instruction::TableGet { table: 0 }); + func.instruction(Instruction::GlobalSet(global % num_globals)); + } + TableOp::SetGlobalFromMake(x, y, z) => { + func.instruction(Instruction::Call(make_refs_func_idx)); + func.instruction(Instruction::GlobalSet(x % num_globals)); + func.instruction(Instruction::GlobalSet(y % num_globals)); + func.instruction(Instruction::GlobalSet(z % num_globals)); + } + TableOp::TakeFromGlobalGet(x, y, z) => { + func.instruction(Instruction::GlobalGet(x % num_globals)); + func.instruction(Instruction::GlobalGet(y % num_globals)); + func.instruction(Instruction::GlobalGet(z % num_globals)); + func.instruction(Instruction::Call(take_refs_func_idx)); } } } @@ -250,6 +330,7 @@ mod tests { fn test_wat_string() { let ops = TableOps { num_params: 5, + num_globals: 1, table_size: 20, ops: vec![ TableOp::Gc, @@ -261,6 +342,11 @@ mod tests { TableOp::TakeFromParams(8, 9, 10), TableOp::TakeFromGet(11, 12, 13), TableOp::TakeFromMake, + TableOp::GetGlobal(14), + TableOp::SetGlobalFromParam(15, 16), + TableOp::SetGlobalFromGet(17, 18), + TableOp::SetGlobalFromMake(19, 20, 21), + TableOp::TakeFromGlobalGet(22, 23, 24), ], }; @@ -320,9 +406,25 @@ mod tests { call 1 call 2 call 1 + global.get 0 + drop + local.get 1 + global.set 0 + i32.const 18 + table.get 0 + global.set 0 + call 2 + global.set 0 + global.set 0 + global.set 0 + global.get 0 + global.get 0 + global.get 0 + call 1 br 0 (;@1;) end) (table (;0;) 20 externref) + (global (;0;) (mut externref) (ref.null extern)) (export "run" (func 3))) "#; eprintln!("expected WAT = {}", expected); diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index 3b443e5e1ded..c1feba550986 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -540,6 +540,9 @@ pub fn table_ops(mut fuzz_config: generators::Config, ops: generators::table_ops .map(|_| Val::ExternRef(Some(ExternRef::new(CountDrops(num_dropped.clone()))))) .collect(); let _ = run.call(&mut store, &args, &mut []); + + // Do a final GC after running the Wasm. + store.gc(); } assert_eq!(num_dropped.load(SeqCst), expected_drops.load(SeqCst)); diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 9677b39afa7e..573dd4b24f5a 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -463,7 +463,7 @@ impl Deref for VMExternRef { /// /// We use this so that we can morally put `VMExternRef`s inside of `HashSet`s /// even though they don't implement `Eq` and `Hash` to avoid foot guns. -#[derive(Clone)] +#[derive(Clone, Debug)] struct VMExternRefWithTraits(VMExternRef); impl Hash for VMExternRefWithTraits { @@ -940,7 +940,9 @@ pub unsafe fn gc( debug_assert!( r.is_null() || activations_table_set.contains(&r), "every on-stack externref inside a Wasm frame should \ - have an entry in the VMExternRefActivationsTable" + have an entry in the VMExternRefActivationsTable; \ + {:?} is not in the table", + r ); if let Some(r) = NonNull::new(r) { VMExternRefActivationsTable::insert_precise_stack_root( diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 2544e1673987..a6b29116727c 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -402,6 +402,16 @@ pub unsafe extern "C" fn wasmtime_activations_table_insert_with_gc( let externref = VMExternRef::clone_from_raw(externref); let instance = (*vmctx).instance(); let (activations_table, module_info_lookup) = (*instance.store()).externref_activations_table(); + + // Invariant: all `externref`s on the stack have an entry in the activations + // table. So we need to ensure that this `externref` is in the table + // *before* we GC, even though `insert_with_gc` will ensure that it is in + // the table *after* the GC. This technically results in one more hash table + // look up than is strictly necessary -- which we could avoid by having an + // additional GC method that is aware of these GC-triggering references -- + // but it isn't really a concern because this is already a slow path. + activations_table.insert_without_gc(externref.clone()); + activations_table.insert_with_gc(externref, module_info_lookup); }