Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/component/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ impl Func {
def.clone()
};
match self.instance.lookup_vmdef(store, &def) {
Export::Function(f) => f.func_ref,
Export::Function(f) => f.vm_func_ref(store),
_ => unreachable!(),
}
}
Expand Down
27 changes: 14 additions & 13 deletions crates/wasmtime/src/runtime/component/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::prelude::*;
use crate::runtime::vm::component::{
CallContexts, ComponentInstance, ResourceTables, TypedResource, TypedResourceIndex,
};
use crate::runtime::vm::{self, ExportFunction, VMFuncRef};
use crate::runtime::vm::{self, VMFuncRef};
use crate::store::StoreOpaque;
use crate::{AsContext, AsContextMut, Engine, Module, StoreContextMut};
use alloc::sync::Arc;
Expand Down Expand Up @@ -470,12 +470,15 @@ pub(crate) fn lookup_vmdef(
) -> vm::Export {
match def {
CoreDef::Export(e) => lookup_vmexport(store, id, e),
CoreDef::Trampoline(idx) => vm::Export::Function(ExportFunction {
func_ref: store
CoreDef::Trampoline(idx) => {
let funcref = store
.store_data_mut()
.component_instance_mut(id)
.trampoline_func_ref(*idx),
}),
.trampoline_func_ref(*idx);
// SAFETY: the `funcref` is owned by `store` and is valid within
// that store, so it's safe to create a `Func`.
vm::Export::Function(unsafe { crate::Func::from_vm_func_ref(store.id(), funcref) })
}
CoreDef::InstanceFlags(idx) => {
let id = StoreComponentInstanceId::new(store.id(), id);
vm::Export::Global(crate::Global::from_component_flags(id, *idx))
Expand Down Expand Up @@ -751,7 +754,7 @@ impl<'a> Instantiator<'a> {
.as_ref()
.map(|dtor| lookup_vmdef(store, self.id, dtor));
let dtor = dtor.map(|export| match export {
crate::runtime::vm::Export::Function(f) => f.func_ref,
crate::runtime::vm::Export::Function(f) => f.vm_func_ref(store),
_ => unreachable!(),
});
let index = self
Expand All @@ -778,7 +781,7 @@ impl<'a> Instantiator<'a> {

fn extract_realloc(&mut self, store: &mut StoreOpaque, realloc: &ExtractRealloc) {
let func_ref = match lookup_vmdef(store, self.id, &realloc.def) {
crate::runtime::vm::Export::Function(f) => f.func_ref,
crate::runtime::vm::Export::Function(f) => f.vm_func_ref(store),
_ => unreachable!(),
};
self.instance_mut(store)
Expand All @@ -787,7 +790,7 @@ impl<'a> Instantiator<'a> {

fn extract_callback(&mut self, store: &mut StoreOpaque, callback: &ExtractCallback) {
let func_ref = match lookup_vmdef(store, self.id, &callback.def) {
crate::runtime::vm::Export::Function(f) => f.func_ref,
crate::runtime::vm::Export::Function(f) => f.vm_func_ref(store),
_ => unreachable!(),
};
self.instance_mut(store)
Expand All @@ -796,7 +799,7 @@ impl<'a> Instantiator<'a> {

fn extract_post_return(&mut self, store: &mut StoreOpaque, post_return: &ExtractPostReturn) {
let func_ref = match lookup_vmdef(store, self.id, &post_return.def) {
crate::runtime::vm::Export::Function(f) => f.func_ref,
crate::runtime::vm::Export::Function(f) => f.vm_func_ref(store),
_ => unreachable!(),
};
self.instance_mut(store)
Expand Down Expand Up @@ -838,9 +841,7 @@ impl<'a> Instantiator<'a> {
// directly from an instance which should only give us valid export
// items.
let export = lookup_vmdef(store, self.id, arg);
unsafe {
self.core_imports.push_export(store, &export);
}
self.core_imports.push_export(store, &export);
}
debug_assert!(imports.next().is_none());

Expand Down Expand Up @@ -868,7 +869,7 @@ impl<'a> Instantiator<'a> {
EngineOrModuleTypeIndex::Module(m) => module.signatures().shared_type(m),
EngineOrModuleTypeIndex::RecGroup(_) => unreachable!(),
};
let actual = unsafe { f.func_ref.as_ref().type_index };
let actual = unsafe { f.vm_func_ref(store).as_ref().type_index };
assert_eq!(
expected,
Some(actual),
Expand Down
4 changes: 1 addition & 3 deletions crates/wasmtime/src/runtime/externals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,7 @@ impl Extern {
store: &StoreOpaque,
) -> Extern {
match wasmtime_export {
crate::runtime::vm::Export::Function(f) => {
Extern::Func(Func::from_wasmtime_function(f, store))
}
crate::runtime::vm::Export::Function(f) => Extern::Func(f),
crate::runtime::vm::Export::Memory { memory, shared } => {
if shared {
Extern::SharedMemory(SharedMemory::from_memory(memory, store))
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/externals/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl Table {
unsafe {
match (*table).get(gc_store, index)? {
runtime::TableElement::FuncRef(f) => {
let func = f.map(|f| Func::from_vm_func_ref(&store, f));
let func = f.map(|f| Func::from_vm_func_ref(store.id(), f));
Some(func.into())
}

Expand Down
38 changes: 18 additions & 20 deletions crates/wasmtime/src/runtime/func.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use crate::prelude::*;
use crate::runtime::Uninhabited;
use crate::runtime::vm::{
ExportFunction, InterpreterRef, SendSyncPtr, StoreBox, VMArrayCallHostFuncContext,
VMCommonStackInformation, VMContext, VMFuncRef, VMFunctionImport, VMOpaqueContext,
VMStoreContext,
InterpreterRef, SendSyncPtr, StoreBox, VMArrayCallHostFuncContext, VMCommonStackInformation,
VMContext, VMFuncRef, VMFunctionImport, VMOpaqueContext, VMStoreContext,
};
use crate::store::{AutoAssertNoGc, StoreId, StoreOpaque};
use crate::type_registry::RegisteredType;
Expand Down Expand Up @@ -528,13 +527,16 @@ impl Func {
);
}

pub(crate) unsafe fn from_vm_func_ref(
store: &StoreOpaque,
func_ref: NonNull<VMFuncRef>,
) -> Func {
/// Creates a new `Func` from a store and a funcref within that store.
///
/// # Safety
///
/// The safety of this function requires that `func_ref` is a valid function
/// pointer owned by `store`.
pub(crate) unsafe fn from_vm_func_ref(store: StoreId, func_ref: NonNull<VMFuncRef>) -> Func {
debug_assert!(func_ref.as_ref().type_index != VMSharedTypeIndex::default());
Func {
store: store.id(),
store,
unsafe_func_ref: func_ref.into(),
}
}
Expand Down Expand Up @@ -1025,7 +1027,10 @@ impl Func {
}

pub(crate) unsafe fn _from_raw(store: &mut StoreOpaque, raw: *mut c_void) -> Option<Func> {
Some(Func::from_vm_func_ref(store, NonNull::new(raw.cast())?))
Some(Func::from_vm_func_ref(
store.id(),
NonNull::new(raw.cast())?,
))
}

/// Extracts the raw value of this `Func`, which is owned by `store`.
Expand Down Expand Up @@ -1177,13 +1182,6 @@ impl Func {
self.unsafe_func_ref.as_non_null()
}

pub(crate) unsafe fn from_wasmtime_function(
export: ExportFunction,
store: &StoreOpaque,
) -> Self {
Self::from_vm_func_ref(store, export.func_ref)
}

pub(crate) fn vmimport(&self, store: &StoreOpaque) -> VMFunctionImport {
unsafe {
let f = self.vm_func_ref(store);
Expand Down Expand Up @@ -2466,7 +2464,7 @@ impl HostFunc {
self.validate_store(store);
let (funcrefs, modules) = store.func_refs_and_modules();
let funcref = funcrefs.push_arc_host(self.clone(), modules);
Func::from_vm_func_ref(store, funcref)
Func::from_vm_func_ref(store.id(), funcref)
}

/// Inserts this `HostFunc` into a `Store`, returning the `Func` pointing to
Expand Down Expand Up @@ -2500,11 +2498,11 @@ impl HostFunc {
match rooted_func_ref {
Some(funcref) => {
debug_assert!(funcref.as_ref().wasm_call.is_some());
Func::from_vm_func_ref(store, funcref)
Func::from_vm_func_ref(store.id(), funcref)
}
None => {
debug_assert!(self.func_ref().wasm_call.is_some());
Func::from_vm_func_ref(store, self.func_ref().into())
Func::from_vm_func_ref(store.id(), self.func_ref().into())
}
}
}
Expand All @@ -2514,7 +2512,7 @@ impl HostFunc {
self.validate_store(store);
let (funcrefs, modules) = store.func_refs_and_modules();
let funcref = funcrefs.push_box_host(Box::new(self), modules);
Func::from_vm_func_ref(store, funcref)
Func::from_vm_func_ref(store.id(), funcref)
}

fn validate_store(&self, store: &mut StoreOpaque) {
Expand Down
4 changes: 2 additions & 2 deletions crates/wasmtime/src/runtime/func/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ unsafe impl WasmTy for Func {
#[inline]
unsafe fn load(store: &mut AutoAssertNoGc<'_>, ptr: &ValRaw) -> Self {
let p = NonNull::new(ptr.get_funcref()).unwrap().cast();
Func::from_vm_func_ref(store, p)
Func::from_vm_func_ref(store.id(), p)
}
}

Expand Down Expand Up @@ -595,7 +595,7 @@ unsafe impl WasmTy for Option<Func> {
#[inline]
unsafe fn load(store: &mut AutoAssertNoGc<'_>, ptr: &ValRaw) -> Self {
let ptr = NonNull::new(ptr.get_funcref())?.cast();
Some(Func::from_vm_func_ref(store, ptr))
Some(Func::from_vm_func_ref(store.id(), ptr))
}
}

Expand Down
16 changes: 8 additions & 8 deletions crates/wasmtime/src/runtime/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,12 +326,14 @@ impl Instance {
fn start_raw<T>(&self, store: &mut StoreContextMut<'_, T>, start: FuncIndex) -> Result<()> {
// If a start function is present, invoke it. Make sure we use all the
// trap-handling configuration in `store` as well.
let store_id = store.0.id();
let mut instance = self.id.get_mut(store.0);
let f = instance.as_mut().get_exported_func(start);
let f = instance.as_mut().get_exported_func(store_id, start);
let caller_vmctx = instance.vmctx();
unsafe {
let funcref = f.vm_func_ref(store.0);
super::func::invoke_wasm_and_catch_traps(store, |_default_caller, vm| {
VMFuncRef::array_call(f.func_ref, vm, caller_vmctx, NonNull::from(&mut []))
VMFuncRef::array_call(funcref, vm, caller_vmctx, NonNull::from(&mut []))
})?;
}
Ok(())
Expand Down Expand Up @@ -644,14 +646,12 @@ impl OwnedImports {
/// Note that this is unsafe as the validity of `item` is not verified and
/// it contains a bunch of raw pointers.
#[cfg(feature = "component-model")]
pub(crate) unsafe fn push_export(
&mut self,
store: &StoreOpaque,
item: &crate::runtime::vm::Export,
) {
pub(crate) fn push_export(&mut self, store: &StoreOpaque, item: &crate::runtime::vm::Export) {
match item {
crate::runtime::vm::Export::Function(f) => {
let f = f.func_ref.as_ref();
// SAFETY: the funcref associated with a `Func` is valid to use
// under the `store` that owns the function.
let f = unsafe { f.vm_func_ref(store).as_ref() };
self.functions.push(VMFunctionImport {
wasm_call: f.wasm_call.unwrap(),
array_call: f.array_call,
Expand Down
27 changes: 1 addition & 26 deletions crates/wasmtime/src/runtime/vm/export.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use crate::runtime::vm::vmcontext::VMFuncRef;
use core::ptr::NonNull;

/// The value of an export passed from one instance to another.
pub enum Export {
/// A function export value.
Function(ExportFunction),
Function(crate::Func),

/// A table export value.
Table(crate::Table),
Expand All @@ -18,25 +15,3 @@ pub enum Export {
/// A tag export value.
Tag(crate::Tag),
}

/// A function export value.
#[derive(Debug, Clone, Copy)]
pub struct ExportFunction {
/// The `VMFuncRef` for this exported function.
///
/// Note that exported functions cannot be a null funcref, so this is a
/// non-null pointer.
pub func_ref: NonNull<VMFuncRef>,
}

// As part of the contract for using `ExportFunction`, synchronization
// properties must be upheld. Therefore, despite containing raw pointers,
// it is declared as Send/Sync.
unsafe impl Send for ExportFunction {}
unsafe impl Sync for ExportFunction {}

impl From<ExportFunction> for Export {
fn from(func: ExportFunction) -> Export {
Export::Function(func)
}
}
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/vm/gc/enabled/arrayref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl VMArrayRef {
.func_ref_table
.get_untyped(func_ref_id);
Val::FuncRef(unsafe {
func_ref.map(|p| Func::from_vm_func_ref(store, p.as_non_null()))
func_ref.map(|p| Func::from_vm_func_ref(store.id(), p.as_non_null()))
})
}
otherwise => unreachable!("not a top type: {otherwise:?}"),
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/vm/gc/enabled/structref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl VMStructRef {
.func_ref_table
.get_untyped(func_ref_id);
Val::FuncRef(unsafe {
func_ref.map(|p| Func::from_vm_func_ref(store, p.as_non_null()))
func_ref.map(|p| Func::from_vm_func_ref(store.id(), p.as_non_null()))
})
}
otherwise => unreachable!("not a top type: {otherwise:?}"),
Expand Down
14 changes: 9 additions & 5 deletions crates/wasmtime/src/runtime/vm/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use crate::runtime::vm::vmcontext::{
VMTableDefinition, VMTableImport, VMTagDefinition, VMTagImport,
};
use crate::runtime::vm::{
ExportFunction, GcStore, Imports, ModuleRuntimeInfo, SendSyncPtr, VMGcRef, VMGlobalKind,
VMStore, VMStoreRawPtr, VmPtr, VmSafe, WasmFault,
GcStore, Imports, ModuleRuntimeInfo, SendSyncPtr, VMGcRef, VMGlobalKind, VMStore,
VMStoreRawPtr, VmPtr, VmSafe, WasmFault,
};
use crate::store::{InstanceId, StoreId, StoreInstanceId, StoreOpaque};
use alloc::sync::Arc;
Expand Down Expand Up @@ -573,9 +573,13 @@ impl Instance {
/// # Panics
///
/// Panics if `index` is out of bounds for this instance.
pub fn get_exported_func(self: Pin<&mut Self>, index: FuncIndex) -> ExportFunction {
pub fn get_exported_func(
self: Pin<&mut Self>,
store: StoreId,
index: FuncIndex,
) -> crate::Func {
let func_ref = self.get_func_ref(index).unwrap();
ExportFunction { func_ref }
unsafe { crate::Func::from_vm_func_ref(store, func_ref) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we actually encapsulate the unsafety here? Because from_vm_func_ref requires that the store owns the given funcref, but because we take the store as an argument but get the funcref from self, we don't technically have the guarantee that the given store owns this funcref. So I think either this function should be unsafe because it relies on passing the correct StoreId in, or we need to get that StoreId from self somehow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point yeah, I meant to come back and fix this up but forgot to do so. It's a bit unfortunate how infectious the unsafety is here but I agree it's required to be sound regardless.

}

/// Lookup a table by index.
Expand Down Expand Up @@ -1497,7 +1501,7 @@ impl Instance {
export: EntityIndex,
) -> Export {
match export {
EntityIndex::Function(i) => Export::Function(self.get_exported_func(i)),
EntityIndex::Function(i) => Export::Function(self.get_exported_func(store, i)),
EntityIndex::Global(i) => Export::Global(self.get_exported_global(store, i)),
EntityIndex::Table(i) => Export::Table(self.get_exported_table(store, i)),
EntityIndex::Memory(i) => Export::Memory {
Expand Down
4 changes: 2 additions & 2 deletions crates/wasmtime/src/runtime/vm/libcalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ unsafe fn array_new_elem(
.iter()
.map(|f| {
let raw_func_ref = instance.as_mut().get_func_ref(*f);
let func = raw_func_ref.map(|p| Func::from_vm_func_ref(store, p));
let func = raw_func_ref.map(|p| Func::from_vm_func_ref(store.id(), p));
Val::FuncRef(func)
}),
);
Expand Down Expand Up @@ -931,7 +931,7 @@ unsafe fn array_init_elem(
.iter()
.map(|f| {
let raw_func_ref = instance.as_mut().get_func_ref(*f);
let func = raw_func_ref.map(|p| Func::from_vm_func_ref(&store, p));
let func = raw_func_ref.map(|p| Func::from_vm_func_ref(store.id(), p));
Val::FuncRef(func)
})
.collect::<Vec<_>>(),
Expand Down
Loading