From 0bee67a852bc887c35847b57bc5edc0e39dd9097 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 17 Jan 2020 09:43:35 -0600 Subject: [PATCH] Document and update the API of the `externals.rs` module (#812) * Document and update the API of the `externals.rs` module This commit ensures that all public methods and items are documented in the `externals.rs` module, notably all external values that can be imported and exported in WebAssembly. Along the way this also tidies up the API and fixes a few bugs: * `Global::new` now returns a `Result` and fails if the provided value does not match the type of the global. * `Global::set` now returns a `Result` and fails if the global is either immutable or the provided value doesn't match the type of the global. * `Table::new` now fails if the provided initializer does not match the element type. * `Table::get` now returns `Option` instead of implicitly returning null. * `Table::set` now returns `Result<()>`, returning an error on out of bounds or if the input type is of the wrong type. * `Table::grow` now returns `Result`, returning the previous number of table elements if succesful or an error if the maximum is reached or the initializer value is of the wrong type. Additionally a bug was fixed here where if the wrong initializer was provided the table would be grown still, but initialization would fail. * `Memory::data` was renamed to `Memory::data_unchecked_mut`. Additionally `Memory::data_unchecked` was added. Lots of caveats were written down about how using the method can go wrong. * `Memory::grow` now returns `Result`, returning an error if growth fails or the number of pages previous the growth if successful. * Run rustfmt * Fix another test * Update crates/api/src/externals.rs Co-Authored-By: Sergei Pepyakin Co-authored-by: Sergei Pepyakin --- crates/api/examples/memory.rs | 22 +- crates/api/src/externals.rs | 336 ++++++++++++++++++---- crates/api/src/trampoline/table.rs | 5 +- crates/api/src/types.rs | 2 +- crates/api/src/values.rs | 9 +- crates/api/tests/externals.rs | 54 ++++ crates/api/tests/invoke_func_via_table.rs | 1 + crates/c-api/src/lib.rs | 40 +-- crates/fuzzing/src/oracles/dummy.rs | 4 +- crates/wast/src/spectest.rs | 10 +- 10 files changed, 384 insertions(+), 99 deletions(-) create mode 100644 crates/api/tests/externals.rs diff --git a/crates/api/examples/memory.rs b/crates/api/examples/memory.rs index a167e6f1ac..15fb127280 100644 --- a/crates/api/examples/memory.rs +++ b/crates/api/examples/memory.rs @@ -105,9 +105,9 @@ fn main() -> Result<(), Error> { println!("Checking memory..."); check!(memory.size(), 2u32); check!(memory.data_size(), 0x20000usize); - check!(unsafe { memory.data()[0] }, 0); - check!(unsafe { memory.data()[0x1000] }, 1); - check!(unsafe { memory.data()[0x1003] }, 4); + check!(unsafe { memory.data_unchecked_mut()[0] }, 0); + check!(unsafe { memory.data_unchecked_mut()[0x1000] }, 1); + check!(unsafe { memory.data_unchecked_mut()[0x1003] }, 4); check!(call!(size_func,), 2); check!(call!(load_func, 0), 0); @@ -119,20 +119,20 @@ fn main() -> Result<(), Error> { // Mutate memory. println!("Mutating memory..."); unsafe { - memory.data()[0x1003] = 5; + memory.data_unchecked_mut()[0x1003] = 5; } check_ok!(store_func, 0x1002, 6); check_trap!(store_func, 0x20000, 0); - check!(unsafe { memory.data()[0x1002] }, 6); - check!(unsafe { memory.data()[0x1003] }, 5); + check!(unsafe { memory.data_unchecked()[0x1002] }, 6); + check!(unsafe { memory.data_unchecked()[0x1003] }, 5); check!(call!(load_func, 0x1002), 6); check!(call!(load_func, 0x1003), 5); // Grow memory. println!("Growing memory..."); - check!(memory.grow(1), true); + memory.grow(1)?; check!(memory.size(), 3u32); check!(memory.data_size(), 0x30000usize); @@ -141,8 +141,8 @@ fn main() -> Result<(), Error> { check_trap!(load_func, 0x30000); check_trap!(store_func, 0x30000, 0); - check!(memory.grow(1), false); - check!(memory.grow(0), true); + memory.grow(1).unwrap_err(); + memory.grow(0).unwrap(); // Create stand-alone memory. // TODO(wasm+): Once Wasm allows multiple memories, turn this into import. @@ -150,8 +150,8 @@ fn main() -> Result<(), Error> { let memorytype = MemoryType::new(Limits::new(5, Some(5))); let memory2 = Memory::new(&store, memorytype); check!(memory2.size(), 5u32); - check!(memory2.grow(1), false); - check!(memory2.grow(0), true); + memory2.grow(1).unwrap_err(); + memory2.grow(0).unwrap(); // Shut down. println!("Shutting down..."); diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index 0062b60b1d..a47496c881 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -1,10 +1,11 @@ use crate::callable::{Callable, NativeCallable, WasmtimeFn, WrappedCallable}; -use crate::r#ref::AnyRef; use crate::runtime::Store; use crate::trampoline::{generate_global_export, generate_memory_export, generate_table_export}; use crate::trap::Trap; use crate::types::{ExternType, FuncType, GlobalType, MemoryType, TableType, ValType}; use crate::values::{from_checked_anyfunc, into_checked_anyfunc, Val}; +use crate::Mutability; +use anyhow::{anyhow, bail, Result}; use std::fmt; use std::rc::Rc; use std::slice; @@ -13,33 +14,60 @@ use wasmtime_runtime::InstanceHandle; // Externals +/// An external item to a WebAssembly module, or a list of what can possibly be +/// exported from a wasm module. +/// +/// This is both returned from [`Instance::exports`](crate::Instance::exports) +/// as well as required by [`Instance::new`](crate::Instance::new). In other +/// words, this is the type of extracted values from an instantiated module, and +/// it's also used to provide imported values when instantiating a module. #[derive(Clone)] pub enum Extern { + /// A WebAssembly `func` which can be called. Func(Func), + /// A WebAssembly `global` which acts like a `Cell` of sorts, supporting + /// `get` and `set` operations. Global(Global), + /// A WebAssembly `table` which is an array of `Val` types. Table(Table), + /// A WebAssembly linear memory. Memory(Memory), } impl Extern { + /// Returns the underlying `Func`, if this external is a function. + /// + /// Returns `None` if this is not a function. pub fn func(&self) -> Option<&Func> { match self { Extern::Func(func) => Some(func), _ => None, } } + + /// Returns the underlying `Global`, if this external is a global. + /// + /// Returns `None` if this is not a global. pub fn global(&self) -> Option<&Global> { match self { Extern::Global(global) => Some(global), _ => None, } } + + /// Returns the underlying `Table`, if this external is a table. + /// + /// Returns `None` if this is not a table. pub fn table(&self) -> Option<&Table> { match self { Extern::Table(table) => Some(table), _ => None, } } + + /// Returns the underlying `Memory`, if this external is a memory. + /// + /// Returns `None` if this is not a memory. pub fn memory(&self) -> Option<&Memory> { match self { Extern::Memory(memory) => Some(memory), @@ -47,6 +75,7 @@ impl Extern { } } + /// Returns the type associated with this `Extern`. pub fn ty(&self) -> ExternType { match self { Extern::Func(ft) => ExternType::Func(ft.ty().clone()), @@ -111,6 +140,22 @@ impl From for Extern { } } +/// A WebAssembly function which can be called. +/// +/// This type can represent a number of callable items, such as: +/// +/// * An exported function from a WebAssembly module. +/// * A user-defined function used to satisfy an import. +/// +/// These types of callable items are all wrapped up in this `Func` and can be +/// used to both instantiate an [`Instance`](crate::Instance) as well as be +/// extracted from an [`Instance`](crate::Instance). +/// +/// # `Func` and `Clone` +/// +/// Functions are internally reference counted so you can `clone` a `Func`. The +/// cloning process only performs a shallow clone, so two cloned `Func` +/// instances are equivalent in their functionality. #[derive(Clone)] pub struct Func { _store: Store, @@ -119,6 +164,21 @@ pub struct Func { } impl Func { + /// Creates a new `Func` with the given arguments, typically to create a + /// user-defined function to pass as an import to a module. + /// + /// * `store` - a cache of data where information is stored, typically + /// shared with a [`Module`](crate::Module). + /// + /// * `ty` - the signature of this function, used to indicate what the + /// inputs and outputs are, which must be WebAssembly types. + /// + /// * `callable` - a type implementing the [`Callable`] trait which + /// is the implementation of this `Func` value. + /// + /// Note that the implementation of `callable` must adhere to the `ty` + /// signature given, error or traps may occur if it does not respect the + /// `ty` signature. pub fn new(store: &Store, ty: FuncType, callable: Rc) -> Self { let callable = Rc::new(NativeCallable::new(callable, &ty, &store)); Func::from_wrapped(store, ty, callable) @@ -136,18 +196,30 @@ impl Func { } } + /// Returns the underlying wasm type that this `Func` has. pub fn ty(&self) -> &FuncType { &self.ty } + /// Returns the number of parameters that this function takes. pub fn param_arity(&self) -> usize { self.ty.params().len() } + /// Returns the number of results this function produces. pub fn result_arity(&self) -> usize { self.ty.results().len() } + /// Invokes this function with the `params` given, returning the results and + /// any trap, if one occurs. + /// + /// The `params` here must match the type signature of this `Func`, or a + /// trap will occur. If a trap occurs while executing this function, then a + /// trap will also be returned. + /// + /// This function should not panic unless the underlying function itself + /// initiates a panic. pub fn call(&self, params: &[Val]) -> Result, Trap> { let mut results = vec![Val::null(); self.result_arity()]; self.callable.call(params, &mut results)?; @@ -183,6 +255,21 @@ impl fmt::Debug for Func { } } +/// A WebAssembly `global` value which can be read and written to. +/// +/// A `global` in WebAssembly is sort of like a global variable within an +/// [`Instance`](crate::Instance). The `global.get` and `global.set` +/// instructions will modify and read global values in a wasm module. Globals +/// can either be imported or exported from wasm modules. +/// +/// If you're familiar with Rust already you can think of a `Global` as a sort +/// of `Rc>`, more or less. +/// +/// # `Global` and `Clone` +/// +/// Globals are internally reference counted so you can `clone` a `Global`. The +/// cloning process only performs a shallow clone, so two cloned `Global` +/// instances are equivalent in their functionality. #[derive(Clone)] pub struct Global { inner: Rc, @@ -197,19 +284,33 @@ struct GlobalInner { } impl Global { - pub fn new(store: &Store, ty: GlobalType, val: Val) -> Global { - let (wasmtime_export, wasmtime_state) = - generate_global_export(&ty, val).expect("generated global"); - Global { + /// Creates a new WebAssembly `global` value with the provide type `ty` and + /// initial value `val`. + /// + /// The `store` argument provided is used as a general global cache for + /// information, and otherwise the `ty` and `val` arguments are used to + /// initialize the global. + /// + /// # Errors + /// + /// Returns an error if the `ty` provided does not match the type of the + /// value `val`. + pub fn new(store: &Store, ty: GlobalType, val: Val) -> Result { + if val.ty() != *ty.content() { + bail!("value provided does not match the type of this global"); + } + let (wasmtime_export, wasmtime_state) = generate_global_export(&ty, val)?; + Ok(Global { inner: Rc::new(GlobalInner { _store: store.clone(), ty, wasmtime_export, wasmtime_state: Some(wasmtime_state), }), - } + }) } + /// Returns the underlying type of this `global`. pub fn ty(&self) -> &GlobalType { &self.inner.ty } @@ -221,6 +322,7 @@ impl Global { } } + /// Returns the current [`Val`] of this global. pub fn get(&self) -> Val { let definition = unsafe { &mut *self.wasmtime_global_definition() }; unsafe { @@ -234,9 +336,18 @@ impl Global { } } - pub fn set(&self, val: Val) { + /// Attempts to set the current value of this global to [`Val`]. + /// + /// # Errors + /// + /// Returns an error if this global has a different type than `Val`, or if + /// it's not a mutable global. + pub fn set(&self, val: Val) -> Result<()> { + if self.ty().mutability() != Mutability::Var { + bail!("immutable global cannot be set"); + } if val.ty() != *self.ty().content() { - panic!( + bail!( "global of type {:?} cannot be set to {:?}", self.ty().content(), val.ty() @@ -252,6 +363,7 @@ impl Global { _ => unimplemented!("Global::set for {:?}", val.ty()), } } + Ok(()) } pub(crate) fn wasmtime_export(&self) -> &wasmtime_runtime::Export { @@ -279,6 +391,21 @@ impl Global { } } +/// A WebAssembly `table`, or an array of values. +/// +/// Like [`Memory`] a table is an indexed array of values, but unlike [`Memory`] +/// it's an array of WebAssembly values rather than bytes. One of the most +/// common usages of a table is a function table for wasm modules, where each +/// element has the `Func` type. +/// +/// Tables, like globals, are not threadsafe and can only be used on one thread. +/// Tables can be grown in size and each element can be read/written. +/// +/// # `Table` and `Clone` +/// +/// Tables are internally reference counted so you can `clone` a `Table`. The +/// cloning process only performs a shallow clone, so two cloned `Table` +/// instances are equivalent in their functionality. #[derive(Clone)] pub struct Table { store: Store, @@ -287,43 +414,35 @@ pub struct Table { wasmtime_export: wasmtime_runtime::Export, } -fn get_table_item( - handle: &InstanceHandle, - store: &Store, - table_index: wasm::DefinedTableIndex, - item_index: u32, -) -> Val { - if let Some(item) = handle.table_get(table_index, item_index) { - from_checked_anyfunc(item, store) - } else { - AnyRef::null().into() - } -} - fn set_table_item( handle: &mut InstanceHandle, - store: &Store, table_index: wasm::DefinedTableIndex, item_index: u32, - val: Val, -) -> bool { - let item = into_checked_anyfunc(val, store); + item: wasmtime_runtime::VMCallerCheckedAnyfunc, +) -> Result<()> { if let Some(item_ref) = handle.table_get_mut(table_index, item_index) { *item_ref = item; - true + Ok(()) } else { - false + bail!("table element index out of bounds") } } impl Table { - pub fn new(store: &Store, ty: TableType, init: Val) -> Table { - match ty.element() { - ValType::FuncRef => (), - _ => panic!("table is not for funcref"), - } - let (mut wasmtime_handle, wasmtime_export) = - generate_table_export(&ty).expect("generated table"); + /// Creates a new `Table` with the given parameters. + /// + /// * `store` - a global cache to store information in + /// * `ty` - the type of this table, containing both the element type as + /// well as the initial size and maximum size, if any. + /// * `init` - the initial value to fill all table entries with, if the + /// table starts with an initial size. + /// + /// # Errors + /// + /// Returns an error if `init` does not match the element type of the table. + pub fn new(store: &Store, ty: TableType, init: Val) -> Result
{ + let item = into_checked_anyfunc(init, store)?; + let (mut wasmtime_handle, wasmtime_export) = generate_table_export(&ty)?; // Initialize entries with the init value. match wasmtime_export { @@ -331,22 +450,22 @@ impl Table { let index = wasmtime_handle.table_index(unsafe { &*definition }); let len = unsafe { (*definition).current_elements }; for i in 0..len { - let _success = - set_table_item(&mut wasmtime_handle, store, index, i, init.clone()); - assert!(_success); + set_table_item(&mut wasmtime_handle, index, i, item.clone())?; } } - _ => panic!("global definition not found"), + _ => unreachable!("export should be a table"), } - Table { + Ok(Table { store: store.clone(), ty, wasmtime_handle, wasmtime_export, - } + }) } + /// Returns the underlying type of this table, including its element type as + /// well as the maximum/minimum lower bounds. pub fn ty(&self) -> &TableType { &self.ty } @@ -360,17 +479,29 @@ impl Table { } } - pub fn get(&self, index: u32) -> Val { + /// Returns the table element value at `index`. + /// + /// Returns `None` if `index` is out of bounds. + pub fn get(&self, index: u32) -> Option { let table_index = self.wasmtime_table_index(); - get_table_item(&self.wasmtime_handle, &self.store, table_index, index) + let item = self.wasmtime_handle.table_get(table_index, index)?; + Some(from_checked_anyfunc(item, &self.store)) } - pub fn set(&self, index: u32, val: Val) -> bool { + /// Writes the `val` provided into `index` within this table. + /// + /// # Errors + /// + /// Returns an error if `index` is out of bounds or if `val` does not have + /// the right type to be stored in this table. + pub fn set(&self, index: u32, val: Val) -> Result<()> { let table_index = self.wasmtime_table_index(); let mut wasmtime_handle = self.wasmtime_handle.clone(); - set_table_item(&mut wasmtime_handle, &self.store, table_index, index, val) + let item = into_checked_anyfunc(val, &self.store)?; + set_table_item(&mut wasmtime_handle, table_index, index, item) } + /// Returns the current size of this table. pub fn size(&self) -> u32 { match self.wasmtime_export { wasmtime_runtime::Export::Table { definition, .. } => unsafe { @@ -380,19 +511,26 @@ impl Table { } } - pub fn grow(&self, delta: u32, init: Val) -> bool { + /// Grows the size of this table by `delta` more elements, initialization + /// all new elements to `init`. + /// + /// # Errors + /// + /// Returns an error if the table cannot be grown by `delta`, for example + /// if it would cause the table to exceed its maximum size. Also returns an + /// error if `init` is not of the right type. + pub fn grow(&self, delta: u32, init: Val) -> Result { let index = self.wasmtime_table_index(); + let item = into_checked_anyfunc(init, &self.store)?; if let Some(len) = self.wasmtime_handle.clone().table_grow(index, delta) { let mut wasmtime_handle = self.wasmtime_handle.clone(); for i in 0..delta { let i = len - (delta - i); - let _success = - set_table_item(&mut wasmtime_handle, &self.store, index, i, init.clone()); - assert!(_success); + set_table_item(&mut wasmtime_handle, index, i, item.clone())?; } - true + Ok(len) } else { - false + bail!("failed to grow table by `{}`", delta) } } @@ -420,6 +558,26 @@ impl Table { } } +/// A WebAssembly linear memory. +/// +/// WebAssembly memories represent a contiguous array of bytes that have a size +/// that is always a multiple of the WebAssembly page size, currently 64 +/// kilobytes. +/// +/// WebAssembly memory is used for global data, statics in C/C++/Rust, shadow +/// stack memory, etc. Accessing wasm memory is generally quite fast! +/// +/// # `Memory` and `Clone` +/// +/// Memories are internally reference counted so you can `clone` a `Memory`. The +/// cloning process only performs a shallow clone, so two cloned `Memory` +/// instances are equivalent in their functionality. +/// +/// # `Memory` and threads +/// +/// It is intended that `Memory` is safe to share between threads. At this time +/// this is not implemented in `wasmtime`, however. This is planned to be +/// implemented though! #[derive(Clone)] pub struct Memory { _store: Store, @@ -429,6 +587,11 @@ pub struct Memory { } impl Memory { + /// Creates a new WebAssembly memory given the configuration of `ty`. + /// + /// The `store` argument is a general location for cache information, and + /// otherwise the memory will immediately be allocated according to the + /// type's configuration. All WebAssembly memory is initialized to zero. pub fn new(store: &Store, ty: MemoryType) -> Memory { let (wasmtime_handle, wasmtime_export) = generate_memory_export(&ty).expect("generated memory"); @@ -440,6 +603,7 @@ impl Memory { } } + /// Returns the underlying type of this memory. pub fn ty(&self) -> &MemoryType { &self.ty } @@ -451,28 +615,88 @@ impl Memory { } } - /// Returns a mutable slice the current memory. + /// Returns this memory as a slice view that can be read natively in Rust. + /// /// # Safety - /// Marked unsafe due to posibility that wasmtime can resize internal memory - /// from other threads. - pub unsafe fn data(&self) -> &mut [u8] { + /// + /// This is an unsafe operation because there is no guarantee that the + /// following operations do not happen concurrently while the slice is in + /// use: + /// + /// * Data could be modified by calling into a wasm module. + /// * Memory could be relocated through growth by calling into a wasm + /// module. + /// * When threads are supported, non-atomic reads will race with other + /// writes. + /// + /// Extreme care need be taken when the data of a `Memory` is read. The + /// above invariants all need to be upheld at a bare minimum, and in + /// general you'll need to ensure that while you're looking at slice you're + /// the only one who can possibly look at the slice and read/write it. + /// + /// Be sure to keep in mind that `Memory` is reference counted, meaning + /// that there may be other users of this `Memory` instance elsewhere in + /// your program. Additionally `Memory` can be shared and used in any number + /// of wasm instances, so calling any wasm code should be considered + /// dangerous while you're holding a slice of memory. + pub unsafe fn data_unchecked(&self) -> &[u8] { + self.data_unchecked_mut() + } + + /// Returns this memory as a slice view that can be read and written + /// natively in Rust. + /// + /// # Safety + /// + /// All of the same safety caveats of [`Memory::data_unchecked`] apply + /// here, doubly so because this is returning a mutable slice! As a + /// double-extra reminder, remember that `Memory` is reference counted, so + /// you can very easily acquire two mutable slices by simply calling this + /// function twice. Extreme caution should be used when using this method, + /// and in general you probably want to result to unsafe accessors and the + /// `data` methods below. + pub unsafe fn data_unchecked_mut(&self) -> &mut [u8] { let definition = &*self.wasmtime_memory_definition(); slice::from_raw_parts_mut(definition.base, definition.current_length) } + /// Returns the base pointer, in the host's address space, that the memory + /// is located at. + /// + /// When reading and manipulating memory be sure to read up on the caveats + /// of [`Memory::data_unchecked`] to make sure that you can safely + /// read/write the memory. pub fn data_ptr(&self) -> *mut u8 { unsafe { (*self.wasmtime_memory_definition()).base } } + /// Returns the byte length of this memory. + /// + /// The returned value will be a multiple of the wasm page size, 64k. pub fn data_size(&self) -> usize { unsafe { (*self.wasmtime_memory_definition()).current_length } } + /// Returns the size, in pages, of this wasm memory. pub fn size(&self) -> u32 { (self.data_size() / wasmtime_environ::WASM_PAGE_SIZE as usize) as u32 } - pub fn grow(&self, delta: u32) -> bool { + /// Grows this WebAssembly memory by `delta` pages. + /// + /// This will attempt to add `delta` more pages of memory on to the end of + /// this `Memory` instance. If successful this may relocate the memory and + /// cause [`Memory::data_ptr`] to return a new value. Additionally previous + /// slices into this memory may no longer be valid. + /// + /// On success returns the number of pages this memory previously had + /// before the growth succeeded. + /// + /// # Errors + /// + /// Returns an error if memory could not be grown, for example if it exceeds + /// the maximum limits of this memory. + pub fn grow(&self, delta: u32) -> Result { match self.wasmtime_export { wasmtime_runtime::Export::Memory { definition, .. } => { let definition = unsafe { &(*definition) }; @@ -480,7 +704,7 @@ impl Memory { self.wasmtime_handle .clone() .memory_grow(index, delta) - .is_some() + .ok_or_else(|| anyhow!("failed to grow memory")) } _ => panic!("memory definition not found"), } diff --git a/crates/api/src/trampoline/table.rs b/crates/api/src/trampoline/table.rs index 713de1b43a..920012dc66 100644 --- a/crates/api/src/trampoline/table.rs +++ b/crates/api/src/trampoline/table.rs @@ -13,10 +13,7 @@ pub fn create_handle_with_table(table: &TableType) -> Result { maximum: table.limits().max(), ty: match table.element() { ValType::FuncRef => wasm::TableElementType::Func, - _ => match table.element().get_wasmtime_type() { - Some(t) => wasm::TableElementType::Val(t), - None => bail!("cannot support {:?} as a table element", table.element()), - }, + _ => bail!("cannot support {:?} as a table element", table.element()), }, }; let tunable = Default::default(); diff --git a/crates/api/src/types.rs b/crates/api/src/types.rs index 5162c49c54..b8361352f6 100644 --- a/crates/api/src/types.rs +++ b/crates/api/src/types.rs @@ -5,7 +5,7 @@ use wasmtime_environ::{ir, wasm}; // Type attributes /// Indicator of whether a global is mutable or not -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub enum Mutability { /// The global is constant and its value does not change Const, diff --git a/crates/api/src/values.rs b/crates/api/src/values.rs index 4d470a2e23..2dbc5f74c2 100644 --- a/crates/api/src/values.rs +++ b/crates/api/src/values.rs @@ -2,6 +2,7 @@ use crate::externals::Func; use crate::r#ref::AnyRef; use crate::runtime::Store; use crate::types::ValType; +use anyhow::{bail, Result}; use std::ptr; use wasmtime_environ::ir; use wasmtime_jit::RuntimeValue; @@ -188,8 +189,8 @@ impl From for Val { pub(crate) fn into_checked_anyfunc( val: Val, store: &Store, -) -> wasmtime_runtime::VMCallerCheckedAnyfunc { - match val { +) -> Result { + Ok(match val { Val::AnyRef(AnyRef::Null) => wasmtime_runtime::VMCallerCheckedAnyfunc { func_ptr: ptr::null(), type_index: wasmtime_runtime::VMSharedSignatureIndex::default(), @@ -211,8 +212,8 @@ pub(crate) fn into_checked_anyfunc( vmctx, } } - _ => panic!("val is not funcref"), - } + _ => bail!("val is not funcref"), + }) } pub(crate) fn from_checked_anyfunc( diff --git a/crates/api/tests/externals.rs b/crates/api/tests/externals.rs new file mode 100644 index 0000000000..749af41e2b --- /dev/null +++ b/crates/api/tests/externals.rs @@ -0,0 +1,54 @@ +use wasmtime::*; + +#[test] +fn bad_globals() { + let ty = GlobalType::new(ValType::I32, Mutability::Var); + assert!(Global::new(&Store::default(), ty.clone(), Val::I64(0)).is_err()); + assert!(Global::new(&Store::default(), ty.clone(), Val::F32(0)).is_err()); + assert!(Global::new(&Store::default(), ty.clone(), Val::F64(0)).is_err()); + + let ty = GlobalType::new(ValType::I32, Mutability::Const); + let g = Global::new(&Store::default(), ty.clone(), Val::I32(0)).unwrap(); + assert!(g.set(Val::I32(1)).is_err()); + + let ty = GlobalType::new(ValType::I32, Mutability::Var); + let g = Global::new(&Store::default(), ty.clone(), Val::I32(0)).unwrap(); + assert!(g.set(Val::I64(0)).is_err()); +} + +#[test] +fn bad_tables() { + // i32 not supported yet + let ty = TableType::new(ValType::I32, Limits::new(0, Some(1))); + assert!(Table::new(&Store::default(), ty.clone(), Val::I32(0)).is_err()); + + // mismatched initializer + let ty = TableType::new(ValType::FuncRef, Limits::new(0, Some(1))); + assert!(Table::new(&Store::default(), ty.clone(), Val::I32(0)).is_err()); + + // get out of bounds + let ty = TableType::new(ValType::FuncRef, Limits::new(0, Some(1))); + let t = Table::new(&Store::default(), ty.clone(), Val::AnyRef(AnyRef::Null)).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::AnyRef(AnyRef::Null)).unwrap(); + assert!(t.set(0, Val::I32(0)).is_err()); + assert!(t.set(0, Val::AnyRef(AnyRef::Null)).is_ok()); + assert!(t.set(1, Val::AnyRef(AnyRef::Null)).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::AnyRef(AnyRef::Null)).unwrap(); + assert!(t.grow(0, Val::AnyRef(AnyRef::Null)).is_ok()); + assert!(t.grow(1, Val::AnyRef(AnyRef::Null)).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::AnyRef(AnyRef::Null)).unwrap(); + assert!(t.grow(1, Val::I32(0)).is_err()); + assert_eq!(t.size(), 1); +} diff --git a/crates/api/tests/invoke_func_via_table.rs b/crates/api/tests/invoke_func_via_table.rs index 3320ebbb9e..1c236e05a6 100644 --- a/crates/api/tests/invoke_func_via_table.rs +++ b/crates/api/tests/invoke_func_via_table.rs @@ -24,6 +24,7 @@ fn test_invoke_func_via_table() -> Result<()> { .table() .unwrap() .get(0) + .unwrap() .funcref() .unwrap() .clone(); diff --git a/crates/c-api/src/lib.rs b/crates/c-api/src/lib.rs index ea647d207d..d55300d8c2 100644 --- a/crates/c-api/src/lib.rs +++ b/crates/c-api/src/lib.rs @@ -1381,11 +1381,16 @@ pub unsafe extern "C" fn wasm_global_new( gt: *const wasm_globaltype_t, val: *const wasm_val_t, ) -> *mut wasm_global_t { - let global = HostRef::new(Global::new( - &(*store).store.borrow(), - (*gt).globaltype.clone(), - (*val).val(), - )); + let global = HostRef::new( + match Global::new( + &(*store).store.borrow(), + (*gt).globaltype.clone(), + (*val).val(), + ) { + Ok(g) => g, + Err(_) => return ptr::null_mut(), + }, + ); let g = Box::new(wasm_global_t { ext: wasm_extern_t { which: ExternHost::Global(global), @@ -1401,7 +1406,8 @@ pub unsafe extern "C" fn wasm_global_get(g: *const wasm_global_t, out: *mut wasm #[no_mangle] pub unsafe extern "C" fn wasm_global_set(g: *mut wasm_global_t, val: *const wasm_val_t) { - (*g).global().borrow().set((*val).val()) + let result = (*g).global().borrow().set((*val).val()); + drop(result); // TODO: should communicate this via the api somehow? } #[no_mangle] @@ -1480,7 +1486,7 @@ pub unsafe extern "C" fn wasm_memory_grow( m: *mut wasm_memory_t, delta: wasm_memory_pages_t, ) -> bool { - (*m).memory().borrow().grow(delta) + (*m).memory().borrow().grow(delta).is_ok() } #[no_mangle] @@ -1570,13 +1576,13 @@ pub unsafe extern "C" fn wasm_table_new( } else { Val::AnyRef(AnyRef::Null) }; + let table = match Table::new(&(*store).store.borrow(), (*tt).tabletype.clone(), init) { + Ok(table) => table, + Err(_) => return ptr::null_mut(), + }; let t = Box::new(wasm_table_t { ext: wasm_extern_t { - which: ExternHost::Table(HostRef::new(Table::new( - &(*store).store.borrow(), - (*tt).tabletype.clone(), - init, - ))), + which: ExternHost::Table(HostRef::new(table)), }, }); Box::into_raw(t) @@ -1607,8 +1613,10 @@ pub unsafe extern "C" fn wasm_table_get( t: *const wasm_table_t, index: wasm_table_size_t, ) -> *mut wasm_ref_t { - let val = (*t).table().borrow().get(index); - into_funcref(val) + match (*t).table().borrow().get(index) { + Some(val) => into_funcref(val), + None => into_funcref(Val::AnyRef(AnyRef::Null)), + } } #[no_mangle] @@ -1618,7 +1626,7 @@ pub unsafe extern "C" fn wasm_table_set( r: *mut wasm_ref_t, ) -> bool { let val = from_funcref(r); - (*t).table().borrow().set(index, val) + (*t).table().borrow().set(index, val).is_ok() } #[no_mangle] @@ -1633,7 +1641,7 @@ pub unsafe extern "C" fn wasm_table_grow( init: *mut wasm_ref_t, ) -> bool { let init = from_funcref(init); - (*t).table().borrow().grow(delta, init) + (*t).table().borrow().grow(delta, init).is_ok() } #[no_mangle] diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index 0a0b06b69b..b51e80e1f4 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -73,13 +73,13 @@ pub fn dummy_value(val_ty: &ValType) -> Result { /// Construct a dummy global for the given global type. pub fn dummy_global(store: &Store, ty: GlobalType) -> Result { let val = dummy_value(ty.content())?; - Ok(Global::new(store, ty, val)) + Ok(Global::new(store, ty, val).unwrap()) } /// Construct a dummy table for the given table type. pub fn dummy_table(store: &Store, ty: TableType) -> Result { let init_val = dummy_value(&ty.element())?; - Ok(Table::new(store, ty, init_val)) + Ok(Table::new(store, ty, init_val).unwrap()) } /// Construct a dummy memory for the given memory type. diff --git a/crates/wast/src/spectest.rs b/crates/wast/src/spectest.rs index c180a7689c..3f6d60a01b 100644 --- a/crates/wast/src/spectest.rs +++ b/crates/wast/src/spectest.rs @@ -76,23 +76,23 @@ pub fn instantiate_spectest(store: &Store) -> HashMap<&'static str, Extern> { ret.insert("print_f64_f64", Extern::Func(func)); let ty = GlobalType::new(ValType::I32, Mutability::Const); - let g = Global::new(store, ty, Val::I32(666)); + let g = Global::new(store, ty, Val::I32(666)).unwrap(); ret.insert("global_i32", Extern::Global(g)); let ty = GlobalType::new(ValType::I64, Mutability::Const); - let g = Global::new(store, ty, Val::I64(666)); + let g = Global::new(store, ty, Val::I64(666)).unwrap(); ret.insert("global_i64", Extern::Global(g)); let ty = GlobalType::new(ValType::F32, Mutability::Const); - let g = Global::new(store, ty, Val::F32(0x4426_8000)); + let g = Global::new(store, ty, Val::F32(0x4426_8000)).unwrap(); ret.insert("global_f32", Extern::Global(g)); let ty = GlobalType::new(ValType::F64, Mutability::Const); - let g = Global::new(store, ty, Val::F64(0x4084_d000_0000_0000)); + let g = Global::new(store, ty, Val::F64(0x4084_d000_0000_0000)).unwrap(); ret.insert("global_f64", Extern::Global(g)); let ty = TableType::new(ValType::FuncRef, Limits::new(10, Some(20))); - let table = Table::new(store, ty, Val::AnyRef(AnyRef::Null)); + let table = Table::new(store, ty, Val::AnyRef(AnyRef::Null)).unwrap(); ret.insert("table", Extern::Table(table)); let ty = MemoryType::new(Limits::new(1, Some(2)));