diff --git a/build.rs b/build.rs index fa89812ed91b..a55f3b5da16f 100644 --- a/build.rs +++ b/build.rs @@ -201,11 +201,7 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { ("simd", "simd_splat") => return true, // FIXME Unsupported feature: proposed SIMD operator I32x4TruncSatF32x4S // Still working on implementing these. See #929. - ("reference_types", "global") - | ("reference_types", "linking") - | ("reference_types", "ref_func") - | ("reference_types", "ref_null") - | ("reference_types", "table_fill") => { + ("reference_types", "table_fill") => { return true; } diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index f1e738748232..e325d7696963 100644 --- a/crates/environ/src/func_environ.rs +++ b/crates/environ/src/func_environ.rs @@ -175,6 +175,10 @@ declare_builtin_functions! { /// Returns an index to do a GC and then insert a `VMExternRef` into the /// `VMExternRefActivationsTable`. activations_table_insert_with_gc(vmctx, reference) -> (); + /// Returns an index for Wasm's `global.get` instruction for `externref`s. + externref_global_get(vmctx, i32) -> (reference); + /// Returns an index for Wasm's `global.get` instruction for `externref`s. + externref_global_set(vmctx, i32, reference) -> (); } impl BuiltinFunctionIndex { @@ -432,6 +436,28 @@ impl<'module_environment> FuncEnvironment<'module_environment> { new_ref_count } + + fn get_global_location( + &mut self, + func: &mut ir::Function, + index: GlobalIndex, + ) -> (ir::GlobalValue, i32) { + let pointer_type = self.pointer_type(); + let vmctx = self.vmctx(func); + if let Some(def_index) = self.module.defined_global_index(index) { + let offset = i32::try_from(self.offsets.vmctx_vmglobal_definition(def_index)).unwrap(); + (vmctx, offset) + } else { + let from_offset = self.offsets.vmctx_vmglobal_import_from(index); + let global = func.create_global_value(ir::GlobalValueData::Load { + base: vmctx, + offset: Offset32::new(i32::try_from(from_offset).unwrap()), + global_type: pointer_type, + readonly: true, + }); + (global, 0) + } + } } // TODO: This is necessary as if Lightbeam used `FuncEnvironment` directly it would cause @@ -1042,19 +1068,56 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_custom_global_get( &mut self, - _: cranelift_codegen::cursor::FuncCursor<'_>, - _: cranelift_wasm::GlobalIndex, + mut pos: cranelift_codegen::cursor::FuncCursor<'_>, + index: cranelift_wasm::GlobalIndex, ) -> WasmResult { - unreachable!("we don't make any custom globals") + debug_assert_eq!( + self.module.globals[index].wasm_ty, + WasmType::ExternRef, + "We only use GlobalVariable::Custom for externref" + ); + + let builtin_index = BuiltinFunctionIndex::externref_global_get(); + let builtin_sig = self + .builtin_function_signatures + .externref_global_get(&mut pos.func); + + let (vmctx, builtin_addr) = + self.translate_load_builtin_function_address(&mut pos, builtin_index); + + let global_index_arg = pos.ins().iconst(I32, index.as_u32() as i64); + let call_inst = + pos.ins() + .call_indirect(builtin_sig, builtin_addr, &[vmctx, global_index_arg]); + + Ok(pos.func.dfg.first_result(call_inst)) } fn translate_custom_global_set( &mut self, - _: cranelift_codegen::cursor::FuncCursor<'_>, - _: cranelift_wasm::GlobalIndex, - _: ir::Value, + mut pos: cranelift_codegen::cursor::FuncCursor<'_>, + index: cranelift_wasm::GlobalIndex, + value: ir::Value, ) -> WasmResult<()> { - unreachable!("we don't make any custom globals") + debug_assert_eq!( + self.module.globals[index].wasm_ty, + WasmType::ExternRef, + "We only use GlobalVariable::Custom for externref" + ); + + let builtin_index = BuiltinFunctionIndex::externref_global_set(); + let builtin_sig = self + .builtin_function_signatures + .externref_global_set(&mut pos.func); + + let (vmctx, builtin_addr) = + self.translate_load_builtin_function_address(&mut pos, builtin_index); + + let global_index_arg = pos.ins().iconst(I32, index.as_u32() as i64); + pos.ins() + .call_indirect(builtin_sig, builtin_addr, &[vmctx, global_index_arg, value]); + + Ok(()) } fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> WasmResult { @@ -1141,28 +1204,19 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m func: &mut ir::Function, index: GlobalIndex, ) -> WasmResult { - let pointer_type = self.pointer_type(); - - let (ptr, offset) = { - let vmctx = self.vmctx(func); - if let Some(def_index) = self.module.defined_global_index(index) { - let offset = - i32::try_from(self.offsets.vmctx_vmglobal_definition(def_index)).unwrap(); - (vmctx, offset) - } else { - let from_offset = self.offsets.vmctx_vmglobal_import_from(index); - let global = func.create_global_value(ir::GlobalValueData::Load { - base: vmctx, - offset: Offset32::new(i32::try_from(from_offset).unwrap()), - global_type: pointer_type, - readonly: true, - }); - (global, 0) - } - }; + // Although `ExternRef`s live at the same memory location as any other + // type of global at the same index would, getting or setting them + // requires ref counting barriers. Therefore, we need to use + // `GlobalVariable::Custom`, as that is the only kind of + // `GlobalVariable` for which `cranelift-wasm` supports custom access + // translation. + if self.module.globals[index].wasm_ty == WasmType::ExternRef { + return Ok(GlobalVariable::Custom); + } + let (gv, offset) = self.get_global_location(func, index); Ok(GlobalVariable::Memory { - gv: ptr, + gv, offset: offset.into(), ty: self.module.globals[index].ty, }) diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 4395fefd467b..ea5045ea83da 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -29,7 +29,7 @@ use wasmtime_environ::entity::{packed_option::ReservedValue, BoxedSlice, EntityR use wasmtime_environ::wasm::{ DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, ElemIndex, FuncIndex, GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableElementType, - TableIndex, + TableIndex, WasmType, }; use wasmtime_environ::{ir, DataInitializer, EntityIndex, Module, TableElements, VMOffsets}; @@ -226,6 +226,21 @@ impl Instance { unsafe { self.globals_ptr().add(index) } } + /// Get a raw pointer to the global at the given index regardless whether it + /// is defined locally or imported from another module. + /// + /// Panics if the index is out of bound or is the reserved value. + pub(crate) fn defined_or_imported_global_ptr( + &self, + index: GlobalIndex, + ) -> *mut VMGlobalDefinition { + if let Some(index) = self.module.local.defined_global_index(index) { + self.global_ptr(index) + } else { + self.imported_global(index).from + } + } + /// Return a pointer to the `VMGlobalDefinition`s. fn globals_ptr(&self) -> *mut VMGlobalDefinition { unsafe { self.vmctx_plus_offset(self.offsets.vmctx_globals_begin()) } @@ -1390,8 +1405,16 @@ fn initialize_globals(instance: &Instance) { }; *to = from; } + GlobalInit::RefFunc(f) => { + *(*to).as_anyfunc_mut() = instance.get_caller_checked_anyfunc(f).unwrap() + as *const VMCallerCheckedAnyfunc; + } + GlobalInit::RefNullConst => match global.wasm_ty { + WasmType::FuncRef => *(*to).as_anyfunc_mut() = ptr::null(), + WasmType::ExternRef => *(*to).as_externref_mut() = None, + ty => panic!("unsupported reference type for global: {:?}", ty), + }, GlobalInit::Import => panic!("locally-defined global initialized as import"), - GlobalInit::RefNullConst | GlobalInit::RefFunc(_) => unimplemented!(), } } } diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index eb62d21311cf..c1ff290656d4 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -60,9 +60,11 @@ use crate::externref::VMExternRef; use crate::table::Table; use crate::traphandlers::raise_lib_trap; use crate::vmcontext::{VMCallerCheckedAnyfunc, VMContext}; -use std::ptr::NonNull; +use std::mem; +use std::ptr::{self, NonNull}; use wasmtime_environ::wasm::{ - DataIndex, DefinedMemoryIndex, ElemIndex, MemoryIndex, TableElementType, TableIndex, + DataIndex, DefinedMemoryIndex, ElemIndex, GlobalIndex, MemoryIndex, TableElementType, + TableIndex, }; /// Implementation of f32.ceil @@ -430,3 +432,47 @@ pub unsafe extern "C" fn wasmtime_activations_table_insert_with_gc( let registry = &**instance.stack_map_registry(); activations_table.insert_with_gc(externref, registry); } + +/// Perform a Wasm `global.get` for `externref` globals. +pub unsafe extern "C" fn wasmtime_externref_global_get( + vmctx: *mut VMContext, + index: u32, +) -> *mut u8 { + let index = GlobalIndex::from_u32(index); + let instance = (&mut *vmctx).instance(); + let global = instance.defined_or_imported_global_ptr(index); + match (*global).as_externref().clone() { + None => ptr::null_mut(), + Some(externref) => { + let raw = externref.as_raw(); + let activations_table = &**instance.externref_activations_table(); + let registry = &**instance.stack_map_registry(); + activations_table.insert_with_gc(externref, registry); + raw + } + } +} + +/// Perform a Wasm `global.set` for `externref` globals. +pub unsafe extern "C" fn wasmtime_externref_global_set( + vmctx: *mut VMContext, + index: u32, + externref: *mut u8, +) { + let externref = if externref.is_null() { + None + } else { + Some(VMExternRef::clone_from_raw(externref)) + }; + + let index = GlobalIndex::from_u32(index); + let instance = (&mut *vmctx).instance(); + let global = instance.defined_or_imported_global_ptr(index); + + // Swap the new `externref` value into the global before we drop the old + // value. This protects against an `externref` with a `Drop` implementation + // that calls back into Wasm and touches this global again (we want to avoid + // it observing a halfway-deinitialized value). + let old = mem::replace((*global).as_externref_mut(), externref); + drop(old); +} diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index c98c75b362da..c2b8660a908c 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -1,6 +1,7 @@ //! This file declares `VMContext` and several related structs which contain //! fields that compiled wasm code accesses directly. +use crate::externref::VMExternRef; use crate::instance::Instance; use std::any::Any; use std::ptr::NonNull; @@ -296,6 +297,11 @@ mod test_vmglobal_definition { let offsets = VMOffsets::new(size_of::<*mut u8>() as u8, &module.local); assert_eq!(offsets.vmctx_globals_begin() % 16, 0); } + + #[test] + fn check_vmglobal_can_contain_externref() { + assert!(size_of::() <= size_of::()); + } } impl VMGlobalDefinition { @@ -423,6 +429,30 @@ impl VMGlobalDefinition { pub unsafe fn as_u128_bits_mut(&mut self) -> &mut [u8; 16] { &mut *(self.storage.as_mut().as_mut_ptr() as *mut [u8; 16]) } + + /// Return a reference to the value as an externref. + #[allow(clippy::cast_ptr_alignment)] + pub unsafe fn as_externref(&self) -> &Option { + &*(self.storage.as_ref().as_ptr() as *const Option) + } + + /// Return a mutable reference to the value as an externref. + #[allow(clippy::cast_ptr_alignment)] + pub unsafe fn as_externref_mut(&mut self) -> &mut Option { + &mut *(self.storage.as_mut().as_mut_ptr() as *mut Option) + } + + /// Return a reference to the value as an anyfunc. + #[allow(clippy::cast_ptr_alignment)] + pub unsafe fn as_anyfunc(&self) -> *const VMCallerCheckedAnyfunc { + *(self.storage.as_ref().as_ptr() as *const *const VMCallerCheckedAnyfunc) + } + + /// Return a mutable reference to the value as an anyfunc. + #[allow(clippy::cast_ptr_alignment)] + pub unsafe fn as_anyfunc_mut(&mut self) -> &mut *const VMCallerCheckedAnyfunc { + &mut *(self.storage.as_mut().as_mut_ptr() as *mut *const VMCallerCheckedAnyfunc) + } } /// An index into the shared signature registry, usable for checking signatures @@ -559,6 +589,10 @@ impl VMBuiltinFunctionsArray { wasmtime_drop_externref as usize; ptrs[BuiltinFunctionIndex::activations_table_insert_with_gc().index() as usize] = wasmtime_activations_table_insert_with_gc as usize; + ptrs[BuiltinFunctionIndex::externref_global_get().index() as usize] = + wasmtime_externref_global_get as usize; + ptrs[BuiltinFunctionIndex::externref_global_set().index() as usize] = + wasmtime_externref_global_set as usize; if cfg!(debug_assertions) { for i in 0..ptrs.len() { diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 93c2ddc66544..03b4ef442262 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -7,6 +7,8 @@ use crate::{ ValType, }; use anyhow::{anyhow, bail, Result}; +use std::mem; +use std::ptr; use std::slice; use wasmtime_environ::wasm; use wasmtime_runtime::{self as runtime, InstanceHandle}; @@ -227,6 +229,15 @@ impl Global { ValType::I64 => Val::from(*definition.as_i64()), ValType::F32 => Val::F32(*definition.as_u32()), ValType::F64 => Val::F64(*definition.as_u64()), + ValType::ExternRef => Val::ExternRef( + definition + .as_externref() + .clone() + .map(|inner| ExternRef { inner }), + ), + ValType::FuncRef => { + from_checked_anyfunc(definition.as_anyfunc() as *mut _, &self.instance.store) + } ty => unimplemented!("Global::get for {:?}", ty), } } @@ -256,6 +267,19 @@ impl Global { Val::I64(i) => *definition.as_i64_mut() = i, Val::F32(f) => *definition.as_u32_mut() = f, Val::F64(f) => *definition.as_u64_mut() = f, + Val::FuncRef(f) => { + *definition.as_anyfunc_mut() = f.map_or(ptr::null(), |f| { + f.caller_checked_anyfunc().as_ptr() as *const _ + }); + } + Val::ExternRef(x) => { + // In case the old value's `Drop` implementation is + // re-entrant and tries to touch this global again, do a + // replace, and then drop. This way no one can observe a + // halfway-deinitialized value. + let old = mem::replace(definition.as_externref_mut(), x.map(|x| x.inner)); + drop(old); + } _ => unimplemented!("Global::set for {:?}", val.ty()), } } diff --git a/tests/all/gc.rs b/tests/all/gc.rs index b2a47d31046d..87aa82ba419a 100644 --- a/tests/all/gc.rs +++ b/tests/all/gc.rs @@ -517,3 +517,95 @@ fn pass_externref_into_wasm_during_destructor_in_gc() -> anyhow::Result<()> { } } } + +#[test] +fn gc_on_drop_in_mutable_externref_global() -> anyhow::Result<()> { + let (store, module) = ref_types_module( + r#" + (module + (global $g (mut externref) (ref.null extern)) + + (func (export "set-g") (param externref) + (global.set $g (local.get 0)) + ) + ) + "#, + )?; + + let instance = Instance::new(&store, &module, &[])?; + let set_g = instance.get_func("set-g").unwrap(); + + let gc_count = Rc::new(Cell::new(0)); + + // Put a `GcOnDrop` into the global. + { + let args = vec![Val::ExternRef(Some(ExternRef::new(GcOnDrop { + store: store.clone(), + gc_count: gc_count.clone(), + })))]; + set_g.call(&args)?; + } + + // Remove the `GcOnDrop` from the `VMExternRefActivationsTable`. + store.gc(); + + // Overwrite the `GcOnDrop` global value, causing it to be dropped, and + // triggering a GC. + set_g.call(&[Val::ExternRef(None)])?; + assert_eq!(gc_count.get(), 1); + + Ok(()) +} + +#[test] +fn touch_own_externref_global_on_drop() -> anyhow::Result<()> { + let (store, module) = ref_types_module( + r#" + (module + (global $g (export "g") (mut externref) (ref.null extern)) + + (func (export "set-g") (param externref) + (global.set $g (local.get 0)) + ) + ) + "#, + )?; + + let instance = Instance::new(&store, &module, &[])?; + let g = instance.get_global("g").unwrap(); + let set_g = instance.get_func("set-g").unwrap(); + + let touched = Rc::new(Cell::new(false)); + + { + let args = vec![Val::ExternRef(Some(ExternRef::new(TouchGlobalOnDrop { + g, + touched: touched.clone(), + })))]; + set_g.call(&args)?; + } + + // Remove the `TouchGlobalOnDrop` from the `VMExternRefActivationsTable`. + store.gc(); + + set_g.call(&[Val::ExternRef(Some(ExternRef::new("hello".to_string())))])?; + assert!(touched.get()); + + return Ok(()); + + struct TouchGlobalOnDrop { + g: Global, + touched: Rc>, + } + + impl Drop for TouchGlobalOnDrop { + fn drop(&mut self) { + // From the `Drop` implementation, we see the new global value, not + // `self`. + let r = self.g.get().unwrap_externref().unwrap(); + assert!(r.data().is::()); + assert_eq!(r.data().downcast_ref::().unwrap(), "hello"); + self.touched.set(true); + } + } +} diff --git a/tests/misc_testsuite/reference-types/mutable_externref_globals.wast b/tests/misc_testsuite/reference-types/mutable_externref_globals.wast new file mode 100644 index 000000000000..5d4b5f839d8f --- /dev/null +++ b/tests/misc_testsuite/reference-types/mutable_externref_globals.wast @@ -0,0 +1,13 @@ +;; This test contains the changes in +;; https://github.com/WebAssembly/reference-types/pull/104, and can be deleted +;; once that merges and we update our upstream tests. + +(module + (global $mr (mut externref) (ref.null extern)) + (func (export "get-mr") (result externref) (global.get $mr)) + (func (export "set-mr") (param externref) (global.set $mr (local.get 0))) +) + +(assert_return (invoke "get-mr") (ref.null extern)) +(assert_return (invoke "set-mr" (ref.extern 10))) +(assert_return (invoke "get-mr") (ref.extern 10))