Skip to content

Commit

Permalink
wasmtime: Implement funcref with NonNull<VMCallerCheckedAnyfunc>
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fitzgen committed Jun 23, 2020
1 parent 632c3cf commit b9d223c
Show file tree
Hide file tree
Showing 32 changed files with 431 additions and 404 deletions.
2 changes: 2 additions & 0 deletions cranelift/wasm/src/sections_translator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -227,6 +228,7 @@ pub fn parse_global_section(
}
};
let global = Global {
wasm_ty: content_type,
ty: type_to_type(content_type, environ).unwrap(),
mutability: mutable,
initializer,
Expand Down
12 changes: 10 additions & 2 deletions cranelift/wasm/src/translation_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
24 changes: 12 additions & 12 deletions crates/c-api/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub extern "C" fn wasm_table_new(
) -> Option<Box<wasm_table_t>> {
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 {
Expand All @@ -60,8 +60,8 @@ pub extern "C" fn wasmtime_funcref_table_new(
out: &mut *mut wasm_table_t,
) -> Option<Box<wasmtime_error_t>> {
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),
Expand All @@ -85,7 +85,7 @@ pub extern "C" fn wasm_table_type(t: &wasm_table_t) -> Box<wasm_tabletype_t> {
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)),
}
}

Expand All @@ -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,
};
}
Expand All @@ -133,14 +133,14 @@ pub extern "C" fn wasmtime_funcref_table_set(
val: Option<&wasm_func_t>,
) -> Option<Box<wasmtime_error_t>> {
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() {
Expand All @@ -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)
}
}

Expand All @@ -182,8 +182,8 @@ pub extern "C" fn wasmtime_funcref_table_grow(
prev_size: Option<&mut wasm_table_size_t>,
) -> Option<Box<wasmtime_error_t>> {
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 {
Expand Down
71 changes: 44 additions & 27 deletions crates/environ/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, TableIndex,
TargetEnvironment, WasmError, WasmResult,
self, FuncIndex, GlobalIndex, GlobalVariable, MemoryIndex, SignatureIndex, TableElementType,
TableIndex, TargetEnvironment, WasmError, WasmResult,
};
#[cfg(feature = "lightbeam")]
use cranelift_wasm::{DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex};
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -161,10 +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.
table_grow(vmctx, i32, i32, reference) -> (i32);
/// Returns an index for Wasm's `ref.func` instruction.
ref_func(vmctx, i32) -> (reference);
/// 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_externref(vmctx, i32, i32, reference) -> (i32);
}

impl BuiltinFunctionIndex {
Expand Down Expand Up @@ -586,9 +590,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 {
Expand All @@ -607,12 +609,25 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
delta: ir::Value,
init_value: ir::Value,
) -> WasmResult<ir::Value> {
let table_index_arg = pos.ins().iconst(I32, table_index.as_u32() as i64);
let (func_idx, func_sig) = match self.module.table_plans[table_index].table.ty {
TableElementType::Func => (
BuiltinFunctionIndex::table_grow_funcref(),
self.builtin_function_signatures
.table_grow_funcref(&mut pos.func),
),
TableElementType::Val(ty) => {
debug_assert_eq!(ty, self.reference_type());
(
BuiltinFunctionIndex::table_grow_externref(),
self.builtin_function_signatures
.table_grow_externref(&mut pos.func),
)
}
};

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 table_index_arg = pos.ins().iconst(I32, table_index.as_u32() as i64);
let call_inst = pos.ins().call_indirect(
func_sig,
func_addr,
Expand Down Expand Up @@ -663,17 +678,10 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
mut pos: cranelift_codegen::cursor::FuncCursor<'_>,
func_index: FuncIndex,
) -> WasmResult<ir::Value> {
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))
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(
Expand Down Expand Up @@ -843,12 +851,21 @@ 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);
// Note: `anyfunc_ptr` is always non-null, even for null func refs! In
// that case, the pointed-to `VMCallerCheckedAnyfunc`'s `func_ptr`
// member is null, which is checked below.

// 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()),
);

Expand All @@ -875,7 +892,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()),
);

Expand All @@ -892,7 +909,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);
Expand Down
34 changes: 32 additions & 2 deletions crates/environ/src/vmoffsets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
// }

Expand Down Expand Up @@ -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.
Expand All @@ -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()),
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion crates/fuzzing/src/oracles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ pub fn differential_execution(

for (name, f) in instance.exports().filter_map(|e| {
let name = e.name();
e.into_func().map(|f| (name, f))
e.into_func().and_then(|f| f).map(|f| (name, f))
}) {
// Always call the hang limit initializer first, so that we don't
// infinite loop when calling another export.
Expand Down
14 changes: 10 additions & 4 deletions crates/jit/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(_)) => {
Expand Down Expand Up @@ -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(
Expand Down
Loading

0 comments on commit b9d223c

Please sign in to comment.