From fa0b5ee8ee1ac450e53e73e6dea4f7ec270af6a4 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 18 Jun 2020 11:04:40 -0700 Subject: [PATCH] wasmtime: Add support for `func.ref` and `table.grow` with `funcref`s Part of #929 --- build.rs | 3 +- crates/environ/src/func_environ.rs | 65 +++++++----------- crates/runtime/src/externref.rs | 5 +- crates/runtime/src/funcref.rs | 68 +++++++++++++++++++ crates/runtime/src/instance.rs | 10 ++- crates/runtime/src/lib.rs | 2 + crates/runtime/src/libcalls.rs | 63 +++++++++++++++-- crates/runtime/src/table.rs | 11 +++ crates/runtime/src/vmcontext.rs | 4 +- crates/wasmtime/src/instance.rs | 4 +- .../wasmtime/src/trampoline/create_handle.rs | 4 +- .../table_grow_with_funcref.wast | 13 ++++ 12 files changed, 195 insertions(+), 57 deletions(-) create mode 100644 crates/runtime/src/funcref.rs create mode 100644 tests/misc_testsuite/reference-types/table_grow_with_funcref.wast diff --git a/build.rs b/build.rs index 1fae123f2fb7..0cbb9a02bf03 100644 --- a/build.rs +++ b/build.rs @@ -213,7 +213,8 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { ("reference_types", "table_copy_on_imported_tables") | ("reference_types", "externref_id_function") - | ("reference_types", "table_size") => { + | ("reference_types", "table_size") + | ("reference_types", "table_grow_with_funcref") => { // TODO(#1886): Ignore if this isn't x64, because Cranelift only // supports reference types on x64. return env::var("CARGO_CFG_TARGET_ARCH").unwrap() != "x86_64"; diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index a00c4116dfd1..ac01865771a9 100644 --- a/crates/environ/src/func_environ.rs +++ b/crates/environ/src/func_environ.rs @@ -10,8 +10,8 @@ use cranelift_codegen::ir::{AbiParam, ArgumentPurpose, Function, InstBuilder, Si use cranelift_codegen::isa::{self, TargetFrontendConfig}; use cranelift_entity::EntityRef; use cranelift_wasm::{ - self, FuncIndex, GlobalIndex, GlobalVariable, MemoryIndex, SignatureIndex, TableElementType, - TableIndex, TargetEnvironment, WasmError, WasmResult, + self, FuncIndex, GlobalIndex, GlobalVariable, MemoryIndex, SignatureIndex, TableIndex, + TargetEnvironment, WasmError, WasmResult, }; #[cfg(feature = "lightbeam")] use cranelift_wasm::{DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex}; @@ -161,8 +161,10 @@ declare_builtin_functions! { memory_init(vmctx, i32, i32, i32, i32, i32) -> (); /// Returns an index for wasm's `data.drop` instruction. data_drop(vmctx, i32) -> (); - /// Returns an index for Wasm's `table.grow` instruction for `externref`s. - table_grow_extern_ref(vmctx, i32, i32, reference) -> (i32); + /// Returns an index for Wasm's `table.grow` instruction. + table_grow(vmctx, i32, i32, reference) -> (i32); + /// Returns an index for Wasm's `ref.func` instruction. + ref_func(vmctx, i32) -> (reference); } impl BuiltinFunctionIndex { @@ -280,26 +282,6 @@ impl<'module_environment> FuncEnvironment<'module_environment> { } } - fn get_table_grow_func( - &mut self, - func: &mut Function, - table_index: TableIndex, - ) -> WasmResult<(ir::SigRef, BuiltinFunctionIndex, usize)> { - match self.module.table_plans[table_index].table.ty { - TableElementType::Func => Err(WasmError::Unsupported( - "the `table.grow` instruction is not supported with `funcref` yet".into(), - )), - TableElementType::Val(ty) => { - assert_eq!(ty, self.reference_type()); - Ok(( - self.builtin_function_signatures.table_grow_extern_ref(func), - BuiltinFunctionIndex::table_grow_extern_ref(), - table_index.as_u32() as usize, - )) - } - } - } - fn get_table_copy_func( &mut self, func: &mut Function, @@ -625,24 +607,19 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m delta: ir::Value, init_value: ir::Value, ) -> WasmResult { - let (func_sig, func_idx, table_index_arg) = - self.get_table_grow_func(&mut pos.func, table_index)?; - - let table_index_arg = pos.ins().iconst(I32, table_index_arg as i64); + let table_index_arg = pos.ins().iconst(I32, table_index.as_u32() as i64); + let func_idx = BuiltinFunctionIndex::table_grow(); let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx); + + let func_sig = self.builtin_function_signatures.table_grow(&mut pos.func); let call_inst = pos.ins().call_indirect( func_sig, func_addr, &[vmctx, table_index_arg, delta, init_value], ); - Ok(pos - .func - .dfg - .inst_results(call_inst) - .first() - .copied() - .unwrap()) + + Ok(pos.func.dfg.first_result(call_inst)) } fn translate_table_get( @@ -683,12 +660,20 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_ref_func( &mut self, - _: cranelift_codegen::cursor::FuncCursor<'_>, - _: FuncIndex, + mut pos: cranelift_codegen::cursor::FuncCursor<'_>, + func_index: FuncIndex, ) -> WasmResult { - Err(WasmError::Unsupported( - "the `ref.func` instruction is not supported yet".into(), - )) + let func_sig = self.builtin_function_signatures.ref_func(&mut pos.func); + let ref_func_index = BuiltinFunctionIndex::ref_func(); + let func_index_arg = pos.ins().iconst(I32, func_index.as_u32() as i64); + + let (vmctx, func_addr) = + self.translate_load_builtin_function_address(&mut pos, ref_func_index); + let call_inst = pos + .ins() + .call_indirect(func_sig, func_addr, &[vmctx, func_index_arg]); + + Ok(pos.func.dfg.first_result(call_inst)) } fn translate_custom_global_get( diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 04c2bddbeb6c..a085b2c4d697 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -545,7 +545,10 @@ impl VMExternRefActivationsTable { return Err(externref); } - debug_assert!((*next.as_ref().get()).is_none()); + debug_assert!( + (*next.as_ref().get()).is_none(), + "slots >= the `next` bump finger are always `None`" + ); ptr::write(next.as_ptr(), UnsafeCell::new(Some(externref))); let next = NonNull::new_unchecked(next.as_ptr().add(1)); diff --git a/crates/runtime/src/funcref.rs b/crates/runtime/src/funcref.rs new file mode 100644 index 000000000000..54445a3641de --- /dev/null +++ b/crates/runtime/src/funcref.rs @@ -0,0 +1,68 @@ +use crate::{VMCallerCheckedAnyfunc, VMExternRef}; +use std::any::Any; +use std::convert::TryFrom; +use std::fmt; + +/// A `VMFuncRef` is a `funcref` as compiled Wasm code sees it. +/// +/// Internally, a `VMFuncRef` is a `VMExternRef` that always points to a +/// `VMCallerCheckedAnyfunc`. This means that it is managed by the same GC +/// discipline that `VMExternRef` is managed by. It also means that it is +/// represented with a single word (either an `r32` or an `r64` depending on the +/// target pointer width) and is therefore suitable for Wasm code to manipulate +/// on-stack and pass into libcalls (since Cranelift does not support compound +/// structures). +#[derive(Clone)] +#[repr(transparent)] +pub struct VMFuncRef { + inner: VMExternRef, +} + +impl VMFuncRef { + /// Create a new `funcref` from the given `VMCallerCheckedAnyfunc`. + pub fn new(func: VMCallerCheckedAnyfunc) -> Self { + VMFuncRef { + inner: VMExternRef::new(func), + } + } + + /// Get this `funcref`'s inner `VMCallerCheckedAnyfunc`. + pub fn as_func(&self) -> &VMCallerCheckedAnyfunc { + // We always control the inner `VMExternRef`, so we know it is always a + // `VMCallerCheckedAnyfunc`, and we can do an unsafe cast here. + debug_assert!(self.inner.is::()); + let p: *const dyn Any = &*self.inner as _; + let p = p as *const VMCallerCheckedAnyfunc; + unsafe { &*p } + } + + /// Get this `funcref`'s inner `VMExternRef`. + pub fn as_externref(&self) -> &VMExternRef { + &self.inner + } +} + +impl fmt::Debug for VMFuncRef { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let func = self.as_func(); + f.debug_struct("VMFuncRef").field("func", &func).finish() + } +} + +impl From for VMExternRef { + fn from(f: VMFuncRef) -> VMExternRef { + f.inner + } +} + +impl TryFrom for VMFuncRef { + type Error = VMExternRef; + + fn try_from(r: VMExternRef) -> Result { + if r.is::() { + Ok(VMFuncRef { inner: r }) + } else { + Err(r) + } + } +} diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index aea72a0b289e..85becbbe4715 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -27,7 +27,8 @@ use thiserror::Error; use wasmtime_environ::entity::{packed_option::ReservedValue, BoxedSlice, EntityRef, PrimaryMap}; use wasmtime_environ::wasm::{ DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, - ElemIndex, FuncIndex, GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableIndex, + ElemIndex, FuncIndex, GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableElementType, + TableIndex, }; use wasmtime_environ::{ir, DataInitializer, EntityIndex, Module, TableElements, VMOffsets}; @@ -448,6 +449,11 @@ impl Instance { foreign_instance.memory_size(foreign_index) } + pub(crate) fn table_element_type(&self, table_index: TableIndex) -> TableElementType { + let table = self.get_table(table_index); + table.element_type() + } + /// Grow table by the specified amount of elements, filling them with /// `init_value`. /// @@ -514,7 +520,7 @@ impl Instance { } /// Get a `VMCallerCheckedAnyfunc` for the given `FuncIndex`. - fn get_caller_checked_anyfunc(&self, index: FuncIndex) -> VMCallerCheckedAnyfunc { + pub(crate) fn get_caller_checked_anyfunc(&self, index: FuncIndex) -> VMCallerCheckedAnyfunc { if index == FuncIndex::reserved_value() { return VMCallerCheckedAnyfunc::default(); } diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index f0a8d842718d..87f7be0f05df 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -23,6 +23,7 @@ mod export; mod externref; +mod funcref; mod imports; mod instance; mod jit_int; @@ -38,6 +39,7 @@ pub mod libcalls; pub use crate::export::*; pub use crate::externref::*; +pub use crate::funcref::*; pub use crate::imports::Imports; pub use crate::instance::{InstanceHandle, InstantiationError, LinkError}; pub use crate::jit_int::GdbJitImageRegistration; diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 2b730e84787c..a2ebb3fb109f 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -57,10 +57,15 @@ //! ``` use crate::externref::VMExternRef; -use crate::table::{Table, TableElement}; +use crate::funcref::VMFuncRef; +use crate::table::Table; use crate::traphandlers::raise_lib_trap; -use crate::vmcontext::VMContext; -use wasmtime_environ::wasm::{DataIndex, DefinedMemoryIndex, ElemIndex, MemoryIndex, TableIndex}; +use crate::vmcontext::{VMCallerCheckedAnyfunc, VMContext}; +use std::convert::TryFrom; +use wasmtime_environ::ir; +use wasmtime_environ::wasm::{ + DataIndex, DefinedMemoryIndex, ElemIndex, FuncIndex, MemoryIndex, TableElementType, TableIndex, +}; /// Implementation of f32.ceil pub extern "C" fn wasmtime_f32_ceil(x: f32) -> f32 { @@ -227,7 +232,7 @@ pub unsafe extern "C" fn wasmtime_imported_memory32_size( } /// Implementation of `table.grow` for `externref`s. -pub unsafe extern "C" fn wasmtime_table_grow_extern_ref( +pub unsafe extern "C" fn wasmtime_table_grow( vmctx: *mut VMContext, table_index: u32, delta: u32, @@ -241,9 +246,35 @@ pub unsafe extern "C" fn wasmtime_table_grow_extern_ref( let instance = (&mut *vmctx).instance(); let table_index = TableIndex::from_u32(table_index); - instance - .table_grow(table_index, delta, TableElement::ExternRef(init_value)) - .unwrap_or(-1_i32 as u32) + match instance.table_element_type(table_index) { + TableElementType::Func => { + let func = match init_value { + None => VMCallerCheckedAnyfunc::default(), + Some(r) => VMFuncRef::try_from(r) + .expect("due to validation") + .as_func() + .clone(), + }; + instance + .table_grow(table_index, delta, func.into()) + .unwrap_or(-1_i32 as u32) + } + TableElementType::Val(ty) => { + assert_eq!( + ty, + if cfg!(target_pointer_width = "32") { + ir::types::R32 + } else if cfg!(target_pointer_width = "64") { + ir::types::R64 + } else { + unreachable!() + } + ); + instance + .table_grow(table_index, delta, init_value.into()) + .unwrap_or(-1_i32 as u32) + } + } } /// Implementation of `table.copy`. @@ -393,3 +424,21 @@ pub unsafe extern "C" fn wasmtime_data_drop(vmctx: *mut VMContext, data_index: u let instance = (&mut *vmctx).instance(); instance.data_drop(data_index) } + +/// Implementation of `ref.func`. +pub unsafe extern "C" fn wasmtime_ref_func(vmctx: *mut VMContext, func_index: u32) -> *mut u8 { + let instance = (&mut *vmctx).instance(); + + let func_index = FuncIndex::from_u32(func_index); + let func = instance.get_caller_checked_anyfunc(func_index); + + let funcref = VMFuncRef::new(func); + let externref: VMExternRef = funcref.into(); + let raw_ref = externref.as_raw(); + + let activations_table = &**instance.externref_activations_table(); + let registry = &**instance.stack_map_registry(); + activations_table.insert_with_gc(externref, registry); + + raw_ref +} diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index 4023e250738e..c4a268edf614 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -57,6 +57,17 @@ impl Table { } } + /// Returns the type of the elements in this table. + pub fn element_type(&self) -> TableElementType { + match &*self.elements.borrow() { + TableElements::FuncRefs(_) => TableElementType::Func, + #[cfg(target_pointer_width = "32")] + TableElements::ExternRefs(_) => TableElementType::Val(ir::types::R32), + #[cfg(target_pointer_width = "64")] + TableElements::ExternRefs(_) => TableElementType::Val(ir::types::R64), + } + } + /// Returns the number of allocated elements. pub fn size(&self) -> u32 { match &*self.elements.borrow() { diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index e9f681926cb6..69cc7e74dd19 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -549,8 +549,7 @@ impl VMBuiltinFunctionsArray { ptrs[BuiltinFunctionIndex::imported_memory32_size().index() as usize] = wasmtime_imported_memory32_size as usize; ptrs[BuiltinFunctionIndex::table_copy().index() as usize] = wasmtime_table_copy as usize; - ptrs[BuiltinFunctionIndex::table_grow_extern_ref().index() as usize] = - wasmtime_table_grow_extern_ref as usize; + ptrs[BuiltinFunctionIndex::table_grow().index() as usize] = wasmtime_table_grow as usize; ptrs[BuiltinFunctionIndex::table_init().index() as usize] = wasmtime_table_init as usize; ptrs[BuiltinFunctionIndex::elem_drop().index() as usize] = wasmtime_elem_drop as usize; ptrs[BuiltinFunctionIndex::defined_memory_copy().index() as usize] = @@ -562,6 +561,7 @@ impl VMBuiltinFunctionsArray { wasmtime_imported_memory_fill as usize; ptrs[BuiltinFunctionIndex::memory_init().index() as usize] = wasmtime_memory_init as usize; ptrs[BuiltinFunctionIndex::data_drop().index() as usize] = wasmtime_data_drop as usize; + ptrs[BuiltinFunctionIndex::ref_func().index() as usize] = wasmtime_ref_func as usize; if cfg!(debug_assertions) { for i in 0..ptrs.len() { diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 1477c0a0ba59..3f5fa4186f33 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -50,8 +50,8 @@ fn instantiate( config.memory_creator.as_ref().map(|a| a as _), store.interrupts().clone(), host, - &*store.externref_activations_table() as *const _ as *mut _, - &*store.stack_map_registry() as *const _ as *mut _, + &**store.externref_activations_table() as *const _ as *mut _, + &**store.stack_map_registry() as *const _ as *mut _, )?; // After we've created the `InstanceHandle` we still need to run diff --git a/crates/wasmtime/src/trampoline/create_handle.rs b/crates/wasmtime/src/trampoline/create_handle.rs index 12faef120e10..089dd2397ca5 100644 --- a/crates/wasmtime/src/trampoline/create_handle.rs +++ b/crates/wasmtime/src/trampoline/create_handle.rs @@ -46,8 +46,8 @@ pub(crate) fn create_handle( signatures.into_boxed_slice(), state, store.interrupts().clone(), - &*store.externref_activations_table() as *const _ as *mut _, - &*store.stack_map_registry() as *const _ as *mut _, + &**store.externref_activations_table() as *const _ as *mut _, + &**store.stack_map_registry() as *const _ as *mut _, )?; Ok(store.add_instance(handle)) } diff --git a/tests/misc_testsuite/reference-types/table_grow_with_funcref.wast b/tests/misc_testsuite/reference-types/table_grow_with_funcref.wast new file mode 100644 index 000000000000..029f032f02ba --- /dev/null +++ b/tests/misc_testsuite/reference-types/table_grow_with_funcref.wast @@ -0,0 +1,13 @@ +(module + (table $t 0 funcref) + (func (export "size") (result i32) + (table.size $t) + ) + (func $f (export "grow-by-1") (result i32) + (table.grow $t (ref.func $f) (i32.const 1)) + ) +) + +(assert_return (invoke "size") (i32.const 0)) +(assert_return (invoke "grow-by-1") (i32.const 0)) +(assert_return (invoke "size") (i32.const 1))