From 353f1b48ab73aab92e014b214a7770cbbf99c2ab Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 31 Mar 2022 16:24:42 -0500 Subject: [PATCH] Split wasmtime-runtime's single getter into typed getters (#3987) This splits the existing `lookup_by_declaration` function into a lookup-per-type-of-item. This refactor ends up cleaning up a fair bit of code in the `wasmtime` crate by removing a number of `unreachable!()` blocks which are now no longer necessary. --- crates/runtime/src/instance.rs | 125 +++++++++++++---------- crates/wasmtime/src/func.rs | 8 +- crates/wasmtime/src/instance.rs | 11 +- crates/wasmtime/src/trampoline.rs | 26 ++--- crates/wasmtime/src/trampoline/global.rs | 8 +- 5 files changed, 91 insertions(+), 87 deletions(-) diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 22be3f0ca0..b08af11a17 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -287,55 +287,51 @@ impl Instance { self.vmctx() as *const VMContext as *mut VMContext } - /// Lookup an export with the given export declaration. - pub fn lookup_by_declaration(&mut self, export: &EntityIndex) -> Export { - match export { - EntityIndex::Function(index) => { - 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) = - if let Some(def_index) = self.module().defined_table_index(*index) { - (self.table_ptr(def_index), self.vmctx_ptr()) - } else { - let import = self.imported_table(*index); - (import.from, import.vmctx) - }; - ExportTable { - definition, - vmctx, - table: self.module().table_plans[*index].clone(), - } - .into() - } - EntityIndex::Memory(index) => { - let (definition, vmctx) = - if let Some(def_index) = self.module().defined_memory_index(*index) { - (self.memory_ptr(def_index), self.vmctx_ptr()) - } else { - let import = self.imported_memory(*index); - (import.from, import.vmctx) - }; - ExportMemory { - definition, - vmctx, - memory: self.module().memory_plans[*index].clone(), - } - .into() - } - EntityIndex::Global(index) => ExportGlobal { - definition: if let Some(def_index) = self.module().defined_global_index(*index) { - self.global_ptr(def_index) - } else { - self.imported_global(*index).from - }, - vmctx: self.vmctx_ptr(), - global: self.module().globals[*index], - } - .into(), + fn get_exported_func(&mut self, index: FuncIndex) -> ExportFunction { + let anyfunc = self.get_caller_checked_anyfunc(index).unwrap(); + let anyfunc = NonNull::new(anyfunc as *const VMCallerCheckedAnyfunc as *mut _).unwrap(); + ExportFunction { anyfunc } + } + + fn get_exported_table(&mut self, index: TableIndex) -> ExportTable { + let (definition, vmctx) = if let Some(def_index) = self.module().defined_table_index(index) + { + (self.table_ptr(def_index), self.vmctx_ptr()) + } else { + let import = self.imported_table(index); + (import.from, import.vmctx) + }; + ExportTable { + definition, + vmctx, + table: self.module().table_plans[index].clone(), + } + } + + fn get_exported_memory(&mut self, index: MemoryIndex) -> ExportMemory { + let (definition, vmctx) = if let Some(def_index) = self.module().defined_memory_index(index) + { + (self.memory_ptr(def_index), self.vmctx_ptr()) + } else { + let import = self.imported_memory(index); + (import.from, import.vmctx) + }; + ExportMemory { + definition, + vmctx, + memory: self.module().memory_plans[index].clone(), + } + } + + fn get_exported_global(&mut self, index: GlobalIndex) -> ExportGlobal { + ExportGlobal { + definition: if let Some(def_index) = self.module().defined_global_index(index) { + self.global_ptr(def_index) + } else { + self.imported_global(index).from + }, + vmctx: self.vmctx_ptr(), + global: self.module().globals[index], } } @@ -1062,9 +1058,34 @@ impl InstanceHandle { self.instance().module() } - /// Lookup an export with the given export declaration. - pub fn lookup_by_declaration(&mut self, export: &EntityIndex) -> Export { - self.instance_mut().lookup_by_declaration(export) + /// Lookup a function by index. + pub fn get_exported_func(&mut self, export: FuncIndex) -> ExportFunction { + self.instance_mut().get_exported_func(export) + } + + /// Lookup a global by index. + pub fn get_exported_global(&mut self, export: GlobalIndex) -> ExportGlobal { + self.instance_mut().get_exported_global(export) + } + + /// Lookup a memory by index. + pub fn get_exported_memory(&mut self, export: MemoryIndex) -> ExportMemory { + self.instance_mut().get_exported_memory(export) + } + + /// Lookup a table by index. + pub fn get_exported_table(&mut self, export: TableIndex) -> ExportTable { + self.instance_mut().get_exported_table(export) + } + + /// Lookup an item with the given index. + pub fn get_export_by_index(&mut self, export: EntityIndex) -> Export { + match export { + EntityIndex::Function(i) => Export::Function(self.get_exported_func(i)), + EntityIndex::Global(i) => Export::Global(self.get_exported_global(i)), + EntityIndex::Table(i) => Export::Table(self.get_exported_table(i)), + EntityIndex::Memory(i) => Export::Memory(self.get_exported_memory(i)), + } } /// Return an iterator over the exports of this instance. diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 12d1d07ec5..4142b2322e 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -10,7 +10,7 @@ use std::panic::{self, AssertUnwindSafe}; use std::pin::Pin; use std::ptr::NonNull; use std::sync::Arc; -use wasmtime_environ::{EntityIndex, FuncIndex}; +use wasmtime_environ::FuncIndex; use wasmtime_runtime::{ raise_user_trap, ExportFunction, InstanceAllocator, InstanceHandle, OnDemandInstanceAllocator, VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport, VMSharedSignatureIndex, @@ -2047,11 +2047,7 @@ impl HostFunc { /// Requires that this function's signature is already registered within /// `Engine`. This happens automatically during the above two constructors. fn _new(engine: &Engine, mut instance: InstanceHandle, trampoline: VMTrampoline) -> Self { - let idx = EntityIndex::Function(FuncIndex::from_u32(0)); - let export = match instance.lookup_by_declaration(&idx) { - wasmtime_runtime::Export::Function(f) => f, - _ => unreachable!(), - }; + let export = instance.get_exported_func(FuncIndex::from_u32(0)); HostFunc { instance, diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index dbde69252d..4149677288 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -8,9 +8,7 @@ use crate::{ use anyhow::{anyhow, bail, Context, Error, Result}; use std::mem; use std::sync::Arc; -use wasmtime_environ::{ - EntityIndex, EntityType, FuncIndex, GlobalIndex, MemoryIndex, PrimaryMap, TableIndex, -}; +use wasmtime_environ::{EntityType, FuncIndex, GlobalIndex, MemoryIndex, PrimaryMap, TableIndex}; use wasmtime_runtime::{ Imports, InstanceAllocationRequest, InstantiationError, StorePtr, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport, @@ -237,7 +235,7 @@ impl Instance { let id = data.id; let instance = store.instance_mut(id); // reborrow the &mut Instancehandle let item = - unsafe { Extern::from_wasmtime_export(instance.lookup_by_declaration(&index), store) }; + unsafe { Extern::from_wasmtime_export(instance.get_export_by_index(index), store) }; let data = &mut store[self.0]; data.exports[i] = Some(item.clone()); Some(item) @@ -532,10 +530,7 @@ impl<'a> Instantiator<'a> { // If a start function is present, invoke it. Make sure we use all the // trap-handling configuration in `store` as well. let instance = store.0.instance_mut(id); - let f = match instance.lookup_by_declaration(&EntityIndex::Function(start)) { - wasmtime_runtime::Export::Function(f) => f, - _ => unreachable!(), // valid modules shouldn't hit this - }; + let f = instance.get_exported_func(start); let vmctx = instance.vmctx_ptr(); unsafe { super::func::invoke_wasm_and_catch_traps(store, |_default_callee| { diff --git a/crates/wasmtime/src/trampoline.rs b/crates/wasmtime/src/trampoline.rs index f6e92d79cc..357302633e 100644 --- a/crates/wasmtime/src/trampoline.rs +++ b/crates/wasmtime/src/trampoline.rs @@ -17,7 +17,7 @@ use crate::{GlobalType, MemoryType, TableType, Val}; use anyhow::Result; use std::any::Any; use std::sync::Arc; -use wasmtime_environ::{EntityIndex, GlobalIndex, MemoryIndex, Module, SignatureIndex, TableIndex}; +use wasmtime_environ::{GlobalIndex, MemoryIndex, Module, SignatureIndex, TableIndex}; use wasmtime_runtime::{ Imports, InstanceAllocationRequest, InstanceAllocator, OnDemandInstanceAllocator, StorePtr, VMFunctionImport, VMSharedSignatureIndex, @@ -60,11 +60,9 @@ pub fn generate_global_export( val: Val, ) -> Result { let instance = create_global(store, gt, val)?; - let idx = EntityIndex::Global(GlobalIndex::from_u32(0)); - match store.instance_mut(instance).lookup_by_declaration(&idx) { - wasmtime_runtime::Export::Global(g) => Ok(g), - _ => unreachable!(), - } + Ok(store + .instance_mut(instance) + .get_exported_global(GlobalIndex::from_u32(0))) } pub fn generate_memory_export( @@ -72,11 +70,9 @@ pub fn generate_memory_export( m: &MemoryType, ) -> Result { let instance = create_memory(store, m)?; - let idx = EntityIndex::Memory(MemoryIndex::from_u32(0)); - match store.instance_mut(instance).lookup_by_declaration(&idx) { - wasmtime_runtime::Export::Memory(m) => Ok(m), - _ => unreachable!(), - } + Ok(store + .instance_mut(instance) + .get_exported_memory(MemoryIndex::from_u32(0))) } pub fn generate_table_export( @@ -84,9 +80,7 @@ pub fn generate_table_export( t: &TableType, ) -> Result { let instance = create_table(store, t)?; - let idx = EntityIndex::Table(TableIndex::from_u32(0)); - match store.instance_mut(instance).lookup_by_declaration(&idx) { - wasmtime_runtime::Export::Table(t) => Ok(t), - _ => unreachable!(), - } + Ok(store + .instance_mut(instance) + .get_exported_table(TableIndex::from_u32(0))) } diff --git a/crates/wasmtime/src/trampoline/global.rs b/crates/wasmtime/src/trampoline/global.rs index 69b63487bc..ccc00c142b 100644 --- a/crates/wasmtime/src/trampoline/global.rs +++ b/crates/wasmtime/src/trampoline/global.rs @@ -71,11 +71,9 @@ pub fn create_global(store: &mut StoreOpaque, gt: &GlobalType, val: Val) -> Resu if let Some(x) = externref_init { let instance = store.instance_mut(id); - match instance.lookup_by_declaration(&EntityIndex::Global(global_id)) { - wasmtime_runtime::Export::Global(g) => unsafe { - *(*g.definition).as_externref_mut() = Some(x.inner); - }, - _ => unreachable!(), + let g = instance.get_exported_global(global_id); + unsafe { + *(*g.definition).as_externref_mut() = Some(x.inner); } }