From c9cf3ec751787eb8351f043e235a6fb924a32433 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 `funcref`s are implemented as `NonNull`. This should be more efficient than using a `VMExternRef` that points at a `VMCallerCheckedAnyfunc` because it gets rid of an indirection, dynamic allocation, and some reference counting. Note that the null function reference is *NOT* a null pointer; it is a `VMCallerCheckedAnyfunc` that has a null `func_ptr` member. Part of #929 --- build.rs | 3 +- cranelift/wasm/src/sections_translator.rs | 2 + cranelift/wasm/src/translation_utils.rs | 12 +- crates/c-api/src/table.rs | 24 +-- crates/environ/src/func_environ.rs | 139 ++++++++++++------ crates/environ/src/lib.rs | 15 ++ crates/environ/src/module_environ.rs | 4 + crates/environ/src/vmoffsets.rs | 34 ++++- crates/jit/src/imports.rs | 14 +- crates/runtime/src/export.rs | 15 +- crates/runtime/src/externref.rs | 7 +- crates/runtime/src/instance.rs | 114 ++++++++------ crates/runtime/src/lib.rs | 11 ++ crates/runtime/src/libcalls.rs | 44 ++++-- crates/runtime/src/table.rs | 42 +++--- crates/runtime/src/vmcontext.rs | 23 +-- crates/wasmtime/src/externals.rs | 17 +-- crates/wasmtime/src/func.rs | 66 ++++++--- crates/wasmtime/src/instance.rs | 12 +- crates/wasmtime/src/linker.rs | 2 +- .../wasmtime/src/trampoline/create_handle.rs | 7 +- crates/wasmtime/src/trampoline/global.rs | 4 +- crates/wasmtime/src/trampoline/table.rs | 1 + crates/wasmtime/src/types.rs | 22 +-- crates/wasmtime/src/values.rs | 82 ++++++----- crates/wast/src/spectest.rs | 2 +- tests/all/externals.rs | 22 +-- tests/all/funcref.rs | 85 +++++++++++ tests/all/gc.rs | 15 +- tests/all/invoke_func_via_table.rs | 1 + tests/all/linker.rs | 4 +- tests/all/main.rs | 22 +++ tests/all/table.rs | 4 +- tests/all/use_after_drop.rs | 4 +- .../table_grow_with_funcref.wast | 13 ++ 35 files changed, 583 insertions(+), 305 deletions(-) create mode 100644 tests/all/funcref.rs create mode 100644 tests/misc_testsuite/reference-types/table_grow_with_funcref.wast diff --git a/build.rs b/build.rs index ea47c87fc8a3..0cdfd14baaff 100644 --- a/build.rs +++ b/build.rs @@ -215,7 +215,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/cranelift/wasm/src/sections_translator.rs b/cranelift/wasm/src/sections_translator.rs index a762bd0dfb10..3ae2be1b2930 100644 --- a/cranelift/wasm/src/sections_translator.rs +++ b/cranelift/wasm/src/sections_translator.rs @@ -97,6 +97,7 @@ pub fn parse_import_section<'data>( ImportSectionEntryType::Global(ref ty) => { environ.declare_global_import( Global { + wasm_ty: ty.content_type, ty: type_to_type(ty.content_type, environ).unwrap(), mutability: ty.mutable, initializer: GlobalInit::Import, @@ -229,6 +230,7 @@ pub fn parse_global_section( } }; let global = Global { + wasm_ty: content_type, ty: type_to_type(content_type, environ).unwrap(), mutability: mutable, initializer, diff --git a/cranelift/wasm/src/translation_utils.rs b/cranelift/wasm/src/translation_utils.rs index d7990279032d..4bbab4e75ab4 100644 --- a/cranelift/wasm/src/translation_utils.rs +++ b/cranelift/wasm/src/translation_utils.rs @@ -67,10 +67,18 @@ entity_impl!(DataIndex); pub struct ElemIndex(u32); entity_impl!(ElemIndex); -/// WebAssembly global. +/// A WebAssembly global. +/// +/// Note that we record both the original Wasm type and the Cranelift IR type +/// used to represent it. This is because multiple different kinds of Wasm types +/// might be represented with the same Cranelift IR type. For example, both a +/// Wasm `i64` and a `funcref` might be represented with a Cranelift `i64` on +/// 64-bit architectures, and when GC is not required for func refs. #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] pub struct Global { - /// The type of the value stored in the global. + /// The Wasm type of the value stored in the global. + pub wasm_ty: crate::WasmType, + /// The Cranelift IR type of the value stored in the global. pub ty: ir::Type, /// A flag indicating whether the value may change at runtime. pub mutability: bool, diff --git a/crates/c-api/src/table.rs b/crates/c-api/src/table.rs index 773164a59ce8..ad7d8f4cf151 100644 --- a/crates/c-api/src/table.rs +++ b/crates/c-api/src/table.rs @@ -42,7 +42,7 @@ pub extern "C" fn wasm_table_new( ) -> Option> { let init: Val = match init { Some(init) => init.r.into(), - None => Val::ExternRef(None), + None => Val::FuncRef(None), }; let table = Table::new(&store.store, tt.ty().ty.clone(), init).ok()?; Some(Box::new(wasm_table_t { @@ -60,8 +60,8 @@ pub extern "C" fn wasmtime_funcref_table_new( out: &mut *mut wasm_table_t, ) -> Option> { let init: Val = match init { - Some(val) => Val::FuncRef(val.func().borrow().clone()), - None => Val::ExternRef(None), + Some(val) => Val::FuncRef(Some(val.func().borrow().clone())), + None => Val::FuncRef(None), }; handle_result( Table::new(&store.store, tt.ty().ty.clone(), init), @@ -85,7 +85,7 @@ pub extern "C" fn wasm_table_type(t: &wasm_table_t) -> Box { pub extern "C" fn wasm_table_get(t: &wasm_table_t, index: wasm_table_size_t) -> *mut wasm_ref_t { match t.table().borrow().get(index) { Some(val) => into_funcref(val), - None => into_funcref(Val::ExternRef(None)), + None => into_funcref(Val::FuncRef(None)), } } @@ -99,14 +99,14 @@ pub extern "C" fn wasmtime_funcref_table_get( Some(val) => { *ptr = match val { // TODO: what do do about creating new `HostRef` handles here? - Val::FuncRef(f) => { + Val::FuncRef(None) => ptr::null_mut(), + Val::FuncRef(Some(f)) => { let store = match t.table().as_ref().store() { None => return false, Some(store) => store, }; Box::into_raw(Box::new(HostRef::new(&store, f).into())) } - Val::ExternRef(None) => ptr::null_mut(), _ => return false, }; } @@ -133,14 +133,14 @@ pub extern "C" fn wasmtime_funcref_table_set( val: Option<&wasm_func_t>, ) -> Option> { let val = match val { - Some(val) => Val::FuncRef(val.func().borrow().clone()), - None => Val::ExternRef(None), + Some(val) => Val::FuncRef(Some(val.func().borrow().clone())), + None => Val::FuncRef(None), }; handle_result(t.table().borrow().set(index, val), |()| {}) } fn into_funcref(val: Val) -> *mut wasm_ref_t { - if let Val::ExternRef(None) = val { + if let Val::FuncRef(None) = val { return ptr::null_mut(); } let externref = match val.externref() { @@ -155,7 +155,7 @@ unsafe fn from_funcref(r: *mut wasm_ref_t) -> Val { if !r.is_null() { Box::from_raw(r).r.into() } else { - Val::ExternRef(None) + Val::FuncRef(None) } } @@ -182,8 +182,8 @@ pub extern "C" fn wasmtime_funcref_table_grow( prev_size: Option<&mut wasm_table_size_t>, ) -> Option> { let val = match init { - Some(val) => Val::FuncRef(val.func().borrow().clone()), - None => Val::ExternRef(None), + Some(val) => Val::FuncRef(Some(val.func().borrow().clone())), + None => Val::FuncRef(None), }; handle_result(t.table().borrow().grow(delta, val), |prev| { if let Some(ptr) = prev_size { diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index 612a6f9bd556..909a9cd5474f 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, WasmType, }; #[cfg(feature = "lightbeam")] use cranelift_wasm::{DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex}; @@ -68,6 +68,10 @@ macro_rules! declare_builtin_functions { AbiParam::new(self.reference_type) } + fn pointer(&self) -> AbiParam { + AbiParam::new(self.pointer_type) + } + fn i32(&self) -> AbiParam { AbiParam::new(I32) } @@ -161,8 +165,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 `funcref`s. + table_grow_funcref(vmctx, i32, i32, pointer) -> (i32); /// Returns an index for Wasm's `table.grow` instruction for `externref`s. - table_grow_extern_ref(vmctx, i32, i32, reference) -> (i32); + table_grow_externref(vmctx, i32, i32, reference) -> (i32); } impl BuiltinFunctionIndex { @@ -280,26 +286,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, @@ -552,6 +538,10 @@ impl<'module_environment> TargetEnvironment for FuncEnvironment<'module_environm fn target_config(&self) -> TargetFrontendConfig { self.target_config } + + fn reference_type(&self, ty: WasmType) -> ir::Type { + crate::reference_type(ty, self.pointer_type()) + } } impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'module_environment> { @@ -604,9 +594,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m }); let element_size = match self.module.table_plans[index].style { - TableStyle::CallerChecksSignature => { - u64::from(self.offsets.size_of_vmcaller_checked_anyfunc()) - } + TableStyle::CallerChecksSignature => u64::from(self.pointer_type().bytes()), }; Ok(func.create_table(ir::TableData { @@ -626,24 +614,34 @@ 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 (func_idx, func_sig) = + match self.module.table_plans[table_index].table.wasm_ty { + WasmType::FuncRef => ( + BuiltinFunctionIndex::table_grow_funcref(), + self.builtin_function_signatures + .table_grow_funcref(&mut pos.func), + ), + WasmType::ExternRef => ( + BuiltinFunctionIndex::table_grow_externref(), + self.builtin_function_signatures + .table_grow_externref(&mut pos.func), + ), + _ => return Err(WasmError::Unsupported( + "`table.grow` with a table element type that is not `funcref` or `externref`" + .into(), + )), + }; let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx); + + let table_index_arg = pos.ins().iconst(I32, table_index.as_u32() as i64); 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( @@ -684,14 +682,50 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m )) } + fn translate_ref_null( + &mut self, + mut pos: cranelift_codegen::cursor::FuncCursor, + ty: WasmType, + ) -> WasmResult { + Ok(match ty { + WasmType::FuncRef => pos.ins().iconst(self.pointer_type(), 0), + WasmType::ExternRef => pos.ins().null(self.reference_type(ty)), + _ => { + return Err(WasmError::Unsupported( + "`ref.null T` that is not a `funcref` or an `externref`".into(), + )); + } + }) + } + + fn translate_ref_is_null( + &mut self, + mut pos: cranelift_codegen::cursor::FuncCursor, + value: ir::Value, + ) -> WasmResult { + let bool_is_null = match pos.func.dfg.value_type(value) { + // `externref` + ty if ty.is_ref() => pos.ins().is_null(value), + // `funcref` + ty if ty == self.pointer_type() => { + pos.ins() + .icmp_imm(cranelift_codegen::ir::condcodes::IntCC::Equal, value, 0) + } + _ => unreachable!(), + }; + + Ok(pos.ins().bint(ir::types::I32, bool_is_null)) + } + 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 vmctx = self.vmctx(&mut pos.func); + let vmctx = pos.ins().global_value(self.pointer_type(), vmctx); + let offset = self.offsets.vmctx_anyfunc(func_index); + Ok(pos.ins().iadd_imm(vmctx, i64::from(offset))) } fn translate_custom_global_get( @@ -861,18 +895,25 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m let table_entry_addr = pos.ins().table_addr(pointer_type, table, callee, 0); - // Dereference table_entry_addr to get the function address. + // Dereference the table entry to get the pointer to the + // `VMCallerCheckedAnyfunc`. + let anyfunc_ptr = + pos.ins() + .load(pointer_type, ir::MemFlags::trusted(), table_entry_addr, 0); + + // Check for whether the table element is null, and trap if so. + pos.ins() + .trapz(anyfunc_ptr, ir::TrapCode::IndirectCallToNull); + + // Dereference anyfunc pointer to get the function address. let mem_flags = ir::MemFlags::trusted(); let func_addr = pos.ins().load( pointer_type, mem_flags, - table_entry_addr, + anyfunc_ptr, i32::from(self.offsets.vmcaller_checked_anyfunc_func_ptr()), ); - // Check whether `func_addr` is null. - pos.ins().trapz(func_addr, ir::TrapCode::IndirectCallToNull); - // If necessary, check the signature. match self.module.table_plans[table_index].style { TableStyle::CallerChecksSignature => { @@ -893,7 +934,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m let callee_sig_id = pos.ins().load( sig_id_type, mem_flags, - table_entry_addr, + anyfunc_ptr, i32::from(self.offsets.vmcaller_checked_anyfunc_type_index()), ); @@ -910,7 +951,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m let vmctx = pos.ins().load( pointer_type, mem_flags, - table_entry_addr, + anyfunc_ptr, i32::from(self.offsets.vmcaller_checked_anyfunc_vmctx()), ); real_call_args.push(vmctx); diff --git a/crates/environ/src/lib.rs b/crates/environ/src/lib.rs index fa32e116e39d..3de1ec08dded 100644 --- a/crates/environ/src/lib.rs +++ b/crates/environ/src/lib.rs @@ -72,3 +72,18 @@ pub const WASM_MAX_PAGES: u32 = 0x10000; /// Version number of this crate. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); + +pub(crate) fn reference_type( + wasm_ty: cranelift_wasm::WasmType, + pointer_type: ir::Type, +) -> ir::Type { + match wasm_ty { + cranelift_wasm::WasmType::FuncRef => pointer_type, + cranelift_wasm::WasmType::ExternRef => match pointer_type { + ir::types::I32 => ir::types::R32, + ir::types::I64 => ir::types::R64, + _ => panic!("unsupported pointer type"), + }, + _ => panic!("unsupported Wasm reference type"), + } +} diff --git a/crates/environ/src/module_environ.rs b/crates/environ/src/module_environ.rs index 4876c3c2ee26..8d6cd7997f78 100644 --- a/crates/environ/src/module_environ.rs +++ b/crates/environ/src/module_environ.rs @@ -92,6 +92,10 @@ impl<'data> TargetEnvironment for ModuleEnvironment<'data> { fn target_config(&self) -> TargetFrontendConfig { self.result.target_config } + + fn reference_type(&self, ty: cranelift_wasm::WasmType) -> ir::Type { + crate::reference_type(ty, self.pointer_type()) + } } /// This trait is useful for `translate_module` because it tells how to translate diff --git a/crates/environ/src/vmoffsets.rs b/crates/environ/src/vmoffsets.rs index 0d7f08073c06..6efa5914577a 100644 --- a/crates/environ/src/vmoffsets.rs +++ b/crates/environ/src/vmoffsets.rs @@ -15,6 +15,7 @@ // tables: [VMTableDefinition; module.num_defined_tables], // memories: [VMMemoryDefinition; module.num_defined_memories], // globals: [VMGlobalDefinition; module.num_defined_globals], +// anyfuncs: [VMCallerCheckedAnyfunc; module.num_imported_functions + module.num_defined_functions], // builtins: VMBuiltinFunctionsArray, // } @@ -62,6 +63,8 @@ pub struct VMOffsets { pub num_imported_memories: u32, /// The number of imported globals in the module. pub num_imported_globals: u32, + /// The number of defined functions in the module. + pub num_defined_functions: u32, /// The number of defined tables in the module. pub num_defined_tables: u32, /// The number of defined memories in the module. @@ -80,6 +83,7 @@ impl VMOffsets { num_imported_tables: cast_to_u32(module.num_imported_tables), num_imported_memories: cast_to_u32(module.num_imported_memories), num_imported_globals: cast_to_u32(module.num_imported_globals), + num_defined_functions: cast_to_u32(module.functions.len()), num_defined_tables: cast_to_u32(module.table_plans.len()), num_defined_memories: cast_to_u32(module.memory_plans.len()), num_defined_globals: cast_to_u32(module.globals.len()), @@ -390,8 +394,8 @@ impl VMOffsets { align(offset, 16) } - /// The offset of the builtin functions array. - pub fn vmctx_builtin_functions_begin(&self) -> u32 { + /// The offset of the `anyfuncs` array. + pub fn vmctx_anyfuncs_begin(&self) -> u32 { self.vmctx_globals_begin() .checked_add( self.num_defined_globals @@ -401,6 +405,19 @@ impl VMOffsets { .unwrap() } + /// The offset of the builtin functions array. + pub fn vmctx_builtin_functions_begin(&self) -> u32 { + self.vmctx_anyfuncs_begin() + .checked_add( + self.num_imported_functions + .checked_add(self.num_defined_functions) + .unwrap() + .checked_mul(u32::from(self.size_of_vmcaller_checked_anyfunc())) + .unwrap(), + ) + .unwrap() + } + /// Return the size of the `VMContext` allocation. pub fn size_of_vmctx(&self) -> u32 { self.vmctx_builtin_functions_begin() @@ -516,6 +533,19 @@ impl VMOffsets { .unwrap() } + /// Return the offset to the `VMCallerCheckedAnyfunc` for the given function + /// index (either imported or defined). + pub fn vmctx_anyfunc(&self, index: FuncIndex) -> u32 { + self.vmctx_anyfuncs_begin() + .checked_add( + index + .as_u32() + .checked_mul(u32::from(self.size_of_vmcaller_checked_anyfunc())) + .unwrap(), + ) + .unwrap() + } + /// Return the offset to the `body` field in `*const VMFunctionBody` index `index`. pub fn vmctx_vmfunction_import_body(&self, index: FuncIndex) -> u32 { self.vmctx_vmfunction_import(index) diff --git a/crates/jit/src/imports.rs b/crates/jit/src/imports.rs index 5694daa36d3c..a6ba165fca9f 100644 --- a/crates/jit/src/imports.rs +++ b/crates/jit/src/imports.rs @@ -32,7 +32,9 @@ pub fn resolve_imports( match (import, &export) { (EntityIndex::Function(func_index), Some(Export::Function(f))) => { let import_signature = module.local.native_func_signature(*func_index); - let signature = signatures.lookup_native(f.signature).unwrap(); + let signature = signatures + .lookup_native(unsafe { f.anyfunc.as_ref().type_index }) + .unwrap(); if signature != *import_signature { // TODO: If the difference is in the calling convention, // we could emit a wrapper function to fix it up. @@ -43,8 +45,8 @@ pub fn resolve_imports( ))); } function_imports.push(VMFunctionImport { - body: f.address, - vmctx: f.vmctx, + body: unsafe { f.anyfunc.as_ref().func_ptr }, + vmctx: unsafe { f.anyfunc.as_ref().vmctx }, }); } (EntityIndex::Function(_), Some(_)) => { @@ -169,16 +171,20 @@ fn is_global_compatible(exported: &Global, imported: &Global) -> bool { } let Global { + wasm_ty: exported_wasm_ty, ty: exported_ty, mutability: exported_mutability, initializer: _exported_initializer, } = exported; let Global { + wasm_ty: imported_wasm_ty, ty: imported_ty, mutability: imported_mutability, initializer: _imported_initializer, } = imported; - exported_ty == imported_ty && imported_mutability == exported_mutability + exported_wasm_ty == imported_wasm_ty + && exported_ty == imported_ty + && imported_mutability == exported_mutability } fn is_table_element_type_compatible( diff --git a/crates/runtime/src/export.rs b/crates/runtime/src/export.rs index 8cf42b8f8f24..edfa51e1f290 100644 --- a/crates/runtime/src/export.rs +++ b/crates/runtime/src/export.rs @@ -1,7 +1,7 @@ use crate::vmcontext::{ - VMContext, VMFunctionBody, VMGlobalDefinition, VMMemoryDefinition, VMSharedSignatureIndex, - VMTableDefinition, + VMCallerCheckedAnyfunc, VMContext, VMGlobalDefinition, VMMemoryDefinition, VMTableDefinition, }; +use std::ptr::NonNull; use wasmtime_environ::wasm::Global; use wasmtime_environ::{MemoryPlan, TablePlan}; @@ -24,14 +24,11 @@ pub enum Export { /// A function export value. #[derive(Debug, Clone)] pub struct ExportFunction { - /// The address of the native-code function. - pub address: *const VMFunctionBody, - /// Pointer to the containing `VMContext`. - pub vmctx: *mut VMContext, - /// The function signature declaration, used for compatibilty checking. + /// The `VMCallerCheckedAnyfunc` for this exported function. /// - /// Note that this indexes within the module associated with `vmctx`. - pub signature: VMSharedSignatureIndex, + /// Note that exported functions cannot be a null funcref, so this is a + /// non-null pointer. + pub anyfunc: NonNull, } impl From for Export { diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 04c2bddbeb6c..a7bf80b6b376 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)); @@ -1121,6 +1124,7 @@ mod tests { num_imported_tables: 0, num_imported_memories: 0, num_imported_globals: 0, + num_defined_functions: 0, num_defined_tables: 0, num_defined_memories: 0, num_defined_globals: 0, @@ -1147,6 +1151,7 @@ mod tests { num_imported_tables: 0, num_imported_memories: 0, num_imported_globals: 0, + num_defined_functions: 0, num_defined_tables: 0, num_defined_memories: 0, num_defined_globals: 0, diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index aea72a0b289e..4395fefd467b 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -21,13 +21,15 @@ use std::any::Any; use std::cell::RefCell; use std::collections::HashMap; use std::convert::TryFrom; +use std::ptr::NonNull; use std::sync::Arc; use std::{mem, ptr, slice}; 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}; @@ -54,7 +56,7 @@ pub(crate) struct Instance { /// Passive elements in this instantiation. As `elem.drop`s happen, these /// entries get removed. A missing entry is considered equivalent to an /// empty slice. - passive_elements: RefCell>>, + passive_elements: RefCell>>, /// Passive data segments from our module. As `data.drop`s happen, entries /// get removed. A missing entry is considered equivalent to an empty slice. @@ -273,23 +275,10 @@ impl Instance { pub fn lookup_by_declaration(&self, export: &EntityIndex) -> Export { match export { EntityIndex::Function(index) => { - let signature = self.signature_id(self.module.local.functions[*index]); - let (address, vmctx) = - if let Some(def_index) = self.module.local.defined_func_index(*index) { - ( - self.finished_functions[def_index] as *const _, - self.vmctx_ptr(), - ) - } else { - let import = self.imported_function(*index); - (import.body, import.vmctx) - }; - ExportFunction { - address, - signature, - vmctx, - } - .into() + let anyfunc = self.get_caller_checked_anyfunc(*index).unwrap(); + let anyfunc = + NonNull::new(anyfunc as *const VMCallerCheckedAnyfunc as *mut _).unwrap(); + ExportFunction { anyfunc }.into() } EntityIndex::Table(index) => { let (definition, vmctx) = @@ -448,6 +437,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`. /// @@ -513,30 +507,25 @@ impl Instance { Layout::from_size_align(size, align).unwrap() } - /// Get a `VMCallerCheckedAnyfunc` for the given `FuncIndex`. - fn get_caller_checked_anyfunc(&self, index: FuncIndex) -> VMCallerCheckedAnyfunc { + /// Get a `&VMCallerCheckedAnyfunc` for the given `FuncIndex`. + /// + /// Returns `None` if the index is the reserved index value. + /// + /// The returned reference is a stable reference that won't be moved and can + /// be passed into JIT code. + pub(crate) fn get_caller_checked_anyfunc( + &self, + index: FuncIndex, + ) -> Option<&VMCallerCheckedAnyfunc> { if index == FuncIndex::reserved_value() { - return VMCallerCheckedAnyfunc::default(); + return None; } - let sig = self.module.local.functions[index]; - let type_index = self.signature_id(sig); + Some(unsafe { &*self.anyfunc_ptr(index) }) + } - let (func_ptr, vmctx) = if let Some(def_index) = self.module.local.defined_func_index(index) - { - ( - self.finished_functions[def_index] as *const _, - self.vmctx_ptr(), - ) - } else { - let import = self.imported_function(index); - (import.body, import.vmctx) - }; - VMCallerCheckedAnyfunc { - func_ptr, - type_index, - vmctx, - } + unsafe fn anyfunc_ptr(&self, index: FuncIndex) -> *mut VMCallerCheckedAnyfunc { + self.vmctx_plus_offset(self.offsets.vmctx_anyfunc(index)) } /// The `table.init` operation: initializes a portion of a table with a @@ -574,7 +563,7 @@ impl Instance { // TODO(#983): investigate replacing this get/set loop with a `memcpy`. for (dst, src) in (dst..dst + len).zip(src..src + len) { table - .set(dst, TableElement::FuncRef(elem[src as usize].clone())) + .set(dst, TableElement::FuncRef(elem[src as usize])) .expect("should never panic because we already did the bounds check above"); } @@ -932,6 +921,30 @@ impl InstanceHandle { *instance.externref_activations_table() = externref_activations_table; *instance.stack_map_registry() = stack_map_registry; + for (index, sig) in instance.module.local.functions.iter() { + let type_index = instance.signature_id(*sig); + + let (func_ptr, vmctx) = + if let Some(def_index) = instance.module.local.defined_func_index(index) { + ( + NonNull::new(instance.finished_functions[def_index] as *mut _).unwrap(), + instance.vmctx_ptr(), + ) + } else { + let import = instance.imported_function(index); + (import.body, import.vmctx) + }; + + ptr::write( + instance.anyfunc_ptr(index), + VMCallerCheckedAnyfunc { + func_ptr, + type_index, + vmctx, + }, + ); + } + // Perform infallible initialization in this constructor, while fallible // initialization is deferred to the `initialize` method. initialize_passive_elements(instance); @@ -1246,12 +1259,14 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { } for (i, func_idx) in init.elements.iter().enumerate() { - let anyfunc = instance.get_caller_checked_anyfunc(*func_idx); + let anyfunc = instance.get_caller_checked_anyfunc(*func_idx).map_or( + ptr::null_mut(), + |f: &VMCallerCheckedAnyfunc| { + f as *const VMCallerCheckedAnyfunc as *mut VMCallerCheckedAnyfunc + }, + ); table - .set( - u32::try_from(start + i).unwrap(), - TableElement::FuncRef(anyfunc), - ) + .set(u32::try_from(start + i).unwrap(), anyfunc.into()) .unwrap(); } } @@ -1280,7 +1295,14 @@ fn initialize_passive_elements(instance: &Instance) { *idx, segments .iter() - .map(|s| instance.get_caller_checked_anyfunc(*s)) + .map(|s| { + instance.get_caller_checked_anyfunc(*s).map_or( + ptr::null_mut(), + |f: &VMCallerCheckedAnyfunc| { + f as *const VMCallerCheckedAnyfunc as *mut _ + }, + ) + }) .collect(), ) }), diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index f0a8d842718d..c12021df2d0e 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -56,3 +56,14 @@ pub use crate::vmcontext::{ /// Version number of this crate. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); + +/// The Cranelift IR type used for reference types for this target architecture. +pub fn ref_type() -> wasmtime_environ::ir::Type { + if cfg!(target_pointer_width = "32") { + wasmtime_environ::ir::types::R32 + } else if cfg!(target_pointer_width = "64") { + wasmtime_environ::ir::types::R64 + } else { + unreachable!() + } +} diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 2b730e84787c..facdc8d7466f 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -57,10 +57,12 @@ //! ``` use crate::externref::VMExternRef; -use crate::table::{Table, TableElement}; +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 wasmtime_environ::wasm::{ + DataIndex, DefinedMemoryIndex, ElemIndex, MemoryIndex, TableElementType, TableIndex, +}; /// Implementation of f32.ceil pub extern "C" fn wasmtime_f32_ceil(x: f32) -> f32 { @@ -226,24 +228,38 @@ pub unsafe extern "C" fn wasmtime_imported_memory32_size( instance.imported_memory_size(memory_index) } -/// Implementation of `table.grow` for `externref`s. -pub unsafe extern "C" fn wasmtime_table_grow_extern_ref( +/// Implementation of `table.grow`. +pub unsafe extern "C" fn wasmtime_table_grow( vmctx: *mut VMContext, table_index: u32, delta: u32, + // NB: we don't know whether this is a pointer to a `VMCallerCheckedAnyfunc` + // or is a `VMExternRef` until we look at the table type. init_value: *mut u8, ) -> u32 { - let init_value = if init_value.is_null() { - None - } else { - Some(VMExternRef::clone_from_raw(init_value)) - }; - 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 = init_value as *mut VMCallerCheckedAnyfunc; + instance + .table_grow(table_index, delta, func.into()) + .unwrap_or(-1_i32 as u32) + } + TableElementType::Val(ty) => { + debug_assert_eq!(ty, crate::ref_type()); + + let init_value = if init_value.is_null() { + None + } else { + Some(VMExternRef::clone_from_raw(init_value)) + }; + + instance + .table_grow(table_index, delta, init_value.into()) + .unwrap_or(-1_i32 as u32) + } + } } /// Implementation of `table.copy`. diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index 4023e250738e..f3136d3f3203 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -6,6 +6,7 @@ use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition}; use crate::{Trap, VMExternRef}; use std::cell::RefCell; use std::convert::{TryFrom, TryInto}; +use std::ptr; use wasmtime_environ::wasm::TableElementType; use wasmtime_environ::{ir, TablePlan, TableStyle}; @@ -20,35 +21,28 @@ pub struct Table { #[derive(Clone, Debug)] pub enum TableElement { /// A `funcref`. - FuncRef(VMCallerCheckedAnyfunc), + FuncRef(*mut VMCallerCheckedAnyfunc), /// An `exrernref`. ExternRef(Option), } #[derive(Debug)] enum TableElements { - FuncRefs(Vec), + FuncRefs(Vec<*mut VMCallerCheckedAnyfunc>), ExternRefs(Vec>), } impl Table { /// Create a new table instance with specified minimum and maximum number of elements. pub fn new(plan: &TablePlan) -> Self { - let elements = - RefCell::new(match plan.table.ty { - TableElementType::Func => TableElements::FuncRefs(vec![ - VMCallerCheckedAnyfunc::default(); - usize::try_from(plan.table.minimum).unwrap() - ]), - TableElementType::Val(ty) - if (cfg!(target_pointer_width = "64") && ty == ir::types::R64) - || (cfg!(target_pointer_width = "32") && ty == ir::types::R32) => - { - let min = usize::try_from(plan.table.minimum).unwrap(); - TableElements::ExternRefs(vec![None; min]) - } - TableElementType::Val(ty) => unimplemented!("unsupported table type ({})", ty), - }); + let min = usize::try_from(plan.table.minimum).unwrap(); + let elements = RefCell::new(match plan.table.ty { + TableElementType::Func => TableElements::FuncRefs(vec![ptr::null_mut(); min]), + TableElementType::Val(ty) => { + debug_assert_eq!(ty, crate::ref_type()); + TableElements::ExternRefs(vec![None; min]) + } + }); match plan.style { TableStyle::CallerChecksSignature => Self { elements, @@ -57,6 +51,14 @@ 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, + TableElements::ExternRefs(_) => TableElementType::Val(crate::ref_type()), + } + } + /// Returns the number of allocated elements. pub fn size(&self) -> u32 { match &*self.elements.borrow() { @@ -199,7 +201,7 @@ impl Table { } } -impl TryFrom for VMCallerCheckedAnyfunc { +impl TryFrom for *mut VMCallerCheckedAnyfunc { type Error = TableElement; fn try_from(e: TableElement) -> Result { @@ -221,8 +223,8 @@ impl TryFrom for Option { } } -impl From for TableElement { - fn from(f: VMCallerCheckedAnyfunc) -> TableElement { +impl From<*mut VMCallerCheckedAnyfunc> for TableElement { + fn from(f: *mut VMCallerCheckedAnyfunc) -> TableElement { TableElement::FuncRef(f) } } diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index e9f681926cb6..d6df7ab75c15 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -3,8 +3,9 @@ use crate::instance::Instance; use std::any::Any; +use std::ptr::NonNull; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; -use std::{ptr, u32}; +use std::u32; use wasmtime_environ::BuiltinFunctionIndex; /// An imported function. @@ -12,7 +13,7 @@ use wasmtime_environ::BuiltinFunctionIndex; #[repr(C)] pub struct VMFunctionImport { /// A pointer to the imported function body. - pub body: *const VMFunctionBody, + pub body: NonNull, /// A pointer to the `VMContext` that owns the function. pub vmctx: *mut VMContext, @@ -475,7 +476,7 @@ impl Default for VMSharedSignatureIndex { #[repr(C)] pub struct VMCallerCheckedAnyfunc { /// Function body. - pub func_ptr: *const VMFunctionBody, + pub func_ptr: NonNull, /// Function signature id. pub type_index: VMSharedSignatureIndex, /// Function `VMContext`. @@ -513,16 +514,6 @@ mod test_vmcaller_checked_anyfunc { } } -impl Default for VMCallerCheckedAnyfunc { - fn default() -> Self { - Self { - func_ptr: ptr::null_mut(), - type_index: Default::default(), - vmctx: ptr::null_mut(), - } - } -} - /// An array that stores addresses of builtin functions. We translate code /// to use indirect calls. This way, we don't have to patch the code. #[repr(C)] @@ -549,8 +540,10 @@ 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_funcref().index() as usize] = + wasmtime_table_grow as usize; + ptrs[BuiltinFunctionIndex::table_grow_externref().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] = diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 29faa28dd2a2..cbcaa542f1f0 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -205,7 +205,7 @@ impl Global { /// Returns the value type of this `global`. pub fn val_type(&self) -> ValType { - ValType::from_wasmtime_type(self.wasmtime_export.global.ty) + ValType::from_wasm_type(&self.wasmtime_export.global.wasm_ty) .expect("core wasm type should be supported") } @@ -298,14 +298,10 @@ fn set_table_item( instance: &InstanceHandle, table_index: wasm::DefinedTableIndex, item_index: u32, - item: wasmtime_runtime::VMCallerCheckedAnyfunc, + item: *mut wasmtime_runtime::VMCallerCheckedAnyfunc, ) -> Result<()> { instance - .table_set( - table_index, - item_index, - runtime::TableElement::FuncRef(item), - ) + .table_set(table_index, item_index, item.into()) .map_err(|()| anyhow!("table element index out of bounds")) } @@ -329,7 +325,7 @@ impl Table { let definition = unsafe { &*wasmtime_export.definition }; let index = instance.table_index(definition); for i in 0..definition.current_elements { - set_table_item(&instance, index, i, item.clone())?; + set_table_item(&instance, index, i, item)?; } Ok(Table { @@ -356,7 +352,7 @@ impl Table { let item = self.instance.table_get(table_index, index)?; match item { runtime::TableElement::FuncRef(f) => { - Some(from_checked_anyfunc(f, &self.instance.store)) + Some(unsafe { from_checked_anyfunc(f, &self.instance.store) }) } runtime::TableElement::ExternRef(None) => Some(Val::ExternRef(None)), runtime::TableElement::ExternRef(Some(x)) => Some(Val::ExternRef(Some(ExternRef { @@ -398,8 +394,7 @@ impl Table { let orig_size = match self.ty().element() { ValType::FuncRef => { let init = into_checked_anyfunc(init, &self.instance.store)?; - self.instance - .defined_table_grow(index, delta, runtime::TableElement::FuncRef(init)) + self.instance.defined_table_grow(index, delta, init.into()) } ValType::ExternRef => { let init = match init { diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 53c7e5ab01d7..9c2081846f2d 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -6,10 +6,11 @@ use std::cmp::max; use std::fmt; use std::mem; use std::panic::{self, AssertUnwindSafe}; -use std::ptr; +use std::ptr::{self, NonNull}; use std::rc::Weak; -use wasmtime_runtime::{raise_user_trap, ExportFunction, VMTrampoline}; -use wasmtime_runtime::{Export, InstanceHandle, VMContext, VMFunctionBody}; +use wasmtime_runtime::{ + raise_user_trap, Export, InstanceHandle, VMContext, VMFunctionBody, VMTrampoline, +}; /// A WebAssembly function which can be called. /// @@ -140,8 +141,8 @@ use wasmtime_runtime::{Export, InstanceHandle, VMContext, VMFunctionBody}; #[derive(Clone)] pub struct Func { instance: StoreInstanceHandle, - export: ExportFunction, trampoline: VMTrampoline, + export: wasmtime_runtime::ExportFunction, } macro_rules! getters { @@ -175,10 +176,11 @@ macro_rules! getters { .context("Type mismatch in return type")?; ensure!(results.next().is_none(), "Type mismatch: too many return values (expected 1)"); - // Pass the instance into the closure so that we keep it live for the lifetime - // of the closure. Pass the export in so that we can call it. + // Pass the instance into the closure so that we keep it live for + // the lifetime of the closure. Pass the `anyfunc` in so that we can + // call it. let instance = self.instance.clone(); - let export = self.export.clone(); + let anyfunc = self.export.anyfunc; // ... and then once we've passed the typechecks we can hand out our // object since our `transmute` below should be safe! @@ -191,12 +193,12 @@ macro_rules! getters { *mut VMContext, $($args,)* ) -> R, - >(export.address); + >(anyfunc.as_ref().func_ptr.as_ptr()); let mut ret = None; $(let $args = $args.into_abi();)* - invoke_wasm_and_catch_traps(export.vmctx, &instance.store, || { - ret = Some(fnptr(export.vmctx, ptr::null_mut(), $($args,)*)); + invoke_wasm_and_catch_traps(anyfunc.as_ref().vmctx, &instance.store, || { + ret = Some(fnptr(anyfunc.as_ref().vmctx, ptr::null_mut(), $($args,)*)); })?; Ok(ret.unwrap()) @@ -282,8 +284,8 @@ impl Func { crate::trampoline::generate_func_export(&ty, func, store).expect("generated func"); Func { instance, - export, trampoline, + export, } } @@ -488,7 +490,10 @@ impl Func { pub fn ty(&self) -> FuncType { // Signatures should always be registered in the store's registry of // shared signatures, so we should be able to unwrap safely here. - let sig = self.instance.store.lookup_signature(self.export.signature); + let sig = self + .instance + .store + .lookup_signature(unsafe { self.export.anyfunc.as_ref().type_index }); // This is only called with `Export::Function`, and since it's coming // from wasmtime_runtime itself we should support all the types coming @@ -498,13 +503,19 @@ impl Func { /// Returns the number of parameters that this function takes. pub fn param_arity(&self) -> usize { - let sig = self.instance.store.lookup_signature(self.export.signature); + let sig = self + .instance + .store + .lookup_signature(unsafe { self.export.anyfunc.as_ref().type_index }); sig.params.len() } /// Returns the number of results this function produces. pub fn result_arity(&self) -> usize { - let sig = self.instance.store.lookup_signature(self.export.signature); + let sig = self + .instance + .store + .lookup_signature(unsafe { self.export.anyfunc.as_ref().type_index }); sig.returns.len() } @@ -553,14 +564,17 @@ impl Func { } // Call the trampoline. - invoke_wasm_and_catch_traps(self.export.vmctx, &self.instance.store, || unsafe { - (self.trampoline)( - self.export.vmctx, - ptr::null_mut(), - self.export.address, - values_vec.as_mut_ptr(), - ) - })?; + unsafe { + let anyfunc = self.export.anyfunc.as_ref(); + invoke_wasm_and_catch_traps(anyfunc.vmctx, &self.instance.store, || { + (self.trampoline)( + anyfunc.vmctx, + ptr::null_mut(), + anyfunc.func_ptr.as_ptr(), + values_vec.as_mut_ptr(), + ) + })?; + } // Load the return values out of `values_vec`. let mut results = Vec::with_capacity(my_ty.results().len()); @@ -578,6 +592,12 @@ impl Func { &self.export } + pub(crate) fn caller_checked_anyfunc( + &self, + ) -> NonNull { + self.export.anyfunc + } + pub(crate) fn from_wasmtime_function( export: wasmtime_runtime::ExportFunction, instance: StoreInstanceHandle, @@ -586,7 +606,7 @@ impl Func { // on that module as well, so unwrap the result here since otherwise // it's a bug in wasmtime. let trampoline = instance - .trampoline(export.signature) + .trampoline(unsafe { export.anyfunc.as_ref().type_index }) .expect("failed to retrieve trampoline from module"); Func { diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 1477c0a0ba59..da572483067d 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -5,7 +5,9 @@ use std::any::Any; use std::mem; use wasmtime_environ::EntityIndex; use wasmtime_jit::{CompiledModule, Resolver}; -use wasmtime_runtime::{InstantiationError, VMContext, VMFunctionBody}; +use wasmtime_runtime::{ + InstantiationError, StackMapRegistry, VMContext, VMExternRefActivationsTable, VMFunctionBody, +}; struct SimpleResolver<'a> { imports: &'a [Extern], @@ -50,8 +52,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 VMExternRefActivationsTable as *mut _, + &**store.stack_map_registry() as *const StackMapRegistry as *mut _, )?; // After we've created the `InstanceHandle` we still need to run @@ -95,7 +97,9 @@ fn instantiate( mem::transmute::< *const VMFunctionBody, unsafe extern "C" fn(*mut VMContext, *mut VMContext), - >(f.address)(f.vmctx, vmctx_ptr) + >(f.anyfunc.as_ref().func_ptr.as_ptr())( + f.anyfunc.as_ref().vmctx, vmctx_ptr + ) })?; } } diff --git a/crates/wasmtime/src/linker.rs b/crates/wasmtime/src/linker.rs index dcf4bddb3c49..43332216eaeb 100644 --- a/crates/wasmtime/src/linker.rs +++ b/crates/wasmtime/src/linker.rs @@ -427,7 +427,7 @@ impl Linker { Ok(()) }); - self.insert(module_name, export.name(), Extern::Func(func))?; + self.insert(module_name, export.name(), func.into())?; } else if export.name() == "memory" && export.ty().memory().is_some() { // Allow an exported "memory" memory for now. } else if export.name() == "__indirect_function_table" && export.ty().table().is_some() diff --git a/crates/wasmtime/src/trampoline/create_handle.rs b/crates/wasmtime/src/trampoline/create_handle.rs index 12faef120e10..db4d47790593 100644 --- a/crates/wasmtime/src/trampoline/create_handle.rs +++ b/crates/wasmtime/src/trampoline/create_handle.rs @@ -10,7 +10,8 @@ use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::wasm::DefinedFuncIndex; use wasmtime_environ::Module; use wasmtime_runtime::{ - Imports, InstanceHandle, VMFunctionBody, VMSharedSignatureIndex, VMTrampoline, + Imports, InstanceHandle, StackMapRegistry, VMExternRefActivationsTable, VMFunctionBody, + VMSharedSignatureIndex, VMTrampoline, }; pub(crate) fn create_handle( @@ -46,8 +47,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 VMExternRefActivationsTable as *mut _, + &**store.stack_map_registry() as *const StackMapRegistry as *mut _, )?; Ok(store.add_instance(handle)) } diff --git a/crates/wasmtime/src/trampoline/global.rs b/crates/wasmtime/src/trampoline/global.rs index 4fa13e42bdf0..db09d4b81eef 100644 --- a/crates/wasmtime/src/trampoline/global.rs +++ b/crates/wasmtime/src/trampoline/global.rs @@ -1,13 +1,13 @@ use super::create_handle::create_handle; use crate::trampoline::StoreInstanceHandle; -use crate::Store; -use crate::{GlobalType, Mutability, Val}; +use crate::{GlobalType, Mutability, Store, Val}; use anyhow::{bail, Result}; use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::{wasm, EntityIndex, Module}; pub fn create_global(store: &Store, gt: &GlobalType, val: Val) -> Result { let global = wasm::Global { + wasm_ty: gt.content().to_wasm_type(), ty: match gt.content().get_wasmtime_type() { Some(t) => t, None => bail!("cannot support {:?} as a wasm global type", gt.content()), diff --git a/crates/wasmtime/src/trampoline/table.rs b/crates/wasmtime/src/trampoline/table.rs index 4919e0e09c24..a3d26a778726 100644 --- a/crates/wasmtime/src/trampoline/table.rs +++ b/crates/wasmtime/src/trampoline/table.rs @@ -10,6 +10,7 @@ pub fn create_handle_with_table(store: &Store, table: &TableType) -> Result Some(ir::types::F32), ValType::F64 => Some(ir::types::F64), ValType::V128 => Some(ir::types::I8X16), - #[cfg(target_pointer_width = "64")] - ValType::ExternRef => Some(ir::types::R64), - #[cfg(target_pointer_width = "32")] - ValType::ExternRef => Some(ir::types::R32), - _ => None, - } - } - - pub(crate) fn from_wasmtime_type(ty: ir::Type) -> Option { - match ty { - ir::types::I32 => Some(ValType::I32), - ir::types::I64 => Some(ValType::I64), - ir::types::F32 => Some(ValType::F32), - ir::types::F64 => Some(ValType::F64), - ir::types::I8X16 => Some(ValType::V128), - #[cfg(target_pointer_width = "64")] - ir::types::R64 => Some(ValType::ExternRef), - #[cfg(target_pointer_width = "32")] - ir::types::R32 => Some(ValType::ExternRef), + ValType::ExternRef => Some(wasmtime_runtime::ref_type()), _ => None, } } @@ -353,7 +335,7 @@ impl GlobalType { /// Returns `None` if the wasmtime global has a type that we can't /// represent, but that should only very rarely happen and indicate a bug. pub(crate) fn from_wasmtime_global(global: &wasm::Global) -> Option { - let ty = ValType::from_wasmtime_type(global.ty)?; + let ty = ValType::from_wasm_type(&global.wasm_ty)?; let mutability = if global.mutability { Mutability::Var } else { diff --git a/crates/wasmtime/src/values.rs b/crates/wasmtime/src/values.rs index 7885e26c5ea2..8ab44857325c 100644 --- a/crates/wasmtime/src/values.rs +++ b/crates/wasmtime/src/values.rs @@ -1,8 +1,8 @@ use crate::r#ref::ExternRef; use crate::{Func, Store, ValType}; use anyhow::{bail, Result}; -use std::ptr; -use wasmtime_runtime::VMExternRef; +use std::ptr::{self, NonNull}; +use wasmtime_runtime::{self as runtime, VMExternRef}; /// Possible runtime values that a WebAssembly module can either consume or /// produce. @@ -26,11 +26,18 @@ pub enum Val { /// `f64::from_bits` to create an `f64` value. F64(u64), - /// An `externref` value which can hold opaque data to the wasm instance itself. + /// An `externref` value which can hold opaque data to the Wasm instance + /// itself. + /// + /// `ExternRef(None)` is the null external reference, created by `ref.null + /// extern` in Wasm. ExternRef(Option), /// A first-class reference to a WebAssembly function. - FuncRef(Func), + /// + /// `FuncRef(None)` is the null function reference, created by `ref.null + /// func` in Wasm. + FuncRef(Option), /// A 128-bit number V128(u128), @@ -94,7 +101,14 @@ impl Val { .insert_with_gc(x.inner, store.stack_map_registry()); ptr::write(p as *mut *mut u8, externref_ptr) } - _ => unimplemented!("Val::write_value_to"), + Val::FuncRef(f) => ptr::write( + p as *mut *mut runtime::VMCallerCheckedAnyfunc, + if let Some(f) = f { + f.caller_checked_anyfunc().as_ptr() + } else { + ptr::null_mut() + }, + ), } } @@ -116,7 +130,10 @@ impl Val { })) } } - _ => unimplemented!("Val::read_value_from: {:?}", ty), + ValType::FuncRef => { + let func = ptr::read(p as *const *mut runtime::VMCallerCheckedAnyfunc); + from_checked_anyfunc(func, store) + } } } @@ -126,7 +143,7 @@ impl Val { (I64(i64) i64 unwrap_i64 *e) (F32(f32) f32 unwrap_f32 f32::from_bits(*e)) (F64(f64) f64 unwrap_f64 f64::from_bits(*e)) - (FuncRef(&Func) funcref unwrap_funcref e) + (FuncRef(Option<&Func>) funcref unwrap_funcref e.as_ref()) (V128(u128) v128 unwrap_v128 *e) } @@ -160,7 +177,8 @@ impl Val { pub(crate) fn comes_from_same_store(&self, store: &Store) -> bool { match self { - Val::FuncRef(f) => Store::same(store, f.store()), + Val::FuncRef(Some(f)) => Store::same(store, f.store()), + Val::FuncRef(None) => true, // TODO: need to implement this once we actually finalize what // `externref` will look like and it's actually implemented to pass it @@ -211,51 +229,47 @@ impl From> for Val { } } +impl From> for Val { + fn from(val: Option) -> Val { + Val::FuncRef(val) + } +} + impl From for Val { fn from(val: Func) -> Val { - Val::FuncRef(val) + Val::FuncRef(Some(val)) } } pub(crate) fn into_checked_anyfunc( val: Val, store: &Store, -) -> Result { +) -> Result<*mut wasmtime_runtime::VMCallerCheckedAnyfunc> { if !val.comes_from_same_store(store) { bail!("cross-`Store` values are not supported"); } Ok(match val { - Val::ExternRef(None) => wasmtime_runtime::VMCallerCheckedAnyfunc { - func_ptr: ptr::null(), - type_index: wasmtime_runtime::VMSharedSignatureIndex::default(), - vmctx: ptr::null_mut(), - }, - Val::FuncRef(f) => { - let f = f.wasmtime_function(); - wasmtime_runtime::VMCallerCheckedAnyfunc { - func_ptr: f.address, - type_index: f.signature, - vmctx: f.vmctx, - } - } + Val::FuncRef(None) => ptr::null_mut(), + Val::FuncRef(Some(f)) => f.caller_checked_anyfunc().as_ptr(), _ => bail!("val is not funcref"), }) } -pub(crate) fn from_checked_anyfunc( - item: wasmtime_runtime::VMCallerCheckedAnyfunc, +pub(crate) unsafe fn from_checked_anyfunc( + anyfunc: *mut wasmtime_runtime::VMCallerCheckedAnyfunc, store: &Store, ) -> Val { - if item.type_index == wasmtime_runtime::VMSharedSignatureIndex::default() { - return Val::ExternRef(None); - } - let instance_handle = unsafe { wasmtime_runtime::InstanceHandle::from_vmctx(item.vmctx) }; - let export = wasmtime_runtime::ExportFunction { - address: item.func_ptr, - signature: item.type_index, - vmctx: item.vmctx, + let anyfunc = match NonNull::new(anyfunc) { + None => return Val::FuncRef(None), + Some(f) => f, }; + + debug_assert!( + anyfunc.as_ref().type_index != wasmtime_runtime::VMSharedSignatureIndex::default() + ); + let instance_handle = wasmtime_runtime::InstanceHandle::from_vmctx(anyfunc.as_ref().vmctx); + let export = wasmtime_runtime::ExportFunction { anyfunc }; let instance = store.existing_instance_handle(instance_handle); let f = Func::from_wasmtime_function(export, instance); - Val::FuncRef(f) + Val::FuncRef(Some(f)) } diff --git a/crates/wast/src/spectest.rs b/crates/wast/src/spectest.rs index f49125dbe8cb..ef744fa42d50 100644 --- a/crates/wast/src/spectest.rs +++ b/crates/wast/src/spectest.rs @@ -35,7 +35,7 @@ pub fn link_spectest(linker: &mut Linker) -> Result<()> { linker.define("spectest", "global_f64", g)?; let ty = TableType::new(ValType::FuncRef, Limits::new(10, Some(20))); - let table = Table::new(linker.store(), ty, Val::ExternRef(None))?; + let table = Table::new(linker.store(), ty, Val::FuncRef(None))?; linker.define("spectest", "table", table)?; let ty = MemoryType::new(Limits::new(1, Some(2))); diff --git a/tests/all/externals.rs b/tests/all/externals.rs index 05ef48c63271..5d842abbfd59 100644 --- a/tests/all/externals.rs +++ b/tests/all/externals.rs @@ -28,27 +28,27 @@ fn bad_tables() { // get out of bounds let ty = TableType::new(ValType::FuncRef, Limits::new(0, Some(1))); - let t = Table::new(&Store::default(), ty.clone(), Val::ExternRef(None)).unwrap(); + let t = Table::new(&Store::default(), ty.clone(), Val::FuncRef(None)).unwrap(); assert!(t.get(0).is_none()); assert!(t.get(u32::max_value()).is_none()); // set out of bounds or wrong type let ty = TableType::new(ValType::FuncRef, Limits::new(1, Some(1))); - let t = Table::new(&Store::default(), ty.clone(), Val::ExternRef(None)).unwrap(); + let t = Table::new(&Store::default(), ty.clone(), Val::FuncRef(None)).unwrap(); assert!(t.set(0, Val::I32(0)).is_err()); - assert!(t.set(0, Val::ExternRef(None)).is_ok()); - assert!(t.set(1, Val::ExternRef(None)).is_err()); + assert!(t.set(0, Val::FuncRef(None)).is_ok()); + assert!(t.set(1, Val::FuncRef(None)).is_err()); // grow beyond max let ty = TableType::new(ValType::FuncRef, Limits::new(1, Some(1))); - let t = Table::new(&Store::default(), ty.clone(), Val::ExternRef(None)).unwrap(); - assert!(t.grow(0, Val::ExternRef(None)).is_ok()); - assert!(t.grow(1, Val::ExternRef(None)).is_err()); + let t = Table::new(&Store::default(), ty.clone(), Val::FuncRef(None)).unwrap(); + assert!(t.grow(0, Val::FuncRef(None)).is_ok()); + assert!(t.grow(1, Val::FuncRef(None)).is_err()); assert_eq!(t.size(), 1); // grow wrong type let ty = TableType::new(ValType::FuncRef, Limits::new(1, Some(2))); - let t = Table::new(&Store::default(), ty.clone(), Val::ExternRef(None)).unwrap(); + let t = Table::new(&Store::default(), ty.clone(), Val::FuncRef(None)).unwrap(); assert!(t.grow(1, Val::I32(0)).is_err()); assert_eq!(t.size(), 1); } @@ -69,7 +69,7 @@ fn cross_store() -> anyhow::Result<()> { let ty = MemoryType::new(Limits::new(1, None)); let memory = Memory::new(&store2, ty); let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); - let table = Table::new(&store2, ty, Val::ExternRef(None))?; + let table = Table::new(&store2, ty, Val::FuncRef(None))?; let need_func = Module::new(&engine, r#"(module (import "" "" (func)))"#)?; assert!(Instance::new(&store1, &need_func, &[func.into()]).is_err()); @@ -85,8 +85,8 @@ fn cross_store() -> anyhow::Result<()> { // ============ Cross-store globals ============== - let store1val = Val::FuncRef(Func::wrap(&store1, || {})); - let store2val = Val::FuncRef(Func::wrap(&store2, || {})); + let store1val = Val::FuncRef(Some(Func::wrap(&store1, || {}))); + let store2val = Val::FuncRef(Some(Func::wrap(&store2, || {}))); let ty = GlobalType::new(ValType::FuncRef, Mutability::Var); assert!(Global::new(&store2, ty.clone(), store1val.clone()).is_err()); diff --git a/tests/all/funcref.rs b/tests/all/funcref.rs new file mode 100644 index 000000000000..940d42bcb0a6 --- /dev/null +++ b/tests/all/funcref.rs @@ -0,0 +1,85 @@ +use super::ref_types_module; +use wasmtime::*; + +#[test] +fn pass_funcref_in_and_out_of_wasm() -> anyhow::Result<()> { + let (store, module) = ref_types_module( + r#" + (module + (func (export "func") (param funcref) (result funcref) + local.get 0 + ) + ) + "#, + )?; + + let instance = Instance::new(&store, &module, &[])?; + let func = instance.get_func("func").unwrap(); + + // Pass in a non-null funcref. + { + let results = func.call(&[Val::FuncRef(Some(func.clone()))])?; + assert_eq!(results.len(), 1); + + // Can't compare `Func` for equality, so this is the best we can do here. + let result_func = results[0].unwrap_funcref().unwrap(); + assert_eq!(func.ty(), result_func.ty()); + } + + // Pass in a null funcref. + { + let results = func.call(&[Val::FuncRef(None)])?; + assert_eq!(results.len(), 1); + + let result_func = results[0].unwrap_funcref(); + assert!(result_func.is_none()); + } + + // Pass in a `funcref` from another instance. + { + let other_instance = Instance::new(&store, &module, &[])?; + let other_instance_func = other_instance.get_func("func").unwrap(); + + let results = func.call(&[Val::FuncRef(Some(other_instance_func.clone()))])?; + assert_eq!(results.len(), 1); + + // Can't compare `Func` for equality, so this is the best we can do here. + let result_func = results[0].unwrap_funcref().unwrap(); + assert_eq!(other_instance_func.ty(), result_func.ty()); + } + + // Passing in a `funcref` from another store fails. + { + let (other_store, other_module) = ref_types_module(r#"(module (func (export "f")))"#)?; + let other_store_instance = Instance::new(&other_store, &other_module, &[])?; + let f = other_store_instance.get_func("f").unwrap(); + + assert!(func.call(&[Val::FuncRef(Some(f))]).is_err()); + } + + Ok(()) +} + +#[test] +fn receive_null_funcref_from_wasm() -> anyhow::Result<()> { + let (store, module) = ref_types_module( + r#" + (module + (func (export "get-null") (result funcref) + ref.null func + ) + ) + "#, + )?; + + let instance = Instance::new(&store, &module, &[])?; + let get_null = instance.get_func("get-null").unwrap(); + + let results = get_null.call(&[])?; + assert_eq!(results.len(), 1); + + let result_func = results[0].unwrap_funcref(); + assert!(result_func.is_none()); + + Ok(()) +} diff --git a/tests/all/gc.rs b/tests/all/gc.rs index e6950ccbbe68..78ff8dc8e3c5 100644 --- a/tests/all/gc.rs +++ b/tests/all/gc.rs @@ -1,21 +1,8 @@ +use super::ref_types_module; use std::cell::Cell; use std::rc::Rc; use wasmtime::*; -fn ref_types_module(source: &str) -> anyhow::Result<(Store, Module)> { - let _ = env_logger::try_init(); - - let mut config = Config::new(); - config.wasm_reference_types(true); - - let engine = Engine::new(&config); - let store = Store::new(&engine); - - let module = Module::new(&engine, source)?; - - Ok((store, module)) -} - #[test] fn smoke_test_gc() -> anyhow::Result<()> { let (store, module) = ref_types_module( diff --git a/tests/all/invoke_func_via_table.rs b/tests/all/invoke_func_via_table.rs index a73f7c1c404d..cc77a028c7d0 100644 --- a/tests/all/invoke_func_via_table.rs +++ b/tests/all/invoke_func_via_table.rs @@ -23,6 +23,7 @@ fn test_invoke_func_via_table() -> Result<()> { .unwrap() .funcref() .unwrap() + .unwrap() .clone(); let result = f.call(&[]).unwrap(); assert_eq!(result[0].unwrap_i64(), 42); diff --git a/tests/all/linker.rs b/tests/all/linker.rs index 0db288aedacc..675c109cf18c 100644 --- a/tests/all/linker.rs +++ b/tests/all/linker.rs @@ -59,11 +59,11 @@ fn link_twice_bad() -> Result<()> { // tables let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); - let table = Table::new(&store, ty, Val::ExternRef(None))?; + let table = Table::new(&store, ty, Val::FuncRef(None))?; linker.define("", "", table.clone())?; assert!(linker.define("", "", table.clone()).is_err()); let ty = TableType::new(ValType::FuncRef, Limits::new(2, None)); - let table = Table::new(&store, ty, Val::ExternRef(None))?; + let table = Table::new(&store, ty, Val::FuncRef(None))?; assert!(linker.define("", "", table.clone()).is_err()); Ok(()) } diff --git a/tests/all/main.rs b/tests/all/main.rs index e278d0750435..e8f952ff7436 100644 --- a/tests/all/main.rs +++ b/tests/all/main.rs @@ -21,4 +21,26 @@ mod wast; // TODO(#1886): Cranelift only supports reference types on x64. #[cfg(target_arch = "x86_64")] +mod funcref; +#[cfg(target_arch = "x86_64")] mod gc; + +/// A helper to compile a module in a new store with reference types enabled. +#[cfg(target_arch = "x86_64")] +pub(crate) fn ref_types_module( + source: &str, +) -> anyhow::Result<(wasmtime::Store, wasmtime::Module)> { + use wasmtime::*; + + let _ = env_logger::try_init(); + + let mut config = Config::new(); + config.wasm_reference_types(true); + + let engine = Engine::new(&config); + let store = Store::new(&engine); + + let module = Module::new(&engine, source)?; + + Ok((store, module)) +} diff --git a/tests/all/table.rs b/tests/all/table.rs index 9bf7cb4cc484..26830c4ea9bf 100644 --- a/tests/all/table.rs +++ b/tests/all/table.rs @@ -4,9 +4,9 @@ use wasmtime::*; fn get_none() { let store = Store::default(); let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); - let table = Table::new(&store, ty, Val::ExternRef(None)).unwrap(); + let table = Table::new(&store, ty, Val::FuncRef(None)).unwrap(); match table.get(0) { - Some(Val::ExternRef(None)) => {} + Some(Val::FuncRef(None)) => {} _ => panic!(), } assert!(table.get(1).is_none()); diff --git a/tests/all/use_after_drop.rs b/tests/all/use_after_drop.rs index 625bac897fed..293dacafd023 100644 --- a/tests/all/use_after_drop.rs +++ b/tests/all/use_after_drop.rs @@ -11,10 +11,10 @@ fn use_func_after_drop() -> Result<()> { assert_eq!(closed_over_data, "abcd"); }); let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); - table = Table::new(&store, ty, Val::ExternRef(None))?; + table = Table::new(&store, ty, Val::FuncRef(None))?; table.set(0, func.into())?; } - let func = table.get(0).unwrap().funcref().unwrap().clone(); + let func = table.get(0).unwrap().funcref().unwrap().unwrap().clone(); let func = func.get0::<()>()?; func()?; Ok(()) 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))