From 18a355e0926ffcaf7bb88aa878a644d0e3110f8e Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 28 Sep 2021 12:21:13 -0700 Subject: [PATCH 01/29] give sychronous ResourceLimiter an async alternative --- Cargo.lock | 6 +- Cargo.toml | 1 + crates/runtime/src/instance.rs | 89 +----------- crates/runtime/src/instance/allocator.rs | 78 ++++++---- .../runtime/src/instance/allocator/pooling.rs | 25 ++-- crates/runtime/src/lib.rs | 17 ++- crates/runtime/src/memory.rs | 67 +++------ crates/runtime/src/table.rs | 35 ++--- crates/wasmtime/Cargo.toml | 3 +- crates/wasmtime/src/externals.rs | 2 +- crates/wasmtime/src/instance.rs | 4 +- crates/wasmtime/src/limits.rs | 123 +++++++++++++++- crates/wasmtime/src/memory.rs | 2 +- crates/wasmtime/src/store.rs | 133 ++++++++++++++++-- crates/wasmtime/src/trampoline.rs | 4 +- crates/wasmtime/src/trampoline/func.rs | 5 +- tests/all/limits.rs | 3 - tests/all/memory.rs | 10 +- tests/rlimited-memory.rs | 2 - 19 files changed, 373 insertions(+), 236 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8c73ff9549..5631a2d6c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -108,9 +108,9 @@ dependencies = [ [[package]] name = "async-trait" -version = "0.1.50" +version = "0.1.51" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b98e84bbb4cbcdd97da190ba0c58a1bb0de2c1fdf67d159e192ed766aeca722" +checksum = "44318e776df68115a881de9a8fd1b9e53368d7a4a5ce4cc48517da3393233a5e" dependencies = [ "proc-macro2", "quote", @@ -3362,6 +3362,7 @@ name = "wasmtime" version = "0.30.0" dependencies = [ "anyhow", + "async-trait", "backtrace", "bincode", "cfg-if 1.0.0", @@ -3459,6 +3460,7 @@ name = "wasmtime-cli" version = "0.30.0" dependencies = [ "anyhow", + "async-trait", "criterion", "env_logger 0.8.3", "file-per-thread-logger", diff --git a/Cargo.toml b/Cargo.toml index 0fa869d330..a5d19c3c88 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ criterion = "0.3.4" num_cpus = "1.13.0" winapi = { version = "0.3.9", features = ['memoryapi'] } memchr = "2.4" +async-trait = "0.1" [build-dependencies] anyhow = "1.0.19" diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index f69b17a8ae..3bff9803ea 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -12,7 +12,6 @@ use crate::vmcontext::{ VMInterrupts, VMMemoryDefinition, VMMemoryImport, VMTableDefinition, VMTableImport, }; use crate::{ExportFunction, ExportGlobal, ExportMemory, ExportTable, Store}; -use anyhow::Error; use memoffset::offset_of; use more_asserts::assert_lt; use std::alloc::Layout; @@ -33,86 +32,6 @@ mod allocator; pub use allocator::*; -/// Value returned by [`ResourceLimiter::instances`] default method -pub const DEFAULT_INSTANCE_LIMIT: usize = 10000; -/// Value returned by [`ResourceLimiter::tables`] default method -pub const DEFAULT_TABLE_LIMIT: usize = 10000; -/// Value returned by [`ResourceLimiter::memories`] default method -pub const DEFAULT_MEMORY_LIMIT: usize = 10000; - -/// Used by hosts to limit resource consumption of instances. -/// -/// An instance can be created with a resource limiter so that hosts can take into account -/// non-WebAssembly resource usage to determine if a linear memory or table should grow. -pub trait ResourceLimiter { - /// Notifies the resource limiter that an instance's linear memory has been - /// requested to grow. - /// - /// * `current` is the current size of the linear memory in bytes. - /// * `desired` is the desired size of the linear memory in bytes. - /// * `maximum` is either the linear memory's maximum or a maximum from an - /// instance allocator, also in bytes. A value of `None` - /// indicates that the linear memory is unbounded. - /// - /// This function should return `true` to indicate that the growing - /// operation is permitted or `false` if not permitted. Returning `true` - /// when a maximum has been exceeded will have no effect as the linear - /// memory will not grow. - /// - /// This function is not guaranteed to be invoked for all requests to - /// `memory.grow`. Requests where the allocation requested size doesn't fit - /// in `usize` or exceeds the memory's listed maximum size may not invoke - /// this method. - fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option) -> bool; - - /// Notifies the resource limiter that growing a linear memory, permitted by - /// the `memory_growing` method, has failed. - /// - /// Reasons for failure include: the growth exceeds the `maximum` passed to - /// `memory_growing`, or the operating system failed to allocate additional - /// memory. In that case, `error` might be downcastable to a `std::io::Error`. - fn memory_grow_failed(&mut self, _error: &Error) {} - - /// Notifies the resource limiter that an instance's table has been requested to grow. - /// - /// * `current` is the current number of elements in the table. - /// * `desired` is the desired number of elements in the table. - /// * `maximum` is either the table's maximum or a maximum from an instance allocator. - /// A value of `None` indicates that the table is unbounded. - /// - /// This function should return `true` to indicate that the growing operation is permitted or - /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no - /// effect as the table will not grow. - fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; - - /// The maximum number of instances that can be created for a `Store`. - /// - /// Module instantiation will fail if this limit is exceeded. - /// - /// This value defaults to 10,000. - fn instances(&self) -> usize { - DEFAULT_INSTANCE_LIMIT - } - - /// The maximum number of tables that can be created for a `Store`. - /// - /// Module instantiation will fail if this limit is exceeded. - /// - /// This value defaults to 10,000. - fn tables(&self) -> usize { - DEFAULT_TABLE_LIMIT - } - - /// The maximum number of linear memories that can be created for a `Store` - /// - /// Instantiation will fail with an error if this limit is exceeded. - /// - /// This value defaults to 10,000. - fn memories(&self) -> usize { - DEFAULT_MEMORY_LIMIT - } -} - /// A type that roughly corresponds to a WebAssembly instance, but is also used /// for host-defined objects. /// @@ -441,10 +360,10 @@ impl Instance { (foreign_memory_index, foreign_instance) } }; - let limiter = unsafe { (*instance.store()).limiter() }; + let store = unsafe { &mut *instance.store() }; let memory = &mut instance.memories[idx]; - let result = unsafe { memory.grow(delta, limiter) }; + let result = unsafe { memory.grow(delta, store) }; let vmmemory = memory.vmmemory(); // Update the state used by wasm code in case the base pointer and/or @@ -480,13 +399,13 @@ impl Instance { delta: u32, init_value: TableElement, ) -> Option { - let limiter = unsafe { (*self.store()).limiter() }; + let store = unsafe { &mut *self.store() }; let table = self .tables .get_mut(table_index) .unwrap_or_else(|| panic!("no table for index {}", table_index.index())); - let result = unsafe { table.grow(delta, init_value, limiter) }; + let result = unsafe { table.grow(delta, init_value, store) }; // Keep the `VMContext` pointers used by compiled Wasm code up to // date. diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index 0caa891c27..0826eecd8b 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -1,5 +1,5 @@ use crate::imports::Imports; -use crate::instance::{Instance, InstanceHandle, ResourceLimiter, RuntimeMemoryCreator}; +use crate::instance::{Instance, InstanceHandle, RuntimeMemoryCreator}; use crate::memory::{DefaultMemoryCreator, Memory}; use crate::table::Table; use crate::traphandlers::Trap; @@ -58,13 +58,13 @@ pub struct InstanceAllocationRequest<'a> { /// are a bit of a lie. This is done purely so a store can learn about /// itself when it gets called as a host function, and additionally so this /// runtime can access internals as necessary (such as the - /// VMExternRefActivationsTable or the ResourceLimiter). + /// VMExternRefActivationsTable or the resource limiter methods). /// /// Note that this ends up being a self-pointer to the instance when stored. /// The reason is that the instance itself is then stored within the store. /// We use a number of `PhantomPinned` declarations to indicate this to the /// compiler. More info on this in `wasmtime/src/store.rs` - pub store: Option<*mut dyn Store>, + pub store: StorePtr, /// A list of all wasm data that can be referenced by the module that /// will be allocated. The `Module` given here has active/passive data @@ -77,6 +77,37 @@ pub struct InstanceAllocationRequest<'a> { pub wasm_data: *const [u8], } +/// A pointer to a Store. This Option<*mut dyn Store> is wrapped in a struct +/// so that the function to create a &mut dyn Store is a method on a member of +/// InstanceAllocationRequest, rather than on a &mut InstanceAllocationRequest +/// itself, because several use-sites require a split mut borrow on the +/// InstanceAllocationRequest. +pub struct StorePtr(Option<*mut dyn Store>); +impl StorePtr { + /// A pointer to no Store. + pub fn empty() -> Self { + Self(None) + } + /// A pointer to a Store. + pub fn new(ptr: *mut dyn Store) -> Self { + Self(Some(ptr)) + } + /* + /// Update an empty StorePtr to point to a Store. + pub fn set(&mut self, ptr: *mut dyn Store) { + self.0 = Some(ptr) + } + */ + /// Use the StorePtr as a mut ref to the Store. + // XXX should this be an unsafe fn? is it always safe at a use site? + pub(crate) fn get(&mut self) -> Option<&mut dyn Store> { + match self.0 { + Some(ptr) => Some(unsafe { &mut *ptr }), + None => None, + } + } +} + /// An link error while instantiating a module. #[derive(Error, Debug)] #[error("Link error: {0}")] @@ -430,7 +461,7 @@ fn initialize_instance( } unsafe fn initialize_vmcontext(instance: &mut Instance, req: InstanceAllocationRequest) { - if let Some(store) = req.store { + if let Some(store) = req.store.0 { *instance.interrupts() = (*store).vminterrupts(); *instance.externref_activations_table() = (*store).externref_activations_table().0; instance.set_store(store); @@ -581,17 +612,6 @@ pub struct OnDemandInstanceAllocator { stack_size: usize, } -// rustc is quite strict with the lifetimes when dealing with mutable borrows, -// so this is a little helper to get a shorter lifetime on `Option<&mut T>` -fn borrow_limiter<'a>( - limiter: &'a mut Option<&mut dyn ResourceLimiter>, -) -> Option<&'a mut dyn ResourceLimiter> { - match limiter { - Some(limiter) => Some(&mut **limiter), - None => None, - } -} - impl OnDemandInstanceAllocator { /// Creates a new on-demand instance allocator. pub fn new(mem_creator: Option>, stack_size: usize) -> Self { @@ -605,15 +625,20 @@ impl OnDemandInstanceAllocator { fn create_tables( module: &Module, - mut limiter: Option<&mut dyn ResourceLimiter>, + store: &mut StorePtr, ) -> Result, InstantiationError> { let num_imports = module.num_imported_tables; let mut tables: PrimaryMap = PrimaryMap::with_capacity(module.table_plans.len() - num_imports); for table in &module.table_plans.values().as_slice()[num_imports..] { tables.push( - Table::new_dynamic(table, borrow_limiter(&mut limiter)) - .map_err(InstantiationError::Resource)?, + Table::new_dynamic( + table, + store + .get() + .expect("if module has table plans, store is not empty"), + ) + .map_err(InstantiationError::Resource)?, ); } Ok(tables) @@ -622,7 +647,7 @@ impl OnDemandInstanceAllocator { fn create_memories( &self, module: &Module, - mut limiter: Option<&mut dyn ResourceLimiter>, + store: &mut StorePtr, ) -> Result, InstantiationError> { let creator = self .mem_creator @@ -633,8 +658,14 @@ impl OnDemandInstanceAllocator { PrimaryMap::with_capacity(module.memory_plans.len() - num_imports); for plan in &module.memory_plans.values().as_slice()[num_imports..] { memories.push( - Memory::new_dynamic(plan, creator, borrow_limiter(&mut limiter)) - .map_err(InstantiationError::Resource)?, + Memory::new_dynamic( + plan, + creator, + store + .get() + .expect("if module has memory plans, store is not empty"), + ) + .map_err(InstantiationError::Resource)?, ); } Ok(memories) @@ -656,9 +687,8 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator { &self, mut req: InstanceAllocationRequest, ) -> Result { - let mut limiter = req.store.and_then(|s| (*s).limiter()); - let memories = self.create_memories(&req.module, borrow_limiter(&mut limiter))?; - let tables = Self::create_tables(&req.module, borrow_limiter(&mut limiter))?; + let memories = self.create_memories(&req.module, &mut req.store)?; + let tables = Self::create_tables(&req.module, &mut req.store)?; let host_state = std::mem::replace(&mut req.host_state, Box::new(())); diff --git a/crates/runtime/src/instance/allocator/pooling.rs b/crates/runtime/src/instance/allocator/pooling.rs index 2d074aa17c..b99986a498 100644 --- a/crates/runtime/src/instance/allocator/pooling.rs +++ b/crates/runtime/src/instance/allocator/pooling.rs @@ -7,10 +7,9 @@ //! Using the pooling instance allocator can speed up module instantiation //! when modules can be constrained based on configurable limits. -use super::borrow_limiter; use super::{ initialize_instance, initialize_vmcontext, InstanceAllocationRequest, InstanceAllocator, - InstanceHandle, InstantiationError, ResourceLimiter, + InstanceHandle, InstantiationError, }; use crate::{instance::Instance, Memory, Mmap, Table, VMContext}; use anyhow::{anyhow, bail, Context, Result}; @@ -385,19 +384,16 @@ impl InstancePool { instance.host_state = std::mem::replace(&mut req.host_state, Box::new(())); instance.wasm_data = &*req.wasm_data; - let mut limiter = req.store.and_then(|s| (*s).limiter()); Self::set_instance_memories( instance, self.memories.get(index), self.memories.max_wasm_pages, - borrow_limiter(&mut limiter), )?; Self::set_instance_tables( instance, self.tables.get(index).map(|x| x as *mut usize), self.tables.max_elements, - borrow_limiter(&mut limiter), )?; initialize_vmcontext(instance, req); @@ -503,7 +499,6 @@ impl InstancePool { instance: &mut Instance, mut memories: impl Iterator, max_pages: u64, - mut limiter: Option<&mut dyn ResourceLimiter>, ) -> Result<(), InstantiationError> { let module = instance.module.as_ref(); @@ -519,12 +514,9 @@ impl InstancePool { ) }; instance.memories.push( - Memory::new_static( - plan, - memory, - commit_memory_pages, - borrow_limiter(&mut limiter), - ) + Memory::new_static(plan, memory, commit_memory_pages, unsafe { + &mut *instance.store() + }) .map_err(InstantiationError::Resource)?, ); } @@ -538,7 +530,6 @@ impl InstancePool { instance: &mut Instance, mut tables: impl Iterator, max_elements: u32, - mut limiter: Option<&mut dyn ResourceLimiter>, ) -> Result<(), InstantiationError> { let module = instance.module.as_ref(); @@ -555,7 +546,7 @@ impl InstancePool { let table = unsafe { std::slice::from_raw_parts_mut(base, max_elements as usize) }; instance.tables.push( - Table::new_static(plan, table, borrow_limiter(&mut limiter)) + Table::new_static(plan, table, unsafe { &mut *instance.store() }) .map_err(InstantiationError::Resource)?, ); } @@ -1052,7 +1043,7 @@ unsafe impl InstanceAllocator for PoolingInstanceAllocator { #[cfg(test)] mod test { use super::*; - use crate::{Imports, VMSharedSignatureIndex}; + use crate::{Imports, StorePtr, VMSharedSignatureIndex}; use wasmtime_environ::{ EntityRef, Global, GlobalInit, Memory, MemoryPlan, ModuleType, SignatureIndex, Table, TablePlan, TableStyle, WasmType, @@ -1414,7 +1405,7 @@ mod test { }, shared_signatures: VMSharedSignatureIndex::default().into(), host_state: Box::new(()), - store: None, + store: StorePtr::empty(), wasm_data: &[], }, ) @@ -1438,7 +1429,7 @@ mod test { }, shared_signatures: VMSharedSignatureIndex::default().into(), host_state: Box::new(()), - store: None, + store: StorePtr::empty(), wasm_data: &[], }, ) { diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 4a2ca4d3a2..f9afeed984 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -42,8 +42,7 @@ pub use crate::imports::Imports; pub use crate::instance::{ InstanceAllocationRequest, InstanceAllocator, InstanceHandle, InstanceLimits, InstantiationError, LinkError, ModuleLimits, OnDemandInstanceAllocator, - PoolingAllocationStrategy, PoolingInstanceAllocator, ResourceLimiter, DEFAULT_INSTANCE_LIMIT, - DEFAULT_MEMORY_LIMIT, DEFAULT_TABLE_LIMIT, + PoolingAllocationStrategy, PoolingInstanceAllocator, StorePtr, }; pub use crate::jit_int::GdbJitImageRegistration; pub use crate::memory::{Memory, RuntimeLinearMemory, RuntimeMemoryCreator}; @@ -91,8 +90,18 @@ pub unsafe trait Store { &mut self, ) -> (&mut VMExternRefActivationsTable, &dyn ModuleInfoLookup); - /// Returns a reference to the store's limiter for limiting resources, if any. - fn limiter(&mut self) -> Option<&mut dyn ResourceLimiter>; + /// Callback invoked to allow the store's resource limiter to reject a memory grow operation. + fn limiter_memory_growing( + &mut self, + current: usize, + desired: usize, + maximum: Option, + ) -> bool; + /// Callback invoked to notify the store's resource limiter that a memory grow operation has + /// failed. + fn limiter_memory_grow_failed(&mut self, error: &anyhow::Error); + /// Callback invoked to allow the store's resource limiter to reject a table grow operation. + fn limiter_table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; /// Callback invoked whenever fuel runs out by a wasm instance. If an error /// is returned that's raised as a trap. Otherwise wasm execution will diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index 0f25c7a1d8..e9dde6689f 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -4,8 +4,8 @@ use crate::mmap::Mmap; use crate::vmcontext::VMMemoryDefinition; -use crate::ResourceLimiter; -use anyhow::{bail, format_err, Error, Result}; +use crate::Store; +use anyhow::{bail, format_err, Result}; use more_asserts::{assert_ge, assert_le}; use std::convert::TryFrom; use wasmtime_environ::{MemoryPlan, MemoryStyle, WASM32_MAX_PAGES, WASM64_MAX_PAGES}; @@ -212,33 +212,14 @@ pub enum Memory { Dynamic(Box), } -fn memory_growing( - limiter: &mut Option<&mut dyn ResourceLimiter>, - current: usize, - desired: usize, - maximum: Option, -) -> bool { - match limiter { - Some(ref mut l) => l.memory_growing(current, desired, maximum), - None => true, - } -} - -fn memory_grow_failed(limiter: &mut Option<&mut dyn ResourceLimiter>, error: &Error) { - match limiter { - Some(l) => l.memory_grow_failed(error), - None => {} - } -} - impl Memory { /// Create a new dynamic (movable) memory instance for the specified plan. pub fn new_dynamic( plan: &MemoryPlan, creator: &dyn RuntimeMemoryCreator, - limiter: Option<&mut dyn ResourceLimiter>, + store: &mut dyn Store, ) -> Result { - let (minimum, maximum) = Self::limit_new(plan, limiter)?; + let (minimum, maximum) = Self::limit_new(plan, store)?; Ok(Memory::Dynamic(creator.new_memory(plan, minimum, maximum)?)) } @@ -247,9 +228,9 @@ impl Memory { plan: &MemoryPlan, base: &'static mut [u8], make_accessible: fn(*mut u8, usize) -> Result<()>, - limiter: Option<&mut dyn ResourceLimiter>, + store: &mut dyn Store, ) -> Result { - let (minimum, maximum) = Self::limit_new(plan, limiter)?; + let (minimum, maximum) = Self::limit_new(plan, store)?; let base = match maximum { Some(max) if max < base.len() => &mut base[..max], @@ -269,15 +250,11 @@ impl Memory { }) } - /// Calls the `limiter`, if specified, to optionally prevent a memory from - /// being allocated. + /// Calls the `store`'s limiter to optionally prevent a memory from being allocated. /// /// Returns the minimum size and optional maximum size of the memory, in /// bytes. - fn limit_new( - plan: &MemoryPlan, - mut limiter: Option<&mut dyn ResourceLimiter>, - ) -> Result<(usize, Option)> { + fn limit_new(plan: &MemoryPlan, store: &mut dyn Store) -> Result<(usize, Option)> { // Sanity-check what should already be true from wasm module validation. let absolute_max = if plan.memory.memory64 { WASM64_MAX_PAGES @@ -291,7 +268,7 @@ impl Memory { // allocate, which is our entire address space minus a wasm page. That // shouldn't ever actually work in terms of an allocation because // presumably the kernel wants *something* for itself, but this is used - // to pass to the `limiter` specified, if present, for a requested size + // to pass to the `store`'s limiter for a requested size // to approximate the scale of the request that the wasm module is // making. This is necessary because the limiter works on `usize` bytes // whereas we're working with possibly-overflowing `u64` calculations @@ -302,7 +279,7 @@ impl Memory { // If the minimum memory size overflows the size of our own address // space, then we can't satisfy this request, but defer the error to - // later so the `limiter` can be informed that an effective oom is + // later so the `store` can be informed that an effective oom is // happening. let minimum = plan .memory @@ -332,13 +309,13 @@ impl Memory { maximum = usize::try_from(1u64 << 32).ok(); } - // Inform the limiter what's about to happen. This will let the limiter + // Inform the store's limiter what's about to happen. This will let the limiter // reject anything if necessary, and this also guarantees that we should // call the limiter for all requested memories, even if our `minimum` // calculation overflowed. This means that the `minimum` we're informing // the limiter is lossy and may not be 100% accurate, but for now the - // expected uses of `limiter` means that's ok. - if !memory_growing(&mut limiter, 0, minimum.unwrap_or(absolute_max), maximum) { + // expected uses of limiter means that's ok. + if !store.limiter_memory_growing(0, minimum.unwrap_or(absolute_max), maximum) { bail!( "memory minimum size of {} pages exceeds memory limits", plan.memory.minimum @@ -400,11 +377,7 @@ impl Memory { /// /// Generally, prefer using `InstanceHandle::memory_grow`, which encapsulates /// this unsafety. - pub unsafe fn grow( - &mut self, - delta_pages: u64, - mut limiter: Option<&mut dyn ResourceLimiter>, - ) -> Option { + pub unsafe fn grow(&mut self, delta_pages: u64, store: &mut dyn Store) -> Option { let old_byte_size = self.byte_size(); // Wasm spec: when growing by 0 pages, always return the current size. if delta_pages == 0 { @@ -428,15 +401,15 @@ impl Memory { }; let maximum = self.maximum_byte_size(); - // Limiter gets first chance to reject memory_growing. - if !memory_growing(&mut limiter, old_byte_size, new_byte_size, maximum) { + // Store limiter gets first chance to reject memory_growing. + if !store.limiter_memory_growing(old_byte_size, new_byte_size, maximum) { return None; } // Never exceed maximum, even if limiter permitted it. if let Some(max) = maximum { if new_byte_size > max { - memory_grow_failed(&mut limiter, &format_err!("Memory maximum size exceeded")); + store.limiter_memory_grow_failed(&format_err!("Memory maximum size exceeded")); return None; } } @@ -458,7 +431,7 @@ impl Memory { } => { // Never exceed static memory size if new_byte_size > base.len() { - memory_grow_failed(&mut limiter, &format_err!("static memory size exceeded")); + store.limiter_memory_grow_failed(&format_err!("static memory size exceeded")); return None; } @@ -467,13 +440,13 @@ impl Memory { base.as_mut_ptr().add(old_byte_size), new_byte_size - old_byte_size, ); - r.map_err(|e| memory_grow_failed(&mut limiter, &e)).ok()?; + r.map_err(|e| store.limiter_memory_grow_failed(&e)).ok()?; *size = new_byte_size; } Memory::Dynamic(mem) => { let r = mem.grow_to(new_byte_size); - r.map_err(|e| memory_grow_failed(&mut limiter, &e)).ok()?; + r.map_err(|e| store.limiter_memory_grow_failed(&e)).ok()?; } } Some(old_byte_size) diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index c6e977d308..690a1ed82a 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -3,7 +3,7 @@ //! `Table` is to WebAssembly tables what `LinearMemory` is to WebAssembly linear memories. use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition}; -use crate::{ResourceLimiter, Trap, VMExternRef}; +use crate::{Store, Trap, VMExternRef}; use anyhow::{bail, Result}; use std::convert::{TryFrom, TryInto}; use std::ops::Range; @@ -137,11 +137,8 @@ fn wasm_to_table_type(ty: WasmType) -> Result { impl Table { /// Create a new dynamic (movable) table instance for the specified table plan. - pub fn new_dynamic( - plan: &TablePlan, - limiter: Option<&mut dyn ResourceLimiter>, - ) -> Result { - Self::limit_new(plan, limiter)?; + pub fn new_dynamic(plan: &TablePlan, store: &mut dyn Store) -> Result { + Self::limit_new(plan, store)?; let elements = vec![0; plan.table.minimum as usize]; let ty = wasm_to_table_type(plan.table.wasm_ty)?; let maximum = plan.table.maximum; @@ -157,9 +154,9 @@ impl Table { pub fn new_static( plan: &TablePlan, data: &'static mut [usize], - limiter: Option<&mut dyn ResourceLimiter>, + store: &mut dyn Store, ) -> Result { - Self::limit_new(plan, limiter)?; + Self::limit_new(plan, store)?; let size = plan.table.minimum; let ty = wasm_to_table_type(plan.table.wasm_ty)?; let data = match plan.table.maximum { @@ -170,14 +167,12 @@ impl Table { Ok(Table::Static { data, size, ty }) } - fn limit_new(plan: &TablePlan, limiter: Option<&mut dyn ResourceLimiter>) -> Result<()> { - if let Some(limiter) = limiter { - if !limiter.table_growing(0, plan.table.minimum, plan.table.maximum) { - bail!( - "table minimum size of {} elements exceeds table limits", - plan.table.minimum - ); - } + fn limit_new(plan: &TablePlan, store: &mut dyn Store) -> Result<()> { + if !store.limiter_table_growing(0, plan.table.minimum, plan.table.maximum) { + bail!( + "table minimum size of {} elements exceeds table limits", + plan.table.minimum + ); } Ok(()) } @@ -292,15 +287,13 @@ impl Table { &mut self, delta: u32, init_value: TableElement, - limiter: Option<&mut dyn ResourceLimiter>, + store: &mut dyn Store, ) -> Option { let old_size = self.size(); let new_size = old_size.checked_add(delta)?; - if let Some(limiter) = limiter { - if !limiter.table_growing(old_size, new_size, self.maximum()) { - return None; - } + if !store.limiter_table_growing(old_size, new_size, self.maximum()) { + return None; } if let Some(max) = self.maximum() { diff --git a/crates/wasmtime/Cargo.toml b/crates/wasmtime/Cargo.toml index 188920241f..daa9edadaf 100644 --- a/crates/wasmtime/Cargo.toml +++ b/crates/wasmtime/Cargo.toml @@ -38,6 +38,7 @@ psm = "0.1.11" lazy_static = "1.4" rayon = { version = "1.0", optional = true } object = { version = "0.27", default-features = false, features = ['read_core', 'elf'] } +async-trait = { version = "0.1.51", optional = true } [target.'cfg(target_os = "windows")'.dependencies] winapi = "0.3.7" @@ -73,7 +74,7 @@ cache = ["wasmtime-cache"] # Enables support for "async stores" as well as defining host functions as # `async fn` and calling functions asynchronously. -async = ["wasmtime-fiber", "wasmtime-runtime/async"] +async = ["wasmtime-fiber", "wasmtime-runtime/async", "async-trait"] # Enables userfaultfd support in the runtime's pooling allocator when building on Linux uffd = ["wasmtime-runtime/uffd"] diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 6324bac13e..8f3d7e1645 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -551,7 +551,7 @@ impl Table { let init = init.into_table_element(store, ty)?; let table = self.wasmtime_table(store); unsafe { - match (*table).grow(delta, init, store.limiter()) { + match (*table).grow(delta, init, store) { Some(size) => { let vm = (*table).vmtable(); *store[self.0].definition = vm; diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index bd7afcfba8..e11971739a 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -15,7 +15,7 @@ use wasmtime_environ::{ }; use wasmtime_jit::TypeTables; use wasmtime_runtime::{ - Imports, InstanceAllocationRequest, InstantiationError, VMContext, VMFunctionBody, + Imports, InstanceAllocationRequest, InstantiationError, StorePtr, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport, }; @@ -734,7 +734,7 @@ impl<'a> Instantiator<'a> { imports: self.cur.build(), shared_signatures: self.cur.module.signatures().as_module_map().into(), host_state: Box::new(Instance(instance_to_be)), - store: Some(store.traitobj()), + store: StorePtr::new(store.traitobj()), wasm_data: compiled_module.wasm_data(), })?; diff --git a/crates/wasmtime/src/limits.rs b/crates/wasmtime/src/limits.rs index 6f9ca1e23e..acca7dad4e 100644 --- a/crates/wasmtime/src/limits.rs +++ b/crates/wasmtime/src/limits.rs @@ -1,4 +1,118 @@ -pub use wasmtime_runtime::ResourceLimiter; +/// Value returned by [`ResourceLimiter::instances`] default method +pub const DEFAULT_INSTANCE_LIMIT: usize = 10000; +/// Value returned by [`ResourceLimiter::tables`] default method +pub const DEFAULT_TABLE_LIMIT: usize = 10000; +/// Value returned by [`ResourceLimiter::memories`] default method +pub const DEFAULT_MEMORY_LIMIT: usize = 10000; + +/// Used by hosts to limit resource consumption of instances. +/// +/// An instance can be created with a resource limiter so that hosts can take into account +/// non-WebAssembly resource usage to determine if a linear memory or table should grow. +pub trait ResourceLimiter { + /// Notifies the resource limiter that an instance's linear memory has been + /// requested to grow. + /// + /// * `current` is the current size of the linear memory in bytes. + /// * `desired` is the desired size of the linear memory in bytes. + /// * `maximum` is either the linear memory's maximum or a maximum from an + /// instance allocator, also in bytes. A value of `None` + /// indicates that the linear memory is unbounded. + /// + /// This function should return `true` to indicate that the growing + /// operation is permitted or `false` if not permitted. Returning `true` + /// when a maximum has been exceeded will have no effect as the linear + /// memory will not grow. + /// + /// This function is not guaranteed to be invoked for all requests to + /// `memory.grow`. Requests where the allocation requested size doesn't fit + /// in `usize` or exceeds the memory's listed maximum size may not invoke + /// this method. + fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option) -> bool; + + /// Notifies the resource limiter that growing a linear memory, permitted by + /// the `memory_growing` method, has failed. + /// + /// Reasons for failure include: the growth exceeds the `maximum` passed to + /// `memory_growing`, or the operating system failed to allocate additional + /// memory. In that case, `error` might be downcastable to a `std::io::Error`. + fn memory_grow_failed(&mut self, _error: &anyhow::Error) {} + + /// Notifies the resource limiter that an instance's table has been requested to grow. + /// + /// * `current` is the current number of elements in the table. + /// * `desired` is the desired number of elements in the table. + /// * `maximum` is either the table's maximum or a maximum from an instance allocator. + /// A value of `None` indicates that the table is unbounded. + /// + /// This function should return `true` to indicate that the growing operation is permitted or + /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no + /// effect as the table will not grow. + fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; + + /// The maximum number of instances that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + /// + /// This value defaults to 10,000. + fn instances(&self) -> usize { + DEFAULT_INSTANCE_LIMIT + } + + /// The maximum number of tables that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + /// + /// This value defaults to 10,000. + fn tables(&self) -> usize { + DEFAULT_TABLE_LIMIT + } + + /// The maximum number of linear memories that can be created for a `Store` + /// + /// Instantiation will fail with an error if this limit is exceeded. + /// + /// This value defaults to 10,000. + fn memories(&self) -> usize { + DEFAULT_MEMORY_LIMIT + } +} + +#[cfg(feature = "async")] +/// Used by hosts to limit resource consumption of instances. +/// Identical to [`ResourceLimiter`], except that the `memory_growing` and `table_growing` +/// functions are async. Must be used with an async [`Store`]. +#[async_trait::async_trait] +pub trait ResourceLimiterAsync { + /// Async version of [`ResourceLimiter::memory_growing`] + async fn memory_growing( + &mut self, + current: usize, + desired: usize, + maximum: Option, + ) -> bool; + + /// Identical to [`ResourceLimiter::memory_grow_failed`] + fn memory_grow_failed(&mut self, error: &anyhow::Error); + + /// Asynchronous version of [`ResourceLimiter::table_growing`] + async fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; + + /// Identical to [`ResourceLimiter::instances`]` + fn instances(&self) -> usize { + DEFAULT_INSTANCE_LIMIT + } + + /// Identical to [`ResourceLimiter::tables`]` + fn tables(&self) -> usize { + DEFAULT_TABLE_LIMIT + } + + /// Identical to [`ResourceLimiter::memories`]` + fn memories(&self) -> usize { + DEFAULT_MEMORY_LIMIT + } +} /// Used to build [`StoreLimits`]. pub struct StoreLimitsBuilder(StoreLimits); @@ -79,13 +193,14 @@ impl Default for StoreLimits { Self { memory_size: None, table_elements: None, - instances: wasmtime_runtime::DEFAULT_INSTANCE_LIMIT, - tables: wasmtime_runtime::DEFAULT_TABLE_LIMIT, - memories: wasmtime_runtime::DEFAULT_MEMORY_LIMIT, + instances: DEFAULT_INSTANCE_LIMIT, + tables: DEFAULT_TABLE_LIMIT, + memories: DEFAULT_MEMORY_LIMIT, } } } +#[cfg_attr(feature = "async", async_trait::async_trait)] impl ResourceLimiter for StoreLimits { fn memory_growing(&mut self, _current: usize, desired: usize, _maximum: Option) -> bool { match self.memory_size { diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index a1b305118b..1d04a6edad 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -461,7 +461,7 @@ impl Memory { let store = store.as_context_mut().0; let mem = self.wasmtime_memory(store); unsafe { - match (*mem).grow(delta, store.limiter()) { + match (*mem).grow(delta, store) { Some(size) => { let vm = (*mem).vmmemory(); *store[self.0].definition = vm; diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 365574bd11..4a72c18959 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -93,8 +93,8 @@ use std::sync::Arc; use std::task::{Context, Poll}; use wasmtime_runtime::{ InstanceAllocationRequest, InstanceAllocator, InstanceHandle, ModuleInfo, - OnDemandInstanceAllocator, SignalHandler, VMCallerCheckedAnyfunc, VMContext, VMExternRef, - VMExternRefActivationsTable, VMInterrupts, VMSharedSignatureIndex, VMTrampoline, + OnDemandInstanceAllocator, SignalHandler, StorePtr, VMCallerCheckedAnyfunc, VMContext, + VMExternRef, VMExternRefActivationsTable, VMInterrupts, VMSharedSignatureIndex, VMTrampoline, }; mod context; @@ -197,12 +197,18 @@ pub struct StoreInner { /// Generic metadata about the store that doesn't need access to `T`. inner: StoreOpaque, - limiter: Option &mut (dyn crate::ResourceLimiter) + Send + Sync>>, + limiter: Option>, call_hook: Option Result<(), crate::Trap> + Send + Sync>>, // for comments about `ManuallyDrop`, see `Store::into_data` data: ManuallyDrop, } +enum ResourceLimiterInner { + Sync(Box &mut (dyn crate::ResourceLimiter) + Send + Sync>), + #[cfg(feature = "async")] + Async(Box &mut (dyn crate::ResourceLimiterAsync) + Send + Sync>), +} + // Forward methods on `StoreOpaque` to also being on `StoreInner` impl Deref for StoreInner { type Target = StoreOpaque; @@ -402,7 +408,7 @@ impl Store { shared_signatures: None.into(), imports: Default::default(), module: Arc::new(wasmtime_environ::Module::default()), - store: None, + store: StorePtr::empty(), wasm_data: &[], }) .expect("failed to allocate default callee") @@ -418,11 +424,11 @@ impl Store { modules: ModuleRegistry::default(), host_trampolines: HashMap::default(), instance_count: 0, - instance_limit: wasmtime_runtime::DEFAULT_INSTANCE_LIMIT, + instance_limit: crate::DEFAULT_INSTANCE_LIMIT, memory_count: 0, - memory_limit: wasmtime_runtime::DEFAULT_MEMORY_LIMIT, + memory_limit: crate::DEFAULT_MEMORY_LIMIT, table_count: 0, - table_limit: wasmtime_runtime::DEFAULT_TABLE_LIMIT, + table_limit: crate::DEFAULT_TABLE_LIMIT, fuel_adj: 0, #[cfg(feature = "async")] async_state: AsyncState { @@ -525,7 +531,36 @@ impl Store { innermost.memory_limit = memory_limit; // Save the limiter accessor function: - inner.limiter = Some(Box::new(limiter)); + inner.limiter = Some(ResourceLimiterInner::Sync(Box::new(limiter))); + } + + /// Configures the [`ResourceLimiterAsync`](crate::ResourceLimiterAsync) used to limit + /// resource creation within this [`Store`]. Must be used with an async `Store`!. + /// + /// Note that this limiter is only used to limit the creation/growth of + /// resources in the future, this does not retroactively attempt to apply + /// limits to the [`Store`]. + pub fn limiter_async( + &mut self, + mut limiter: impl FnMut(&mut T) -> &mut (dyn crate::ResourceLimiterAsync) + + Send + + Sync + + 'static, + ) { + debug_assert!(self.inner.async_support()); + // Apply the limits on instances, tables, and memory given by the limiter: + let inner = &mut self.inner; + let (instance_limit, table_limit, memory_limit) = { + let l = limiter(&mut inner.data); + (l.instances(), l.tables(), l.memories()) + }; + let innermost = &mut inner.inner; + innermost.instance_limit = instance_limit; + innermost.table_limit = table_limit; + innermost.memory_limit = memory_limit; + + // Save the limiter accessor function: + inner.limiter = Some(ResourceLimiterInner::Async(Box::new(limiter))); } /// Configure a function that runs on calls and returns between WebAssembly @@ -872,11 +907,6 @@ impl StoreInner { &mut self.data } - pub fn limiter(&mut self) -> Option<&mut dyn crate::limits::ResourceLimiter> { - let accessor = self.limiter.as_mut()?; - Some(accessor(&mut self.data)) - } - pub fn call_hook(&mut self, s: CallHook) -> Result<(), Trap> { if let Some(hook) = &mut self.call_hook { hook(&mut self.data, s) @@ -1496,8 +1526,81 @@ unsafe impl wasmtime_runtime::Store for StoreInner { (&mut inner.externref_activations_table, &inner.modules) } - fn limiter(&mut self) -> Option<&mut dyn wasmtime_runtime::ResourceLimiter> { - ::limiter(self) + fn limiter_memory_growing( + &mut self, + current: usize, + desired: usize, + maximum: Option, + ) -> bool { + // Need to borrow async_cx before the mut borrow of the limiter. + // self.async_cx() panicks when used with a non-async store, so + // wrap this in an option. + #[cfg(feature = "async")] + let async_cx = if self.async_support() { + Some(self.async_cx()) + } else { + None + }; + match self.limiter { + Some(ResourceLimiterInner::Sync(ref mut limiter)) => { + limiter(&mut self.data).memory_growing(current, desired, maximum) + } + #[cfg(feature = "async")] + Some(ResourceLimiterInner::Async(ref mut limiter)) => unsafe { + async_cx + .expect("ResourceLimiterAsync requires async Store") + .block_on( + limiter(&mut self.data) + .memory_growing(current, desired, maximum) + .as_mut(), + ) + .expect("FIXME idk how to deal with a trap here!") + }, + None => true, + } + } + + fn limiter_memory_grow_failed(&mut self, error: &anyhow::Error) { + match self.limiter { + Some(ResourceLimiterInner::Sync(ref mut limiter)) => { + limiter(&mut self.data).memory_grow_failed(error) + } + #[cfg(feature = "async")] + Some(ResourceLimiterInner::Async(ref mut limiter)) => { + limiter(&mut self.data).memory_grow_failed(error) + } + None => {} + } + } + + fn limiter_table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool { + // Need to borrow async_cx before the mut borrow of the limiter. + // self.async_cx() panicks when used with a non-async store, so + // wrap this in an option. + #[cfg(feature = "async")] + let async_cx = if self.async_support() { + Some(self.async_cx()) + } else { + None + }; + + match self.limiter { + Some(ResourceLimiterInner::Sync(ref mut limiter)) => { + limiter(&mut self.data).table_growing(current, desired, maximum) + } + #[cfg(feature = "async")] + Some(ResourceLimiterInner::Async(ref mut limiter)) => unsafe { + async_cx + .expect("ResourceLimiterAsync requires async Store") + .block_on( + limiter(&mut self.data) + .table_growing(current, desired, maximum) + .as_mut(), + ) + .expect("FIXME idk how to deal with a trap here!") + }, + None => true, + } } fn out_of_gas(&mut self) -> Result<(), Box> { diff --git a/crates/wasmtime/src/trampoline.rs b/crates/wasmtime/src/trampoline.rs index 4452d7b42a..c1f8038a5a 100644 --- a/crates/wasmtime/src/trampoline.rs +++ b/crates/wasmtime/src/trampoline.rs @@ -18,7 +18,7 @@ use std::any::Any; use std::sync::Arc; use wasmtime_environ::{EntityIndex, GlobalIndex, MemoryIndex, Module, TableIndex}; use wasmtime_runtime::{ - Imports, InstanceAllocationRequest, InstanceAllocator, OnDemandInstanceAllocator, + Imports, InstanceAllocationRequest, InstanceAllocator, OnDemandInstanceAllocator, StorePtr, VMFunctionImport, VMSharedSignatureIndex, }; @@ -46,7 +46,7 @@ fn create_handle( imports, shared_signatures: shared_signature_id.into(), host_state, - store: Some(store.traitobj()), + store: StorePtr::new(store.traitobj()), wasm_data: &[], }, )?; diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index 5c2e2a1b84..e37803763f 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -9,7 +9,8 @@ use wasmtime_environ::{EntityIndex, Module, ModuleType, PrimaryMap, SignatureInd use wasmtime_jit::{CodeMemory, MmapVec}; use wasmtime_runtime::{ Imports, InstanceAllocationRequest, InstanceAllocator, InstanceHandle, - OnDemandInstanceAllocator, VMContext, VMFunctionBody, VMSharedSignatureIndex, VMTrampoline, + OnDemandInstanceAllocator, StorePtr, VMContext, VMFunctionBody, VMSharedSignatureIndex, + VMTrampoline, }; struct TrampolineState { @@ -131,7 +132,7 @@ pub unsafe fn create_raw_function( imports: Imports::default(), shared_signatures: sig.into(), host_state, - store: None, + store: StorePtr::empty(), wasm_data: &[], })?, ) diff --git a/tests/all/limits.rs b/tests/all/limits.rs index c6490c05a8..7e1b303906 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -300,7 +300,6 @@ impl ResourceLimiter for MemoryContext { self.wasm_memory_used = desired; true } - fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { true } @@ -401,11 +400,9 @@ impl ResourceLimiter for MemoryGrowFailureDetector { self.desired = desired; true } - fn memory_grow_failed(&mut self, err: &anyhow::Error) { self.error = Some(err.to_string()); } - fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { true } diff --git a/tests/all/memory.rs b/tests/all/memory.rs index e2d823508b..c212f50a5a 100644 --- a/tests/all/memory.rs +++ b/tests/all/memory.rs @@ -311,12 +311,16 @@ fn massive_64_bit_still_limited() -> Result<()> { } impl ResourceLimiter for MyLimiter { - fn memory_growing(&mut self, _request: usize, _min: usize, _max: Option) -> bool { + fn memory_growing( + &mut self, + _current: usize, + _request: usize, + _max: Option, + ) -> bool { self.hit = true; true } - - fn table_growing(&mut self, _request: u32, _min: u32, _max: Option) -> bool { + fn table_growing(&mut self, _current: u32, _request: u32, _max: Option) -> bool { unreachable!() } } diff --git a/tests/rlimited-memory.rs b/tests/rlimited-memory.rs index 2d64a6edef..5acc2f5d8f 100644 --- a/tests/rlimited-memory.rs +++ b/tests/rlimited-memory.rs @@ -18,11 +18,9 @@ impl ResourceLimiter for MemoryGrowFailureDetector { self.desired = desired; true } - fn memory_grow_failed(&mut self, err: &anyhow::Error) { self.error = Some(err.to_string()); } - fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { true } From 147c8f8ed7b44a171427ede7a20157ad320ba3a7 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 20 Oct 2021 16:32:39 -0700 Subject: [PATCH 02/29] rename --- crates/runtime/src/lib.rs | 11 +++-------- crates/runtime/src/memory.rs | 12 ++++++------ crates/runtime/src/table.rs | 4 ++-- crates/wasmtime/src/store.rs | 11 +++-------- 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index f9afeed984..d3aee4b75e 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -91,17 +91,12 @@ pub unsafe trait Store { ) -> (&mut VMExternRefActivationsTable, &dyn ModuleInfoLookup); /// Callback invoked to allow the store's resource limiter to reject a memory grow operation. - fn limiter_memory_growing( - &mut self, - current: usize, - desired: usize, - maximum: Option, - ) -> bool; + fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option) -> bool; /// Callback invoked to notify the store's resource limiter that a memory grow operation has /// failed. - fn limiter_memory_grow_failed(&mut self, error: &anyhow::Error); + fn memory_grow_failed(&mut self, error: &anyhow::Error); /// Callback invoked to allow the store's resource limiter to reject a table grow operation. - fn limiter_table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; + fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; /// Callback invoked whenever fuel runs out by a wasm instance. If an error /// is returned that's raised as a trap. Otherwise wasm execution will diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index e9dde6689f..2561038b81 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -315,7 +315,7 @@ impl Memory { // calculation overflowed. This means that the `minimum` we're informing // the limiter is lossy and may not be 100% accurate, but for now the // expected uses of limiter means that's ok. - if !store.limiter_memory_growing(0, minimum.unwrap_or(absolute_max), maximum) { + if !store.memory_growing(0, minimum.unwrap_or(absolute_max), maximum) { bail!( "memory minimum size of {} pages exceeds memory limits", plan.memory.minimum @@ -402,14 +402,14 @@ impl Memory { let maximum = self.maximum_byte_size(); // Store limiter gets first chance to reject memory_growing. - if !store.limiter_memory_growing(old_byte_size, new_byte_size, maximum) { + if !store.memory_growing(old_byte_size, new_byte_size, maximum) { return None; } // Never exceed maximum, even if limiter permitted it. if let Some(max) = maximum { if new_byte_size > max { - store.limiter_memory_grow_failed(&format_err!("Memory maximum size exceeded")); + store.memory_grow_failed(&format_err!("Memory maximum size exceeded")); return None; } } @@ -431,7 +431,7 @@ impl Memory { } => { // Never exceed static memory size if new_byte_size > base.len() { - store.limiter_memory_grow_failed(&format_err!("static memory size exceeded")); + store.memory_grow_failed(&format_err!("static memory size exceeded")); return None; } @@ -440,13 +440,13 @@ impl Memory { base.as_mut_ptr().add(old_byte_size), new_byte_size - old_byte_size, ); - r.map_err(|e| store.limiter_memory_grow_failed(&e)).ok()?; + r.map_err(|e| store.memory_grow_failed(&e)).ok()?; *size = new_byte_size; } Memory::Dynamic(mem) => { let r = mem.grow_to(new_byte_size); - r.map_err(|e| store.limiter_memory_grow_failed(&e)).ok()?; + r.map_err(|e| store.memory_grow_failed(&e)).ok()?; } } Some(old_byte_size) diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index 690a1ed82a..c5fbfe3f67 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -168,7 +168,7 @@ impl Table { } fn limit_new(plan: &TablePlan, store: &mut dyn Store) -> Result<()> { - if !store.limiter_table_growing(0, plan.table.minimum, plan.table.maximum) { + if !store.table_growing(0, plan.table.minimum, plan.table.maximum) { bail!( "table minimum size of {} elements exceeds table limits", plan.table.minimum @@ -292,7 +292,7 @@ impl Table { let old_size = self.size(); let new_size = old_size.checked_add(delta)?; - if !store.limiter_table_growing(old_size, new_size, self.maximum()) { + if !store.table_growing(old_size, new_size, self.maximum()) { return None; } diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 4a72c18959..644633f053 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -1526,12 +1526,7 @@ unsafe impl wasmtime_runtime::Store for StoreInner { (&mut inner.externref_activations_table, &inner.modules) } - fn limiter_memory_growing( - &mut self, - current: usize, - desired: usize, - maximum: Option, - ) -> bool { + fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option) -> bool { // Need to borrow async_cx before the mut borrow of the limiter. // self.async_cx() panicks when used with a non-async store, so // wrap this in an option. @@ -1560,7 +1555,7 @@ unsafe impl wasmtime_runtime::Store for StoreInner { } } - fn limiter_memory_grow_failed(&mut self, error: &anyhow::Error) { + fn memory_grow_failed(&mut self, error: &anyhow::Error) { match self.limiter { Some(ResourceLimiterInner::Sync(ref mut limiter)) => { limiter(&mut self.data).memory_grow_failed(error) @@ -1573,7 +1568,7 @@ unsafe impl wasmtime_runtime::Store for StoreInner { } } - fn limiter_table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool { + fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool { // Need to borrow async_cx before the mut borrow of the limiter. // self.async_cx() panicks when used with a non-async store, so // wrap this in an option. From 9d1b24632ecb30d3712257c0502376bb1edfc26c Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 20 Oct 2021 16:34:13 -0700 Subject: [PATCH 03/29] fix --- crates/wasmtime/src/store.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 644633f053..8c3a63df48 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -540,6 +540,7 @@ impl Store { /// Note that this limiter is only used to limit the creation/growth of /// resources in the future, this does not retroactively attempt to apply /// limits to the [`Store`]. + #[cfg(feature = "async")] pub fn limiter_async( &mut self, mut limiter: impl FnMut(&mut T) -> &mut (dyn crate::ResourceLimiterAsync) From 67a6c27e225ba6c54b1be1c4eb1902857d6d75b0 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 20 Oct 2021 16:53:24 -0700 Subject: [PATCH 04/29] pooling needs the store earlier --- crates/runtime/src/instance/allocator.rs | 10 ++++------ crates/runtime/src/instance/allocator/pooling.rs | 6 ++++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index 0826eecd8b..fead139467 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -92,12 +92,10 @@ impl StorePtr { pub fn new(ptr: *mut dyn Store) -> Self { Self(Some(ptr)) } - /* - /// Update an empty StorePtr to point to a Store. - pub fn set(&mut self, ptr: *mut dyn Store) { - self.0 = Some(ptr) + /// The raw contents of this struct + pub fn as_raw(&self) -> Option<*mut dyn Store> { + self.0.clone() } - */ /// Use the StorePtr as a mut ref to the Store. // XXX should this be an unsafe fn? is it always safe at a use site? pub(crate) fn get(&mut self) -> Option<&mut dyn Store> { @@ -461,7 +459,7 @@ fn initialize_instance( } unsafe fn initialize_vmcontext(instance: &mut Instance, req: InstanceAllocationRequest) { - if let Some(store) = req.store.0 { + if let Some(store) = req.store.as_raw() { *instance.interrupts() = (*store).vminterrupts(); *instance.externref_activations_table() = (*store).externref_activations_table().0; instance.set_store(store); diff --git a/crates/runtime/src/instance/allocator/pooling.rs b/crates/runtime/src/instance/allocator/pooling.rs index b99986a498..76614137d5 100644 --- a/crates/runtime/src/instance/allocator/pooling.rs +++ b/crates/runtime/src/instance/allocator/pooling.rs @@ -384,6 +384,12 @@ impl InstancePool { instance.host_state = std::mem::replace(&mut req.host_state, Box::new(())); instance.wasm_data = &*req.wasm_data; + // set_instance_memories and _tables will need the store before we can completely + // initialize the vmcontext. + if let Some(store) = req.store.as_raw() { + instance.set_store(store); + } + Self::set_instance_memories( instance, self.memories.get(index), From adf7521e30c3c2d57e5914500f801be95693e5ca Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 20 Oct 2021 17:26:59 -0700 Subject: [PATCH 05/29] introduce a failing test --- tests/all/limits.rs | 90 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 7e1b303906..dab1542794 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -65,6 +65,96 @@ fn test_limits() -> Result<()> { Ok(()) } +#[tokio::test] +async fn test_limits_async() -> Result<()> { + let mut config = Config::new(); + config.async_support(true); + let engine = Engine::new(&config).unwrap(); + let module = Module::new( + &engine, + r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, + )?; + + struct LimitsAsync { + memory_size: usize, + table_elements: u32, + } + #[async_trait::async_trait] + impl ResourceLimiterAsync for LimitsAsync { + async fn memory_growing( + &mut self, + _current: usize, + desired: usize, + _maximum: Option, + ) -> bool { + desired <= self.memory_size + } + fn memory_grow_failed(&mut self, _error: &anyhow::Error) {} + async fn table_growing( + &mut self, + _current: u32, + desired: u32, + _maximum: Option, + ) -> bool { + desired <= self.table_elements + } + } + + let mut store = Store::new( + &engine, + LimitsAsync { + memory_size: 10 * WASM_PAGE_SIZE, + table_elements: 5, + }, + ); + + store.limiter_async(|s| s as &mut dyn ResourceLimiterAsync); + + let instance = Instance::new_async(&mut store, &module, &[]).await?; + + // Test instance exports and host objects hitting the limit + for memory in std::array::IntoIter::new([ + instance.get_memory(&mut store, "m").unwrap(), + Memory::new(&mut store, MemoryType::new(0, None))?, + ]) { + memory.grow(&mut store, 3)?; + memory.grow(&mut store, 5)?; + memory.grow(&mut store, 2)?; + + assert_eq!( + memory + .grow(&mut store, 1) + .map_err(|e| e.to_string()) + .unwrap_err(), + "failed to grow memory by `1`" + ); + } + + // Test instance exports and host objects hitting the limit + for table in std::array::IntoIter::new([ + instance.get_table(&mut store, "t").unwrap(), + Table::new( + &mut store, + TableType::new(ValType::FuncRef, 0, None), + Val::FuncRef(None), + )?, + ]) { + table.grow(&mut store, 2, Val::FuncRef(None))?; + table.grow(&mut store, 1, Val::FuncRef(None))?; + table.grow(&mut store, 2, Val::FuncRef(None))?; + + assert_eq!( + table + .grow(&mut store, 1, Val::FuncRef(None)) + .map_err(|e| e.to_string()) + .unwrap_err(), + "failed to grow table by `1`" + ); + } + + Ok(()) +} + #[test] fn test_limits_memory_only() -> Result<()> { let engine = Engine::default(); From 2225722373be5bf56335d7664459a7bd76f2d7d5 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 20 Oct 2021 18:34:26 -0700 Subject: [PATCH 06/29] Memory::new_async, grow_async now work! --- crates/wasmtime/src/instance.rs | 24 +++++++++++----------- crates/wasmtime/src/memory.rs | 35 +++++++++++++++++++++++++++++++++ tests/all/limits.rs | 11 ++++++----- 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index e11971739a..b2cf6affa5 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -503,19 +503,19 @@ impl<'a> Instantiator<'a> { // NB: this is the same code as `run`. It's intentionally // small but should be kept in sync (modulo the async bits). - loop { - let step = self.step(store.0)?; - if let Some((instance, start, toplevel)) = step { - if let Some(start) = start { - store - .on_fiber(|store| Instantiator::start_raw(store, instance, start)) - .await??; + store + .on_fiber(|store| loop { + let step = self.step(store.0)?; + if let Some((instance, start, toplevel)) = step { + if let Some(start) = start { + Instantiator::start_raw(store, instance, start)? + } + if toplevel { + break Ok(instance); + } } - if toplevel { - break Ok(instance); - } - } - } + }) + .await? } /// Processes the next initializer for the next instance being created diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index 1d04a6edad..de15665afd 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -223,6 +223,24 @@ impl Memory { Memory::_new(store.as_context_mut().0, ty) } + /// Async variant of [`Memory::new`]. You must use this variant with Stores which have a + /// [`ResourceLimiterAsync`]. + #[cfg(feature = "async")] + pub async fn new_async( + mut store: impl AsContextMut, + ty: MemoryType, + ) -> Result + where + T: Send, + { + let mut store = store.as_context_mut(); + assert!( + store.0.async_support(), + "cannot use `new_async` without enabling async support on the config" + ); + store.on_fiber(|store| Memory::_new(store.0, ty)).await? + } + fn _new(store: &mut StoreOpaque, ty: MemoryType) -> Result { unsafe { let export = generate_memory_export(store, &ty)?; @@ -472,6 +490,23 @@ impl Memory { } } + /// Async variant of [`Memory::grow`]. Required when using a [`ResourceLimiterAsync`]. + #[cfg(feature = "async")] + pub async fn grow_async( + &self, + mut store: impl AsContextMut, + delta: u64, + ) -> Result + where + T: Send, + { + let mut store = store.as_context_mut(); + assert!( + store.0.async_support(), + "cannot use `grow_async` without enabling async support on the config" + ); + store.on_fiber(|store| self.grow(store, delta)).await? + } fn wasmtime_memory(&self, store: &mut StoreOpaque) -> *mut wasmtime_runtime::Memory { unsafe { let export = &store[self.0]; diff --git a/tests/all/limits.rs b/tests/all/limits.rs index dab1542794..3a5388cf20 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -115,15 +115,16 @@ async fn test_limits_async() -> Result<()> { // Test instance exports and host objects hitting the limit for memory in std::array::IntoIter::new([ instance.get_memory(&mut store, "m").unwrap(), - Memory::new(&mut store, MemoryType::new(0, None))?, + Memory::new_async(&mut store, MemoryType::new(0, None)).await?, ]) { - memory.grow(&mut store, 3)?; - memory.grow(&mut store, 5)?; - memory.grow(&mut store, 2)?; + memory.grow_async(&mut store, 3).await?; + memory.grow_async(&mut store, 5).await?; + memory.grow_async(&mut store, 2).await?; assert_eq!( memory - .grow(&mut store, 1) + .grow_async(&mut store, 1) + .await .map_err(|e| e.to_string()) .unwrap_err(), "failed to grow memory by `1`" From a5007f318ffa0d6331fe4926e3008c999f606908 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 11:36:48 -0700 Subject: [PATCH 07/29] runtime: use anyhow::Error instead of Box --- crates/runtime/src/instance.rs | 11 +++++--- crates/runtime/src/lib.rs | 20 +++++++++++---- crates/runtime/src/libcalls.rs | 40 +++++++++++++---------------- crates/runtime/src/memory.rs | 41 +++++++++++++++++++----------- crates/runtime/src/table.rs | 18 ++++++++----- crates/runtime/src/traphandlers.rs | 8 +++--- 6 files changed, 81 insertions(+), 57 deletions(-) diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 3bff9803ea..64afecd760 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -12,6 +12,7 @@ use crate::vmcontext::{ VMInterrupts, VMMemoryDefinition, VMMemoryImport, VMTableDefinition, VMTableImport, }; use crate::{ExportFunction, ExportGlobal, ExportMemory, ExportTable, Store}; +use anyhow::Error; use memoffset::offset_of; use more_asserts::assert_lt; use std::alloc::Layout; @@ -348,7 +349,11 @@ impl Instance { /// Returns `None` if memory can't be grown by the specified amount /// of pages. Returns `Some` with the old size in bytes if growth was /// successful. - pub(crate) fn memory_grow(&mut self, index: MemoryIndex, delta: u64) -> Option { + pub(crate) fn memory_grow( + &mut self, + index: MemoryIndex, + delta: u64, + ) -> Result, Error> { let (idx, instance) = if let Some(idx) = self.module.defined_memory_index(index) { (idx, self) } else { @@ -387,7 +392,7 @@ impl Instance { table_index: TableIndex, delta: u32, init_value: TableElement, - ) -> Option { + ) -> Result, Error> { let (defined_table_index, instance) = self.get_defined_table_index_and_instance(table_index); instance.defined_table_grow(defined_table_index, delta, init_value) @@ -398,7 +403,7 @@ impl Instance { table_index: DefinedTableIndex, delta: u32, init_value: TableElement, - ) -> Option { + ) -> Result, Error> { let store = unsafe { &mut *self.store() }; let table = self .tables diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index d3aee4b75e..4f531d4220 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -20,7 +20,7 @@ ) )] -use std::error::Error; +use anyhow::Error; mod export; mod externref; @@ -91,15 +91,25 @@ pub unsafe trait Store { ) -> (&mut VMExternRefActivationsTable, &dyn ModuleInfoLookup); /// Callback invoked to allow the store's resource limiter to reject a memory grow operation. - fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option) -> bool; + fn memory_growing( + &mut self, + current: usize, + desired: usize, + maximum: Option, + ) -> Result; /// Callback invoked to notify the store's resource limiter that a memory grow operation has /// failed. - fn memory_grow_failed(&mut self, error: &anyhow::Error); + fn memory_grow_failed(&mut self, error: &Error); /// Callback invoked to allow the store's resource limiter to reject a table grow operation. - fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; + fn table_growing( + &mut self, + current: u32, + desired: u32, + maximum: Option, + ) -> Result; /// Callback invoked whenever fuel runs out by a wasm instance. If an error /// is returned that's raised as a trap. Otherwise wasm execution will /// continue as normal. - fn out_of_gas(&mut self) -> Result<(), Box>; + fn out_of_gas(&mut self) -> Result<(), Error>; } diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index f307a4dbfe..067e0b8503 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -193,8 +193,9 @@ pub unsafe extern "C" fn wasmtime_memory32_grow( let instance = (*vmctx).instance_mut(); let memory_index = MemoryIndex::from_u32(memory_index); match instance.memory_grow(memory_index, delta) { - Some(size_in_bytes) => size_in_bytes / (wasmtime_environ::WASM_PAGE_SIZE as usize), - None => usize::max_value(), + Ok(Some(size_in_bytes)) => size_in_bytes / (wasmtime_environ::WASM_PAGE_SIZE as usize), + Ok(None) => usize::max_value(), + Err(err) => crate::traphandlers::raise_user_trap(err), } } @@ -220,9 +221,11 @@ pub unsafe extern "C" fn wasmtime_table_grow( init_value.into() } }; - instance - .table_grow(table_index, delta, element) - .unwrap_or(-1_i32 as u32) + match instance.table_grow(table_index, delta, element) { + Ok(Some(r)) => r, + Ok(None) => -1_i32 as u32, + Err(err) => crate::traphandlers::raise_user_trap(err), + } } /// Implementation of `table.fill`. @@ -436,15 +439,6 @@ pub unsafe extern "C" fn wasmtime_externref_global_set( drop(old); } -#[derive(Debug)] -struct Unimplemented(&'static str); -impl std::error::Error for Unimplemented {} -impl std::fmt::Display for Unimplemented { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error> { - write!(f, "unimplemented: {}", self.0) - } -} - /// Implementation of `memory.atomic.notify` for locally defined memories. pub unsafe extern "C" fn wasmtime_memory_atomic_notify( vmctx: *mut VMContext, @@ -460,9 +454,9 @@ pub unsafe extern "C" fn wasmtime_memory_atomic_notify( // just to be sure. let addr_to_check = addr.checked_add(4).unwrap(); validate_atomic_addr(instance, memory, addr_to_check).and_then(|()| { - Err(Trap::User(Box::new(Unimplemented( - "wasm atomics (fn wasmtime_memory_atomic_notify) unsupported", - )))) + Err(Trap::User(anyhow::anyhow!( + "unimplemented: wasm atomics (fn wasmtime_memory_atomic_notify) unsupported", + ))) }) }; match result { @@ -486,9 +480,9 @@ pub unsafe extern "C" fn wasmtime_memory_atomic_wait32( // but we still double-check let addr_to_check = addr.checked_add(4).unwrap(); validate_atomic_addr(instance, memory, addr_to_check).and_then(|()| { - Err(Trap::User(Box::new(Unimplemented( - "wasm atomics (fn wasmtime_memory_atomic_wait32) unsupported", - )))) + Err(Trap::User(anyhow::anyhow!( + "unimplemented: wasm atomics (fn wasmtime_memory_atomic_wait32) unsupported", + ))) }) }; match result { @@ -512,9 +506,9 @@ pub unsafe extern "C" fn wasmtime_memory_atomic_wait64( // but we still double-check let addr_to_check = addr.checked_add(8).unwrap(); validate_atomic_addr(instance, memory, addr_to_check).and_then(|()| { - Err(Trap::User(Box::new(Unimplemented( - "wasm atomics (fn wasmtime_memory_atomic_wait64) unsupported", - )))) + Err(Trap::User(anyhow::anyhow!( + "unimplemented: wasm atomics (fn wasmtime_memory_atomic_wait64) unsupported", + ))) }) }; match result { diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index 2561038b81..7e2180e5d3 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -5,6 +5,7 @@ use crate::mmap::Mmap; use crate::vmcontext::VMMemoryDefinition; use crate::Store; +use anyhow::Error; use anyhow::{bail, format_err, Result}; use more_asserts::{assert_ge, assert_le}; use std::convert::TryFrom; @@ -315,7 +316,7 @@ impl Memory { // calculation overflowed. This means that the `minimum` we're informing // the limiter is lossy and may not be 100% accurate, but for now the // expected uses of limiter means that's ok. - if !store.memory_growing(0, minimum.unwrap_or(absolute_max), maximum) { + if !store.memory_growing(0, minimum.unwrap_or(absolute_max), maximum)? { bail!( "memory minimum size of {} pages exceeds memory limits", plan.memory.minimum @@ -377,11 +378,15 @@ impl Memory { /// /// Generally, prefer using `InstanceHandle::memory_grow`, which encapsulates /// this unsafety. - pub unsafe fn grow(&mut self, delta_pages: u64, store: &mut dyn Store) -> Option { + pub unsafe fn grow( + &mut self, + delta_pages: u64, + store: &mut dyn Store, + ) -> Result, Error> { let old_byte_size = self.byte_size(); // Wasm spec: when growing by 0 pages, always return the current size. if delta_pages == 0 { - return Some(old_byte_size); + return Ok(Some(old_byte_size)); } // largest wasm-page-aligned region of memory it is possible to @@ -402,15 +407,15 @@ impl Memory { let maximum = self.maximum_byte_size(); // Store limiter gets first chance to reject memory_growing. - if !store.memory_growing(old_byte_size, new_byte_size, maximum) { - return None; + if !store.memory_growing(old_byte_size, new_byte_size, maximum)? { + return Ok(None); } // Never exceed maximum, even if limiter permitted it. if let Some(max) = maximum { if new_byte_size > max { store.memory_grow_failed(&format_err!("Memory maximum size exceeded")); - return None; + return Ok(None); } } @@ -418,7 +423,10 @@ impl Memory { { if self.is_static() { // Reset any faulted guard pages before growing the memory. - self.reset_guard_pages().ok()?; + if let Err(e) = self.reset_guard_pages() { + store.memory_grow_failed(&e); + return Ok(None); + } } } @@ -432,24 +440,27 @@ impl Memory { // Never exceed static memory size if new_byte_size > base.len() { store.memory_grow_failed(&format_err!("static memory size exceeded")); - return None; + return Ok(None); } // Operating system can fail to make memory accessible - let r = make_accessible( + if let Err(e) = make_accessible( base.as_mut_ptr().add(old_byte_size), new_byte_size - old_byte_size, - ); - r.map_err(|e| store.memory_grow_failed(&e)).ok()?; - + ) { + store.memory_grow_failed(&e); + return Ok(None); + } *size = new_byte_size; } Memory::Dynamic(mem) => { - let r = mem.grow_to(new_byte_size); - r.map_err(|e| store.memory_grow_failed(&e)).ok()?; + if let Err(e) = mem.grow_to(new_byte_size) { + store.memory_grow_failed(&e); + return Ok(None); + } } } - Some(old_byte_size) + Ok(Some(old_byte_size)) } /// Return a `VMMemoryDefinition` for exposing the memory to compiled wasm code. diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index c5fbfe3f67..7e927700ed 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -4,6 +4,7 @@ use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition}; use crate::{Store, Trap, VMExternRef}; +use anyhow::Error; use anyhow::{bail, Result}; use std::convert::{TryFrom, TryInto}; use std::ops::Range; @@ -168,7 +169,7 @@ impl Table { } fn limit_new(plan: &TablePlan, store: &mut dyn Store) -> Result<()> { - if !store.table_growing(0, plan.table.minimum, plan.table.maximum) { + if !store.table_growing(0, plan.table.minimum, plan.table.maximum)? { bail!( "table minimum size of {} elements exceeds table limits", plan.table.minimum @@ -288,17 +289,20 @@ impl Table { delta: u32, init_value: TableElement, store: &mut dyn Store, - ) -> Option { + ) -> Result, Error> { let old_size = self.size(); - let new_size = old_size.checked_add(delta)?; + let new_size = match old_size.checked_add(delta) { + Some(s) => s, + None => return Ok(None), + }; - if !store.table_growing(old_size, new_size, self.maximum()) { - return None; + if !store.table_growing(old_size, new_size, self.maximum())? { + return Ok(None); } if let Some(max) = self.maximum() { if new_size > max { - return None; + return Ok(None); } } @@ -320,7 +324,7 @@ impl Table { self.fill(old_size, init_value, delta) .expect("table should not be out of bounds"); - Some(old_size) + Ok(Some(old_size)) } /// Get reference to the specified element. diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index ad78dcb05b..3a1bfac8ed 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -2,10 +2,10 @@ //! signalhandling mechanisms. use crate::{VMContext, VMInterrupts}; +use anyhow::Error; use backtrace::Backtrace; use std::any::Any; use std::cell::{Cell, UnsafeCell}; -use std::error::Error; use std::mem::MaybeUninit; use std::ptr; use std::sync::atomic::Ordering::SeqCst; @@ -80,7 +80,7 @@ pub fn init_traps(is_wasm_pc: fn(usize) -> bool) { /// Only safe to call when wasm code is on the stack, aka `catch_traps` must /// have been previously called. Additionally no Rust destructors can be on the /// stack. They will be skipped and not executed. -pub unsafe fn raise_user_trap(data: Box) -> ! { +pub unsafe fn raise_user_trap(data: Error) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::UserTrap(data))) } @@ -114,7 +114,7 @@ pub unsafe fn resume_panic(payload: Box) -> ! { #[derive(Debug)] pub enum Trap { /// A user-raised trap through `raise_user_trap`. - User(Box), + User(Error), /// A trap raised from jit code Jit { @@ -206,7 +206,7 @@ pub struct CallThreadState { enum UnwindReason { Panic(Box), - UserTrap(Box), + UserTrap(Error), LibTrap(Trap), JitTrap { backtrace: Backtrace, pc: usize }, } From abbe28d833d1a869384ec00b74af35cafc4766dc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 11:53:55 -0700 Subject: [PATCH 08/29] propogate changes to use anyhow::Error instead of Box --- crates/wasmtime/src/externals.rs | 2 +- crates/wasmtime/src/func.rs | 27 +++--------------- crates/wasmtime/src/memory.rs | 2 +- crates/wasmtime/src/store.rs | 39 +++++++++++++++----------- crates/wasmtime/src/trampoline/func.rs | 2 +- 5 files changed, 30 insertions(+), 42 deletions(-) diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 8f3d7e1645..d267bf14a7 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -551,7 +551,7 @@ impl Table { let init = init.into_table_element(store, ty)?; let table = self.wasmtime_table(store); unsafe { - match (*table).grow(delta, init, store) { + match (*table).grow(delta, init, store)? { Some(size) => { let vm = (*table).vmtable(); *store[self.0].definition = vm; diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index dd8877e8f9..068d0b4aaa 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -4,8 +4,6 @@ use crate::{ StoreContext, StoreContextMut, Trap, Val, ValRaw, ValType, }; use anyhow::{bail, Context as _, Result}; -use std::error::Error; -use std::fmt; use std::future::Future; use std::mem; use std::panic::{self, AssertUnwindSafe}; @@ -1784,25 +1782,6 @@ impl AsContextMut for Caller<'_, T> { } } -fn cross_store_trap() -> Box { - #[derive(Debug)] - struct CrossStoreError; - - impl Error for CrossStoreError {} - - impl fmt::Display for CrossStoreError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "host function attempted to return cross-`Store` \ - value to Wasm", - ) - } - } - - Box::new(CrossStoreError) -} - macro_rules! impl_into_func { ($num:tt $($args:ident)*) => { // Implement for functions without a leading `&Caller` parameter, @@ -1852,7 +1831,7 @@ macro_rules! impl_into_func { { enum CallResult { Ok(U), - Trap(Box), + Trap(anyhow::Error), Panic(Box), } @@ -1901,7 +1880,9 @@ macro_rules! impl_into_func { // can't assume it returned a value that is // compatible with this store. if !ret.compatible_with_store(caller.store.0) { - CallResult::Trap(cross_store_trap()) + CallResult::Trap(anyhow::anyhow!( + "host function attempted to return cross-`Store` value to Wasm" + )) } else { match ret.into_abi_for_ret(caller.store.0, retptr) { Ok(val) => CallResult::Ok(val), diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index de15665afd..f541c2f81b 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -479,7 +479,7 @@ impl Memory { let store = store.as_context_mut().0; let mem = self.wasmtime_memory(store); unsafe { - match (*mem).grow(delta, store) { + match (*mem).grow(delta, store)? { Some(size) => { let vm = (*mem).vmmemory(); *store[self.0].definition = vm; diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 8c3a63df48..b861befd53 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -81,7 +81,6 @@ use anyhow::{bail, Result}; use std::cell::UnsafeCell; use std::collections::HashMap; use std::convert::TryFrom; -use std::error::Error; use std::fmt; use std::future::Future; use std::marker; @@ -1527,7 +1526,12 @@ unsafe impl wasmtime_runtime::Store for StoreInner { (&mut inner.externref_activations_table, &inner.modules) } - fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option) -> bool { + fn memory_growing( + &mut self, + current: usize, + desired: usize, + maximum: Option, + ) -> Result { // Need to borrow async_cx before the mut borrow of the limiter. // self.async_cx() panicks when used with a non-async store, so // wrap this in an option. @@ -1539,20 +1543,19 @@ unsafe impl wasmtime_runtime::Store for StoreInner { }; match self.limiter { Some(ResourceLimiterInner::Sync(ref mut limiter)) => { - limiter(&mut self.data).memory_growing(current, desired, maximum) + Ok(limiter(&mut self.data).memory_growing(current, desired, maximum)) } #[cfg(feature = "async")] Some(ResourceLimiterInner::Async(ref mut limiter)) => unsafe { - async_cx + Ok(async_cx .expect("ResourceLimiterAsync requires async Store") .block_on( limiter(&mut self.data) .memory_growing(current, desired, maximum) .as_mut(), - ) - .expect("FIXME idk how to deal with a trap here!") + )?) }, - None => true, + None => Ok(true), } } @@ -1569,7 +1572,12 @@ unsafe impl wasmtime_runtime::Store for StoreInner { } } - fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool { + fn table_growing( + &mut self, + current: u32, + desired: u32, + maximum: Option, + ) -> Result { // Need to borrow async_cx before the mut borrow of the limiter. // self.async_cx() panicks when used with a non-async store, so // wrap this in an option. @@ -1582,33 +1590,32 @@ unsafe impl wasmtime_runtime::Store for StoreInner { match self.limiter { Some(ResourceLimiterInner::Sync(ref mut limiter)) => { - limiter(&mut self.data).table_growing(current, desired, maximum) + Ok(limiter(&mut self.data).table_growing(current, desired, maximum)) } #[cfg(feature = "async")] Some(ResourceLimiterInner::Async(ref mut limiter)) => unsafe { - async_cx + Ok(async_cx .expect("ResourceLimiterAsync requires async Store") .block_on( limiter(&mut self.data) .table_growing(current, desired, maximum) .as_mut(), - ) - .expect("FIXME idk how to deal with a trap here!") + )?) }, - None => true, + None => Ok(true), } } - fn out_of_gas(&mut self) -> Result<(), Box> { + fn out_of_gas(&mut self) -> Result<(), anyhow::Error> { return match &mut self.out_of_gas_behavior { - OutOfGas::Trap => Err(Box::new(OutOfGasError)), + OutOfGas::Trap => Err(anyhow::Error::new(OutOfGasError)), #[cfg(feature = "async")] OutOfGas::InjectFuel { injection_count, fuel_to_inject, } => { if *injection_count == 0 { - return Err(Box::new(OutOfGasError)); + return Err(anyhow::Error::new(OutOfGasError)); } *injection_count -= 1; let fuel = *fuel_to_inject; diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index e37803763f..fc217377be 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -57,7 +57,7 @@ unsafe extern "C" fn stub_fn( // call-site, which gets unwrapped in `Trap::from_runtime` later on as we // convert from the internal `Trap` type to our own `Trap` type in this // crate. - Ok(Err(trap)) => wasmtime_runtime::raise_user_trap(Box::new(trap)), + Ok(Err(trap)) => wasmtime_runtime::raise_user_trap(trap.into()), // And finally if the imported function panicked, then we trigger the // form of unwinding that's safe to jump over wasm code on all From 6c70b81ff55fc8dff574dccd0408fccff7d404bc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 12:04:20 -0700 Subject: [PATCH 09/29] review feedback --- crates/runtime/src/instance/allocator.rs | 21 +++++++++------------ crates/runtime/src/memory.rs | 3 +++ crates/wasmtime/src/memory.rs | 10 +++++++++- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index fead139467..abcb94b11f 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -97,10 +97,10 @@ impl StorePtr { self.0.clone() } /// Use the StorePtr as a mut ref to the Store. - // XXX should this be an unsafe fn? is it always safe at a use site? - pub(crate) fn get(&mut self) -> Option<&mut dyn Store> { + /// Safety: must not be used outside the original lifetime of the borrow. + pub(crate) unsafe fn get(&mut self) -> Option<&mut dyn Store> { match self.0 { - Some(ptr) => Some(unsafe { &mut *ptr }), + Some(ptr) => Some(&mut *ptr), None => None, } } @@ -630,12 +630,11 @@ impl OnDemandInstanceAllocator { PrimaryMap::with_capacity(module.table_plans.len() - num_imports); for table in &module.table_plans.values().as_slice()[num_imports..] { tables.push( - Table::new_dynamic( - table, + Table::new_dynamic(table, unsafe { store .get() - .expect("if module has table plans, store is not empty"), - ) + .expect("if module has table plans, store is not empty") + }) .map_err(InstantiationError::Resource)?, ); } @@ -656,13 +655,11 @@ impl OnDemandInstanceAllocator { PrimaryMap::with_capacity(module.memory_plans.len() - num_imports); for plan in &module.memory_plans.values().as_slice()[num_imports..] { memories.push( - Memory::new_dynamic( - plan, - creator, + Memory::new_dynamic(plan, creator, unsafe { store .get() - .expect("if module has memory plans, store is not empty"), - ) + .expect("if module has memory plans, store is not empty") + }) .map_err(InstantiationError::Resource)?, ); } diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index 7e2180e5d3..5a69d2136e 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -378,6 +378,9 @@ impl Memory { /// /// Generally, prefer using `InstanceHandle::memory_grow`, which encapsulates /// this unsafety. + /// + /// Ensure that the provided Store is not used to get access any Memory + /// which lives inside it. pub unsafe fn grow( &mut self, delta_pages: u64, diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index f541c2f81b..3f22f3dd2f 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -223,8 +223,12 @@ impl Memory { Memory::_new(store.as_context_mut().0, ty) } - /// Async variant of [`Memory::new`]. You must use this variant with Stores which have a + /// Async variant of [`Memory::new`]. You must use this variant with [`Store`]s which have a /// [`ResourceLimiterAsync`]. + /// + /// # Panics + /// + /// This function will panic when used with a non-async [`Store`]. #[cfg(feature = "async")] pub async fn new_async( mut store: impl AsContextMut, @@ -491,6 +495,10 @@ impl Memory { } /// Async variant of [`Memory::grow`]. Required when using a [`ResourceLimiterAsync`]. + /// + /// # Panics + /// + /// This function will panic when used with a non-async [`Store`]. #[cfg(feature = "async")] pub async fn grow_async( &self, From d3deaae99d900ff830952f8e75ef2eb61affe568 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 12:09:18 -0700 Subject: [PATCH 10/29] collapse some common code --- crates/wasmtime/src/instance.rs | 58 +++++++++++++-------------------- 1 file changed, 22 insertions(+), 36 deletions(-) diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index b2cf6affa5..bced7cfe4c 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -126,6 +126,11 @@ impl Instance { typecheck_externs(store.0, module, imports)?; Instantiator::new(store.0, module, ImportSource::Externs(imports))? }; + assert!( + !store.0.async_support(), + "cannot use `new` when async support is enabled on the config" + ); + i.run(&mut store) } @@ -162,7 +167,14 @@ impl Instance { typecheck_externs(store.0, module, imports)?; Instantiator::new(store.0, module, ImportSource::Externs(imports))? }; - i.run_async(&mut store).await + let mut store = store.as_context_mut(); + assert!( + store.0.async_support(), + "cannot use `new_async` without enabling async support on the config" + ); + store + .on_fiber(|store| i.run(&mut store.as_context_mut())) + .await? } pub(crate) fn from_wasmtime(handle: InstanceData, store: &mut StoreOpaque) -> Instance { @@ -472,13 +484,6 @@ impl<'a> Instantiator<'a> { } fn run(&mut self, store: &mut StoreContextMut<'_, T>) -> Result { - assert!( - !store.0.async_support(), - "cannot use `new` when async support is enabled on the config" - ); - - // NB: this is the same code as `run_async`. It's intentionally - // small but should be kept in sync (modulo the async bits). loop { if let Some((instance, start, toplevel)) = self.step(store.0)? { if let Some(start) = start { @@ -491,33 +496,6 @@ impl<'a> Instantiator<'a> { } } - #[cfg(feature = "async")] - async fn run_async(&mut self, store: &mut StoreContextMut<'_, T>) -> Result - where - T: Send, - { - assert!( - store.0.async_support(), - "cannot use `new_async` without enabling async support on the config" - ); - - // NB: this is the same code as `run`. It's intentionally - // small but should be kept in sync (modulo the async bits). - store - .on_fiber(|store| loop { - let step = self.step(store.0)?; - if let Some((instance, start, toplevel)) = step { - if let Some(start) = start { - Instantiator::start_raw(store, instance, start)? - } - if toplevel { - break Ok(instance); - } - } - }) - .await? - } - /// Processes the next initializer for the next instance being created /// without running any wasm code. /// @@ -1002,7 +980,15 @@ impl InstancePre { ImportSource::Definitions(&self.items), )? }; - i.run_async(&mut store.as_context_mut()).await + + let mut store = store.as_context_mut(); + assert!( + store.0.async_support(), + "cannot use `new_async` without enabling async support on the config" + ); + store + .on_fiber(|store| i.run(&mut store.as_context_mut())) + .await? } fn ensure_comes_from_same_store(&self, store: &StoreOpaque) -> Result<()> { From 5aef8f47c8993c354fb285561fcb9c206f477e7a Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 12:15:00 -0700 Subject: [PATCH 11/29] catch panic in libcalls for memory and table grow --- crates/runtime/src/libcalls.rs | 58 +++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 067e0b8503..9aba075bc6 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -59,7 +59,7 @@ use crate::externref::VMExternRef; use crate::instance::Instance; use crate::table::{Table, TableElementType}; -use crate::traphandlers::{raise_lib_trap, Trap}; +use crate::traphandlers::{raise_lib_trap, resume_panic, Trap}; use crate::vmcontext::{VMCallerCheckedAnyfunc, VMContext}; use backtrace::Backtrace; use std::mem; @@ -190,13 +190,17 @@ pub unsafe extern "C" fn wasmtime_memory32_grow( delta: u64, memory_index: u32, ) -> usize { - let instance = (*vmctx).instance_mut(); - let memory_index = MemoryIndex::from_u32(memory_index); - match instance.memory_grow(memory_index, delta) { - Ok(Some(size_in_bytes)) => size_in_bytes / (wasmtime_environ::WASM_PAGE_SIZE as usize), - Ok(None) => usize::max_value(), - Err(err) => crate::traphandlers::raise_user_trap(err), - } + std::panic::catch_unwind(|| { + let instance = (*vmctx).instance_mut(); + let memory_index = MemoryIndex::from_u32(memory_index); + match instance.memory_grow(memory_index, delta) { + Ok(Some(size_in_bytes)) => size_in_bytes / (wasmtime_environ::WASM_PAGE_SIZE as usize), + Ok(None) => usize::max_value(), + Err(err) => crate::traphandlers::raise_user_trap(err), + } + }) + .map_err(|panic| resume_panic(panic)) + .unwrap() } /// Implementation of `table.grow`. @@ -208,24 +212,28 @@ pub unsafe extern "C" fn wasmtime_table_grow( // or is a `VMExternRef` until we look at the table type. init_value: *mut u8, ) -> u32 { - let instance = (*vmctx).instance_mut(); - let table_index = TableIndex::from_u32(table_index); - let element = match instance.table_element_type(table_index) { - TableElementType::Func => (init_value as *mut VMCallerCheckedAnyfunc).into(), - TableElementType::Extern => { - let init_value = if init_value.is_null() { - None - } else { - Some(VMExternRef::clone_from_raw(init_value)) - }; - init_value.into() + std::panic::catch_unwind(|| { + let instance = (*vmctx).instance_mut(); + let table_index = TableIndex::from_u32(table_index); + let element = match instance.table_element_type(table_index) { + TableElementType::Func => (init_value as *mut VMCallerCheckedAnyfunc).into(), + TableElementType::Extern => { + let init_value = if init_value.is_null() { + None + } else { + Some(VMExternRef::clone_from_raw(init_value)) + }; + init_value.into() + } + }; + match instance.table_grow(table_index, delta, element) { + Ok(Some(r)) => r, + Ok(None) => -1_i32 as u32, + Err(err) => crate::traphandlers::raise_user_trap(err), } - }; - match instance.table_grow(table_index, delta, element) { - Ok(Some(r)) => r, - Ok(None) => -1_i32 as u32, - Err(err) => crate::traphandlers::raise_user_trap(err), - } + }) + .map_err(|panic| resume_panic(panic)) + .unwrap() } /// Implementation of `table.fill`. From 252ba39c27c8fe3e88488391b7c8189f53e4cc0f Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 14:15:53 -0700 Subject: [PATCH 12/29] implement table _async methods, test passes now --- crates/wasmtime/src/externals.rs | 50 ++++++++++++++++++++++++++++++++ tests/all/limits.rs | 14 +++++---- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index d267bf14a7..686b154472 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -436,6 +436,31 @@ impl Table { Table::_new(store.as_context_mut().0, ty, init) } + /// Async variant of [`Table::new`]. You must use this variant with [`Store`]s which have a + /// [`ResourceLimiterAsync`]. + /// + /// # Panics + /// + /// This function will panic when used with a non-async [`Store`]. + #[cfg(feature = "async")] + pub async fn new_async( + mut store: impl AsContextMut, + ty: TableType, + init: Val, + ) -> Result + where + T: Send, + { + let mut store = store.as_context_mut(); + assert!( + store.0.async_support(), + "cannot use `new_async` without enabling async support on the config" + ); + store + .on_fiber(|store| Table::_new(store.0, ty, init)) + .await? + } + fn _new(store: &mut StoreOpaque, ty: TableType, init: Val) -> Result
{ let wasmtime_export = generate_table_export(store, &ty)?; let init = init.into_table_element(store, ty.element())?; @@ -562,6 +587,31 @@ impl Table { } } + /// Async variant of [`Table::grow`]. Required when using a [`ResourceLimiterAsync`]. + /// + /// # Panics + /// + /// This function will panic when used with a non-async [`Store`]. + #[cfg(feature = "async")] + pub async fn grow_async( + &self, + mut store: impl AsContextMut, + delta: u32, + init: Val, + ) -> Result + where + T: Send, + { + let mut store = store.as_context_mut(); + assert!( + store.0.async_support(), + "cannot use `grow_async` without enabling async support on the config" + ); + store + .on_fiber(|store| self.grow(store, delta, init)) + .await? + } + /// Copy `len` elements from `src_table[src_index..]` into /// `dst_table[dst_index..]`. /// diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 3a5388cf20..c19ab4b047 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -134,19 +134,21 @@ async fn test_limits_async() -> Result<()> { // Test instance exports and host objects hitting the limit for table in std::array::IntoIter::new([ instance.get_table(&mut store, "t").unwrap(), - Table::new( + Table::new_async( &mut store, TableType::new(ValType::FuncRef, 0, None), Val::FuncRef(None), - )?, + ) + .await?, ]) { - table.grow(&mut store, 2, Val::FuncRef(None))?; - table.grow(&mut store, 1, Val::FuncRef(None))?; - table.grow(&mut store, 2, Val::FuncRef(None))?; + table.grow_async(&mut store, 2, Val::FuncRef(None)).await?; + table.grow_async(&mut store, 1, Val::FuncRef(None)).await?; + table.grow_async(&mut store, 2, Val::FuncRef(None)).await?; assert_eq!( table - .grow(&mut store, 1, Val::FuncRef(None)) + .grow_async(&mut store, 1, Val::FuncRef(None)) + .await .map_err(|e| e.to_string()) .unwrap_err(), "failed to grow table by `1`" From 351a51cce6aa407c1038cb7065595b30e95bdf52 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 14:28:40 -0700 Subject: [PATCH 13/29] docs --- crates/wasmtime/src/store.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index b861befd53..ccacd816e3 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -533,12 +533,24 @@ impl Store { inner.limiter = Some(ResourceLimiterInner::Sync(Box::new(limiter))); } - /// Configures the [`ResourceLimiterAsync`](crate::ResourceLimiterAsync) used to limit - /// resource creation within this [`Store`]. Must be used with an async `Store`!. + /// Configures the [`ResourceLimiterAsync`](crate::ResourceLimiterAsync) + /// used to limit resource creation within this [`Store`]. Must be used + /// with an async `Store`!. /// /// Note that this limiter is only used to limit the creation/growth of /// resources in the future, this does not retroactively attempt to apply /// limits to the [`Store`]. + /// + /// This variation on the [`ResourceLimiter`] makes the `memory_growing` + /// and `table_growing` functions `async`. This means that, as part of + /// your resource limiting strategy, the async resource limiter may yield + /// execution until a resource becomes available. + /// + /// By using a [`ResourceLimiterAsync`] with a [`Store`], you can no + /// longer use [`Memory::new`], [`Memory::grow`], [`Table::new`], and + /// [`Table::grow`]. Instead, you must use their `async` variants: + /// [`Memory::new_async`], [`Memory::grow_async`], [`Table::new_async`], + /// and [`Table::grow_async`]. #[cfg(feature = "async")] pub fn limiter_async( &mut self, From c0a1af94cfd2a33aa6c0e52d18fc5661baa0f9d8 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 15:07:32 -0700 Subject: [PATCH 14/29] fix trap behavior --- crates/wasmtime/src/trap.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 61da2dcce5..39f557dcf8 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -385,7 +385,10 @@ impl std::error::Error for Trap { impl From for Trap { fn from(e: anyhow::Error) -> Trap { - Box::::from(e).into() + match e.downcast::() { + Ok(trap) => trap, + Err(e) => Box::::from(e).into(), + } } } From a1301f8dae96012adcf9a756ee5009bd5949f3cc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 15:07:40 -0700 Subject: [PATCH 15/29] add table_grow_failed --- crates/runtime/src/lib.rs | 14 +++++++++----- crates/runtime/src/table.rs | 4 ++-- crates/wasmtime/src/limits.rs | 18 ++++++++++++++++-- crates/wasmtime/src/store.rs | 13 +++++++++++++ 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 4f531d4220..6fc8e66c3d 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -90,24 +90,28 @@ pub unsafe trait Store { &mut self, ) -> (&mut VMExternRefActivationsTable, &dyn ModuleInfoLookup); - /// Callback invoked to allow the store's resource limiter to reject a memory grow operation. + /// Callback invoked to allow the store's resource limiter to reject a + /// memory grow operation. fn memory_growing( &mut self, current: usize, desired: usize, maximum: Option, ) -> Result; - /// Callback invoked to notify the store's resource limiter that a memory grow operation has - /// failed. + /// Callback invoked to notify the store's resource limiter that a memory + /// grow operation has failed. fn memory_grow_failed(&mut self, error: &Error); - /// Callback invoked to allow the store's resource limiter to reject a table grow operation. + /// Callback invoked to allow the store's resource limiter to reject a + /// table grow operation. fn table_growing( &mut self, current: u32, desired: u32, maximum: Option, ) -> Result; - + /// Callback invoked to notify the store's resource limiter that a table + /// grow operation has failed. + fn table_grow_failed(&mut self, error: &Error); /// Callback invoked whenever fuel runs out by a wasm instance. If an error /// is returned that's raised as a trap. Otherwise wasm execution will /// continue as normal. diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index 7e927700ed..53a1f8b7b3 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -4,8 +4,7 @@ use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition}; use crate::{Store, Trap, VMExternRef}; -use anyhow::Error; -use anyhow::{bail, Result}; +use anyhow::{bail, format_err, Error, Result}; use std::convert::{TryFrom, TryInto}; use std::ops::Range; use std::ptr; @@ -302,6 +301,7 @@ impl Table { if let Some(max) = self.maximum() { if new_size > max { + store.table_grow_failed(&format_err!("Table maximum size exceeded")); return Ok(None); } } diff --git a/crates/wasmtime/src/limits.rs b/crates/wasmtime/src/limits.rs index acca7dad4e..02d6fcd814 100644 --- a/crates/wasmtime/src/limits.rs +++ b/crates/wasmtime/src/limits.rs @@ -50,6 +50,13 @@ pub trait ResourceLimiter { /// effect as the table will not grow. fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; + /// Notifies the resource limiter that growing a linear memory, permitted by + /// the `table_growing` method, has failed. + /// + /// Reasons for failure include: the growth exceeds the `maximum` passed to + /// `table_growing`. This could expand in the future. + fn table_grow_failed(&mut self, _error: &anyhow::Error) {} + /// The maximum number of instances that can be created for a `Store`. /// /// Module instantiation will fail if this limit is exceeded. @@ -79,9 +86,13 @@ pub trait ResourceLimiter { } #[cfg(feature = "async")] -/// Used by hosts to limit resource consumption of instances. -/// Identical to [`ResourceLimiter`], except that the `memory_growing` and `table_growing` +/// Used by hosts to limit resource consumption of instances. Identical to +/// [`ResourceLimiter`], except that the `memory_growing` and `table_growing` /// functions are async. Must be used with an async [`Store`]. +/// +/// This trait is used with [`Store::limiter_async`]: see those docs for +/// restrictions on using other Wasmtime interfaces with an async resource +/// limiter. #[async_trait::async_trait] pub trait ResourceLimiterAsync { /// Async version of [`ResourceLimiter::memory_growing`] @@ -98,6 +109,9 @@ pub trait ResourceLimiterAsync { /// Asynchronous version of [`ResourceLimiter::table_growing`] async fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; + /// Identical to [`ResourceLimiter::table_grow_failed`] + fn table_grow_failed(&mut self, _error: &anyhow::Error) {} + /// Identical to [`ResourceLimiter::instances`]` fn instances(&self) -> usize { DEFAULT_INSTANCE_LIMIT diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index ccacd816e3..44ae7fe3f9 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -1618,6 +1618,19 @@ unsafe impl wasmtime_runtime::Store for StoreInner { } } + fn table_grow_failed(&mut self, error: &anyhow::Error) { + match self.limiter { + Some(ResourceLimiterInner::Sync(ref mut limiter)) => { + limiter(&mut self.data).table_grow_failed(error) + } + #[cfg(feature = "async")] + Some(ResourceLimiterInner::Async(ref mut limiter)) => { + limiter(&mut self.data).table_grow_failed(error) + } + None => {} + } + } + fn out_of_gas(&mut self) -> Result<(), anyhow::Error> { return match &mut self.out_of_gas_behavior { OutOfGas::Trap => Err(anyhow::Error::new(OutOfGasError)), From 538c19589bf2f27e35a1a32d093b69169b692468 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 15:21:05 -0700 Subject: [PATCH 16/29] build out async versions of the existing limiter tests --- tests/all/limits.rs | 192 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/tests/all/limits.rs b/tests/all/limits.rs index c19ab4b047..92a960fa45 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -478,6 +478,118 @@ fn test_custom_limiter() -> Result<()> { Ok(()) } +#[async_trait::async_trait] +impl ResourceLimiterAsync for MemoryContext { + async fn memory_growing( + &mut self, + current: usize, + desired: usize, + maximum: Option, + ) -> bool { + // Check if the desired exceeds a maximum (either from Wasm or from the host) + assert!(desired < maximum.unwrap_or(usize::MAX)); + + assert_eq!(current as usize, self.wasm_memory_used); + let desired = desired as usize; + + if desired + self.host_memory_used > self.memory_limit { + self.limit_exceeded = true; + return false; + } + + self.wasm_memory_used = desired; + true + } + fn memory_grow_failed(&mut self, _e: &anyhow::Error) {} + async fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { + true + } + fn table_grow_failed(&mut self, _e: &anyhow::Error) {} +} + +#[tokio::test] +async fn test_custom_limiter_async() -> Result<()> { + let mut config = Config::new(); + config.async_support(true); + let engine = Engine::new(&config).unwrap(); + let mut linker = Linker::new(&engine); + + // This approximates a function that would "allocate" resources that the host tracks. + // Here this is a simple function that increments the current host memory "used". + linker.func_wrap( + "", + "alloc", + |mut caller: Caller<'_, MemoryContext>, size: u32| -> u32 { + let mut ctx = caller.data_mut(); + let size = size as usize; + + if size + ctx.host_memory_used + ctx.wasm_memory_used <= ctx.memory_limit { + ctx.host_memory_used += size; + return 1; + } + + ctx.limit_exceeded = true; + + 0 + }, + )?; + + let module = Module::new( + &engine, + r#"(module (import "" "alloc" (func $alloc (param i32) (result i32))) (memory (export "m") 0) (func (export "f") (param i32) (result i32) local.get 0 call $alloc))"#, + )?; + + let context = MemoryContext { + host_memory_used: 0, + wasm_memory_used: 0, + memory_limit: 1 << 20, // 16 wasm pages is the limit for both wasm + host memory + limit_exceeded: false, + }; + + let mut store = Store::new(&engine, context); + store.limiter_async(|s| s as &mut dyn ResourceLimiterAsync); + let instance = linker.instantiate_async(&mut store, &module).await?; + let memory = instance.get_memory(&mut store, "m").unwrap(); + + // Grow the memory by 640 KiB + memory.grow_async(&mut store, 3).await?; + memory.grow_async(&mut store, 5).await?; + memory.grow_async(&mut store, 2).await?; + + assert!(!store.data().limit_exceeded); + + // Grow the host "memory" by 384 KiB + let f = instance.get_typed_func::(&mut store, "f")?; + + assert_eq!(f.call_async(&mut store, 1 * 0x10000).await?, 1); + assert_eq!(f.call_async(&mut store, 3 * 0x10000).await?, 1); + assert_eq!(f.call_async(&mut store, 2 * 0x10000).await?, 1); + + // Memory is at the maximum, but the limit hasn't been exceeded + assert!(!store.data().limit_exceeded); + + // Try to grow the memory again + assert_eq!( + memory + .grow_async(&mut store, 1) + .await + .map_err(|e| e.to_string()) + .unwrap_err(), + "failed to grow memory by `1`" + ); + + assert!(store.data().limit_exceeded); + + // Try to grow the host "memory" again + assert_eq!(f.call_async(&mut store, 1).await?, 0); + + assert!(store.data().limit_exceeded); + + drop(store); + + Ok(()) +} + #[derive(Default)] struct MemoryGrowFailureDetector { /// Arguments of most recent call to memory_growing @@ -554,3 +666,83 @@ fn custom_limiter_detect_grow_failure() -> Result<()> { Ok(()) } + +#[async_trait::async_trait] +impl ResourceLimiterAsync for MemoryGrowFailureDetector { + async fn memory_growing( + &mut self, + current: usize, + desired: usize, + _maximum: Option, + ) -> bool { + self.current = current; + self.desired = desired; + true + } + fn memory_grow_failed(&mut self, err: &anyhow::Error) { + self.error = Some(err.to_string()); + } + async fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { + true + } + fn table_grow_failed(&mut self, _err: &anyhow::Error) {} +} + +#[tokio::test] +async fn custom_limiter_async_detect_grow_failure() -> Result<()> { + if std::env::var("WASMTIME_TEST_NO_HOG_MEMORY").is_ok() { + return Ok(()); + } + let mut config = Config::new(); + config.async_support(true); + config.allocation_strategy(InstanceAllocationStrategy::Pooling { + strategy: PoolingAllocationStrategy::NextAvailable, + module_limits: ModuleLimits { + memory_pages: 10, + ..Default::default() + }, + instance_limits: InstanceLimits { + ..Default::default() + }, + }); + let engine = Engine::new(&config).unwrap(); + let linker = Linker::new(&engine); + + let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#)?; + + let context = MemoryGrowFailureDetector::default(); + + let mut store = Store::new(&engine, context); + store.limiter_async(|s| s as &mut dyn ResourceLimiterAsync); + let instance = linker.instantiate_async(&mut store, &module).await?; + let memory = instance.get_memory(&mut store, "m").unwrap(); + + // Grow the memory by 640 KiB (10 pages) + memory.grow_async(&mut store, 10).await?; + + assert!(store.data().error.is_none()); + assert_eq!(store.data().current, 0); + assert_eq!(store.data().desired, 10 * 64 * 1024); + + // Grow past the static limit set by ModuleLimits. + // The ResourcLimiterAsync will permit this, but the grow will fail. + assert_eq!( + memory + .grow_async(&mut store, 1) + .await + .unwrap_err() + .to_string(), + "failed to grow memory by `1`" + ); + + assert_eq!(store.data().current, 10 * 64 * 1024); + assert_eq!(store.data().desired, 11 * 64 * 1024); + assert_eq!( + store.data().error.as_ref().unwrap(), + "Memory maximum size exceeded" + ); + + drop(store); + + Ok(()) +} From 758abe39633081cc5a45719535a195d6f2a93a5a Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 15:50:01 -0700 Subject: [PATCH 17/29] add table limiting tests --- tests/all/limits.rs | 219 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 182 insertions(+), 37 deletions(-) diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 92a960fa45..a41a2ba881 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -399,7 +399,7 @@ impl ResourceLimiter for MemoryContext { } #[test] -fn test_custom_limiter() -> Result<()> { +fn test_custom_memory_limiter() -> Result<()> { let engine = Engine::default(); let mut linker = Linker::new(&engine); @@ -508,7 +508,7 @@ impl ResourceLimiterAsync for MemoryContext { } #[tokio::test] -async fn test_custom_limiter_async() -> Result<()> { +async fn test_custom_memory_limiter_async() -> Result<()> { let mut config = Config::new(); config.async_support(true); let engine = Engine::new(&config).unwrap(); @@ -590,27 +590,108 @@ async fn test_custom_limiter_async() -> Result<()> { Ok(()) } -#[derive(Default)] -struct MemoryGrowFailureDetector { - /// Arguments of most recent call to memory_growing - current: usize, - desired: usize, - /// Display impl of most recent call to memory_grow_failed - error: Option, +struct TableContext { + elements_used: u32, + element_limit: u32, + limit_exceeded: bool, } -impl ResourceLimiter for MemoryGrowFailureDetector { +impl ResourceLimiter for TableContext { + fn memory_growing( + &mut self, + _current: usize, + _desired: usize, + _maximum: Option, + ) -> bool { + true + } + fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool { + // Check if the desired exceeds a maximum (either from Wasm or from the host) + assert!(desired < maximum.unwrap_or(u32::MAX)); + assert_eq!(current, self.elements_used); + if desired > self.element_limit { + self.limit_exceeded = true; + return false; + } else { + self.elements_used = desired; + true + } + } +} + +#[test] +fn test_custom_table_limiter() -> Result<()> { + let engine = Engine::default(); + let linker = Linker::new(&engine); + + let module = Module::new(&engine, r#"(module (table (export "t") 0 anyfunc))"#)?; + + let context = TableContext { + elements_used: 0, + element_limit: 10, + limit_exceeded: false, + }; + + let mut store = Store::new(&engine, context); + store.limiter(|s| s as &mut dyn ResourceLimiter); + let instance = linker.instantiate(&mut store, &module)?; + let table = instance.get_table(&mut store, "t").unwrap(); + + // Grow the memory by 10 elements + table.grow(&mut store, 3, Val::FuncRef(None))?; + table.grow(&mut store, 5, Val::FuncRef(None))?; + table.grow(&mut store, 2, Val::FuncRef(None))?; + + assert!(!store.data().limit_exceeded); + + // Table is at the maximum, but the limit hasn't been exceeded + assert!(!store.data().limit_exceeded); + + // Try to grow the memory again + assert_eq!( + table + .grow(&mut store, 1, Val::FuncRef(None)) + .map_err(|e| e.to_string()) + .unwrap_err(), + "failed to grow table by `1`" + ); + + assert!(store.data().limit_exceeded); + + Ok(()) +} + +#[derive(Default)] +struct FailureDetector { + /// Arguments of most recent call to memory_growing + memory_current: usize, + memory_desired: usize, + /// Display impl of most recent call to memory_grow_failed + memory_error: Option, + /// Arguments of most recent call to table_growing + table_current: u32, + table_desired: u32, + /// Display impl of most recent call to table_grow_failed + table_error: Option, +} + +impl ResourceLimiter for FailureDetector { fn memory_growing(&mut self, current: usize, desired: usize, _maximum: Option) -> bool { - self.current = current; - self.desired = desired; + self.memory_current = current; + self.memory_desired = desired; true } fn memory_grow_failed(&mut self, err: &anyhow::Error) { - self.error = Some(err.to_string()); + self.memory_error = Some(err.to_string()); } - fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { + fn table_growing(&mut self, current: u32, desired: u32, _maximum: Option) -> bool { + self.table_current = current; + self.table_desired = desired; true } + fn table_grow_failed(&mut self, err: &anyhow::Error) { + self.table_error = Some(err.to_string()); + } } #[test] @@ -623,6 +704,7 @@ fn custom_limiter_detect_grow_failure() -> Result<()> { strategy: PoolingAllocationStrategy::NextAvailable, module_limits: ModuleLimits { memory_pages: 10, + table_elements: 10, ..Default::default() }, instance_limits: InstanceLimits { @@ -632,9 +714,12 @@ fn custom_limiter_detect_grow_failure() -> Result<()> { let engine = Engine::new(&config).unwrap(); let linker = Linker::new(&engine); - let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#)?; + let module = Module::new( + &engine, + r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, + )?; - let context = MemoryGrowFailureDetector::default(); + let context = FailureDetector::default(); let mut store = Store::new(&engine, context); store.limiter(|s| s as &mut dyn ResourceLimiter); @@ -644,48 +729,78 @@ fn custom_limiter_detect_grow_failure() -> Result<()> { // Grow the memory by 640 KiB (10 pages) memory.grow(&mut store, 10)?; - assert!(store.data().error.is_none()); - assert_eq!(store.data().current, 0); - assert_eq!(store.data().desired, 10 * 64 * 1024); + assert!(store.data().memory_error.is_none()); + assert_eq!(store.data().memory_current, 0); + assert_eq!(store.data().memory_desired, 10 * 64 * 1024); // Grow past the static limit set by ModuleLimits. - // The ResourcLimiter will permit this, but the grow will fail. + // The ResourceLimiter will permit this, but the grow will fail. assert_eq!( memory.grow(&mut store, 1).unwrap_err().to_string(), "failed to grow memory by `1`" ); - assert_eq!(store.data().current, 10 * 64 * 1024); - assert_eq!(store.data().desired, 11 * 64 * 1024); + assert_eq!(store.data().memory_current, 10 * 64 * 1024); + assert_eq!(store.data().memory_desired, 11 * 64 * 1024); assert_eq!( - store.data().error.as_ref().unwrap(), + store.data().memory_error.as_ref().unwrap(), "Memory maximum size exceeded" ); + let table = instance.get_table(&mut store, "t").unwrap(); + // Grow the table 10 elements + table.grow(&mut store, 10, Val::FuncRef(None))?; + + assert!(store.data().table_error.is_none()); + assert_eq!(store.data().table_current, 0); + assert_eq!(store.data().table_desired, 10); + + // Grow past the static limit set by ModuleLimits. + // The ResourceLimiter will permit this, but the grow will fail. + assert_eq!( + table + .grow(&mut store, 1, Val::FuncRef(None)) + .unwrap_err() + .to_string(), + "failed to grow table by `1`" + ); + + assert_eq!(store.data().table_current, 10); + assert_eq!(store.data().table_desired, 11); + assert_eq!( + store.data().table_error.as_ref().unwrap(), + "Table maximum size exceeded" + ); + drop(store); Ok(()) } #[async_trait::async_trait] -impl ResourceLimiterAsync for MemoryGrowFailureDetector { +impl ResourceLimiterAsync for FailureDetector { async fn memory_growing( &mut self, current: usize, desired: usize, _maximum: Option, ) -> bool { - self.current = current; - self.desired = desired; + self.memory_current = current; + self.memory_desired = desired; true } fn memory_grow_failed(&mut self, err: &anyhow::Error) { - self.error = Some(err.to_string()); + self.memory_error = Some(err.to_string()); } - async fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { + + async fn table_growing(&mut self, current: u32, desired: u32, _maximum: Option) -> bool { + self.table_current = current; + self.table_desired = desired; true } - fn table_grow_failed(&mut self, _err: &anyhow::Error) {} + fn table_grow_failed(&mut self, err: &anyhow::Error) { + self.table_error = Some(err.to_string()); + } } #[tokio::test] @@ -699,6 +814,7 @@ async fn custom_limiter_async_detect_grow_failure() -> Result<()> { strategy: PoolingAllocationStrategy::NextAvailable, module_limits: ModuleLimits { memory_pages: 10, + table_elements: 10, ..Default::default() }, instance_limits: InstanceLimits { @@ -708,9 +824,12 @@ async fn custom_limiter_async_detect_grow_failure() -> Result<()> { let engine = Engine::new(&config).unwrap(); let linker = Linker::new(&engine); - let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#)?; + let module = Module::new( + &engine, + r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, + )?; - let context = MemoryGrowFailureDetector::default(); + let context = FailureDetector::default(); let mut store = Store::new(&engine, context); store.limiter_async(|s| s as &mut dyn ResourceLimiterAsync); @@ -720,9 +839,9 @@ async fn custom_limiter_async_detect_grow_failure() -> Result<()> { // Grow the memory by 640 KiB (10 pages) memory.grow_async(&mut store, 10).await?; - assert!(store.data().error.is_none()); - assert_eq!(store.data().current, 0); - assert_eq!(store.data().desired, 10 * 64 * 1024); + assert!(store.data().memory_error.is_none()); + assert_eq!(store.data().memory_current, 0); + assert_eq!(store.data().memory_desired, 10 * 64 * 1024); // Grow past the static limit set by ModuleLimits. // The ResourcLimiterAsync will permit this, but the grow will fail. @@ -735,13 +854,39 @@ async fn custom_limiter_async_detect_grow_failure() -> Result<()> { "failed to grow memory by `1`" ); - assert_eq!(store.data().current, 10 * 64 * 1024); - assert_eq!(store.data().desired, 11 * 64 * 1024); + assert_eq!(store.data().memory_current, 10 * 64 * 1024); + assert_eq!(store.data().memory_desired, 11 * 64 * 1024); assert_eq!( - store.data().error.as_ref().unwrap(), + store.data().memory_error.as_ref().unwrap(), "Memory maximum size exceeded" ); + let table = instance.get_table(&mut store, "t").unwrap(); + // Grow the table 10 elements + table.grow_async(&mut store, 10, Val::FuncRef(None)).await?; + + assert!(store.data().table_error.is_none()); + assert_eq!(store.data().table_current, 0); + assert_eq!(store.data().table_desired, 10); + + // Grow past the static limit set by ModuleLimits. + // The ResourceLimiter will permit this, but the grow will fail. + assert_eq!( + table + .grow_async(&mut store, 1, Val::FuncRef(None)) + .await + .unwrap_err() + .to_string(), + "failed to grow table by `1`" + ); + + assert_eq!(store.data().table_current, 10); + assert_eq!(store.data().table_desired, 11); + assert_eq!( + store.data().table_error.as_ref().unwrap(), + "Table maximum size exceeded" + ); + drop(store); Ok(()) From 3fd674c6bc824ffe35592c878b75052b7bfb1a04 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 16:36:37 -0700 Subject: [PATCH 18/29] async memory_grow_failed can have a default impl idk why this didnt work in the old factoring! but im glad it does --- crates/wasmtime/src/limits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasmtime/src/limits.rs b/crates/wasmtime/src/limits.rs index 02d6fcd814..7ca9cb07f1 100644 --- a/crates/wasmtime/src/limits.rs +++ b/crates/wasmtime/src/limits.rs @@ -104,7 +104,7 @@ pub trait ResourceLimiterAsync { ) -> bool; /// Identical to [`ResourceLimiter::memory_grow_failed`] - fn memory_grow_failed(&mut self, error: &anyhow::Error); + fn memory_grow_failed(&mut self, _error: &anyhow::Error) {} /// Asynchronous version of [`ResourceLimiter::table_growing`] async fn table_growing(&mut self, current: u32, desired: u32, maximum: Option) -> bool; From 175e72bac40f2a89de457195150978b54b9cfbc8 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 16:43:13 -0700 Subject: [PATCH 19/29] test that the catch unwind works --- tests/all/limits.rs | 109 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 3 deletions(-) diff --git a/tests/all/limits.rs b/tests/all/limits.rs index a41a2ba881..05585657e9 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -89,7 +89,6 @@ async fn test_limits_async() -> Result<()> { ) -> bool { desired <= self.memory_size } - fn memory_grow_failed(&mut self, _error: &anyhow::Error) {} async fn table_growing( &mut self, _current: u32, @@ -500,7 +499,6 @@ impl ResourceLimiterAsync for MemoryContext { self.wasm_memory_used = desired; true } - fn memory_grow_failed(&mut self, _e: &anyhow::Error) {} async fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { true } @@ -637,7 +635,7 @@ fn test_custom_table_limiter() -> Result<()> { let instance = linker.instantiate(&mut store, &module)?; let table = instance.get_table(&mut store, "t").unwrap(); - // Grow the memory by 10 elements + // Grow the table by 10 elements table.grow(&mut store, 3, Val::FuncRef(None))?; table.grow(&mut store, 5, Val::FuncRef(None))?; table.grow(&mut store, 2, Val::FuncRef(None))?; @@ -891,3 +889,108 @@ async fn custom_limiter_async_detect_grow_failure() -> Result<()> { Ok(()) } + +struct Panic; + +impl ResourceLimiter for Panic { + fn memory_growing( + &mut self, + _current: usize, + _desired: usize, + _maximum: Option, + ) -> bool { + panic!("resource limiter memory growing"); + } + fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { + panic!("resource limiter table growing"); + } +} +#[async_trait::async_trait] +impl ResourceLimiterAsync for Panic { + async fn memory_growing( + &mut self, + _current: usize, + _desired: usize, + _maximum: Option, + ) -> bool { + panic!("async resource limiter memory growing"); + } + async fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { + panic!("async resource limiter table growing"); + } +} + +#[test] +#[should_panic(expected = "resource limiter memory growing")] +fn panic_in_memory_limiter() { + let engine = Engine::default(); + let linker = Linker::new(&engine); + + let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#).unwrap(); + + let mut store = Store::new(&engine, Panic); + store.limiter(|s| s as &mut dyn ResourceLimiter); + let instance = linker.instantiate(&mut store, &module).unwrap(); + let memory = instance.get_memory(&mut store, "m").unwrap(); + + // Grow the memory, which should panic + memory.grow(&mut store, 3).unwrap(); +} + +#[test] +#[should_panic(expected = "resource limiter table growing")] +fn panic_in_table_limiter() { + let engine = Engine::default(); + let linker = Linker::new(&engine); + + let module = Module::new(&engine, r#"(module (table (export "t") 0 anyfunc))"#).unwrap(); + + let mut store = Store::new(&engine, Panic); + store.limiter(|s| s as &mut dyn ResourceLimiter); + let instance = linker.instantiate(&mut store, &module).unwrap(); + let table = instance.get_table(&mut store, "t").unwrap(); + + // Grow the table, which should panic + table.grow(&mut store, 3, Val::FuncRef(None)).unwrap(); +} + +#[tokio::test] +#[should_panic(expected = "async resource limiter memory growing")] +async fn panic_in_async_memory_limiter() { + let mut config = Config::new(); + config.async_support(true); + let engine = Engine::new(&config).unwrap(); + let linker = Linker::new(&engine); + + let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#).unwrap(); + + let mut store = Store::new(&engine, Panic); + store.limiter_async(|s| s as &mut dyn ResourceLimiterAsync); + let instance = linker.instantiate_async(&mut store, &module).await.unwrap(); + let memory = instance.get_memory(&mut store, "m").unwrap(); + + // Grow the memory, which should panic + memory.grow_async(&mut store, 3).await.unwrap(); +} + +#[tokio::test] +#[should_panic(expected = "async resource limiter table growing")] +async fn panic_in_async_table_limiter() { + let mut config = Config::new(); + config.async_support(true); + let engine = Engine::new(&config).unwrap(); + let linker = Linker::new(&engine); + + let module = Module::new(&engine, r#"(module (table (export "t") 0 anyfunc))"#).unwrap(); + + let mut store = Store::new(&engine, Panic); + store.limiter_async(|s| s as &mut dyn ResourceLimiterAsync); + let instance = linker.instantiate_async(&mut store, &module).await.unwrap(); + let table = instance.get_table(&mut store, "t").unwrap(); + + // Grow the table, which should panic + table + .grow_async(&mut store, 3, Val::FuncRef(None)) + .await + .unwrap(); +} From 0370d5c1a25c502f45dde10acf2b02968df80b6e Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 Oct 2021 16:46:31 -0700 Subject: [PATCH 20/29] code review suggestion --- crates/runtime/src/libcalls.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 9aba075bc6..1c648f0bf1 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -190,7 +190,9 @@ pub unsafe extern "C" fn wasmtime_memory32_grow( delta: u64, memory_index: u32, ) -> usize { - std::panic::catch_unwind(|| { + // Memory grow can invoke user code provided in a ResourceLimiter{,Async}, + // so we need to catch a possible panic + match std::panic::catch_unwind(|| { let instance = (*vmctx).instance_mut(); let memory_index = MemoryIndex::from_u32(memory_index); match instance.memory_grow(memory_index, delta) { @@ -198,9 +200,10 @@ pub unsafe extern "C" fn wasmtime_memory32_grow( Ok(None) => usize::max_value(), Err(err) => crate::traphandlers::raise_user_trap(err), } - }) - .map_err(|panic| resume_panic(panic)) - .unwrap() + }) { + Ok(r) => r, + Err(p) => resume_panic(p), + } } /// Implementation of `table.grow`. @@ -212,7 +215,9 @@ pub unsafe extern "C" fn wasmtime_table_grow( // or is a `VMExternRef` until we look at the table type. init_value: *mut u8, ) -> u32 { - std::panic::catch_unwind(|| { + // Table grow can invoke user code provided in a ResourceLimiter{,Async}, + // so we need to catch a possible panic + match std::panic::catch_unwind(|| { let instance = (*vmctx).instance_mut(); let table_index = TableIndex::from_u32(table_index); let element = match instance.table_element_type(table_index) { @@ -231,9 +236,10 @@ pub unsafe extern "C" fn wasmtime_table_grow( Ok(None) => -1_i32 as u32, Err(err) => crate::traphandlers::raise_user_trap(err), } - }) - .map_err(|panic| resume_panic(panic)) - .unwrap() + }) { + Ok(r) => r, + Err(p) => resume_panic(p), + } } /// Implementation of `table.fill`. From efef0769fe0edf337224275529566404c10b858c Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 22 Oct 2021 08:39:00 -0700 Subject: [PATCH 21/29] make uffd test compile, but not pass --- crates/runtime/src/instance/allocator/pooling/uffd.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/runtime/src/instance/allocator/pooling/uffd.rs b/crates/runtime/src/instance/allocator/pooling/uffd.rs index e82f932eba..bc5fda170c 100644 --- a/crates/runtime/src/instance/allocator/pooling/uffd.rs +++ b/crates/runtime/src/instance/allocator/pooling/uffd.rs @@ -436,7 +436,7 @@ mod test { use super::*; use crate::{ Imports, InstanceAllocationRequest, InstanceLimits, ModuleLimits, - PoolingAllocationStrategy, VMSharedSignatureIndex, + PoolingAllocationStrategy, StorePtr, VMSharedSignatureIndex, }; use std::sync::Arc; use wasmtime_environ::{Memory, MemoryPlan, MemoryStyle, Module, PrimaryMap, Tunables}; @@ -528,7 +528,7 @@ mod test { }, shared_signatures: VMSharedSignatureIndex::default().into(), host_state: Box::new(()), - store: None, + store: StorePtr::empty(), // XXX need a real store here: passing in a module implies a non-null store wasm_data: &[], }, ) From 52542b6c01f8fb6c9aa025deb4a93242861737f6 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 22 Oct 2021 08:54:42 -0700 Subject: [PATCH 22/29] mock enough of the store to pass the uffd test --- .../src/instance/allocator/pooling/uffd.rs | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/crates/runtime/src/instance/allocator/pooling/uffd.rs b/crates/runtime/src/instance/allocator/pooling/uffd.rs index bc5fda170c..df6f18f13b 100644 --- a/crates/runtime/src/instance/allocator/pooling/uffd.rs +++ b/crates/runtime/src/instance/allocator/pooling/uffd.rs @@ -436,7 +436,7 @@ mod test { use super::*; use crate::{ Imports, InstanceAllocationRequest, InstanceLimits, ModuleLimits, - PoolingAllocationStrategy, StorePtr, VMSharedSignatureIndex, + PoolingAllocationStrategy, Store, StorePtr, VMSharedSignatureIndex, }; use std::sync::Arc; use wasmtime_environ::{Memory, MemoryPlan, MemoryStyle, Module, PrimaryMap, Tunables}; @@ -506,6 +506,58 @@ mod test { module_limits.validate(&module).expect("should validate"); + // An InstanceAllocationRequest with a module must also have + // a non-null StorePtr. Here we mock just enough of a store + // to satisfy this test. + struct MockStore { + table: crate::VMExternRefActivationsTable, + info: MockModuleInfo, + } + unsafe impl Store for MockStore { + fn vminterrupts(&self) -> *mut crate::VMInterrupts { + std::ptr::null_mut() + } + fn externref_activations_table( + &mut self, + ) -> ( + &mut crate::VMExternRefActivationsTable, + &dyn crate::ModuleInfoLookup, + ) { + (&mut self.table, &self.info) + } + fn memory_growing( + &mut self, + _current: usize, + _desired: usize, + _maximum: Option, + ) -> Result { + Ok(true) + } + fn memory_grow_failed(&mut self, _error: &anyhow::Error) {} + fn table_growing( + &mut self, + _current: u32, + _desired: u32, + _maximum: Option, + ) -> Result { + Ok(true) + } + fn table_grow_failed(&mut self, _error: &anyhow::Error) {} + fn out_of_gas(&mut self) -> Result<(), anyhow::Error> { + Ok(()) + } + } + struct MockModuleInfo; + impl crate::ModuleInfoLookup for MockModuleInfo { + fn lookup(&self, _pc: usize) -> Option> { + None + } + } + let mut mock_store = MockStore { + table: crate::VMExternRefActivationsTable::new(), + info: MockModuleInfo, + }; + let mut handles = Vec::new(); let module = Arc::new(module); let functions = &PrimaryMap::new(); @@ -528,7 +580,7 @@ mod test { }, shared_signatures: VMSharedSignatureIndex::default().into(), host_state: Box::new(()), - store: StorePtr::empty(), // XXX need a real store here: passing in a module implies a non-null store + store: StorePtr::new(&mut mock_store), wasm_data: &[], }, ) From b00d811e83c518917284c5860f848eb7fb2f53fc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 22 Oct 2021 10:43:46 -0700 Subject: [PATCH 23/29] code review --- crates/runtime/src/libcalls.rs | 20 ++++++++------------ crates/wasmtime/src/func.rs | 4 +--- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 1c648f0bf1..8d5130950f 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -195,13 +195,11 @@ pub unsafe extern "C" fn wasmtime_memory32_grow( match std::panic::catch_unwind(|| { let instance = (*vmctx).instance_mut(); let memory_index = MemoryIndex::from_u32(memory_index); - match instance.memory_grow(memory_index, delta) { - Ok(Some(size_in_bytes)) => size_in_bytes / (wasmtime_environ::WASM_PAGE_SIZE as usize), - Ok(None) => usize::max_value(), - Err(err) => crate::traphandlers::raise_user_trap(err), - } + instance.memory_grow(memory_index, delta) }) { - Ok(r) => r, + Ok(Ok(Some(size_in_bytes))) => size_in_bytes / (wasmtime_environ::WASM_PAGE_SIZE as usize), + Ok(Ok(None)) => usize::max_value(), + Ok(Err(err)) => crate::traphandlers::raise_user_trap(err), Err(p) => resume_panic(p), } } @@ -231,13 +229,11 @@ pub unsafe extern "C" fn wasmtime_table_grow( init_value.into() } }; - match instance.table_grow(table_index, delta, element) { - Ok(Some(r)) => r, - Ok(None) => -1_i32 as u32, - Err(err) => crate::traphandlers::raise_user_trap(err), - } + instance.table_grow(table_index, delta, element) }) { - Ok(r) => r, + Ok(Ok(Some(r))) => r, + Ok(Ok(None)) => -1_i32 as u32, + Ok(Err(err)) => crate::traphandlers::raise_user_trap(err), Err(p) => resume_panic(p), } } diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 068d0b4aaa..c011b52d27 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -1880,9 +1880,7 @@ macro_rules! impl_into_func { // can't assume it returned a value that is // compatible with this store. if !ret.compatible_with_store(caller.store.0) { - CallResult::Trap(anyhow::anyhow!( - "host function attempted to return cross-`Store` value to Wasm" - )) + CallResult::Trap(anyhow::anyhow!("host function attempted to return cross-`Store` value to Wasm")) } else { match ret.into_abi_for_ret(caller.store.0, retptr) { Ok(val) => CallResult::Ok(val), From bcbd74678a484d5b568b699a4b80c165404e9b4d Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 22 Oct 2021 11:08:09 -0700 Subject: [PATCH 24/29] add some tests that show behavior is the same when on wasm stack --- tests/all/limits.rs | 87 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 05585657e9..04cb0f5769 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -8,7 +8,12 @@ fn test_limits() -> Result<()> { let engine = Engine::default(); let module = Module::new( &engine, - r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, + r#"(module + (memory $m (export "m") 0) + (table (export "t") 0 anyfunc) + (func (export "grow") (param i32) (result i32) + (memory.grow $m (local.get 0))) + )"#, )?; let mut store = Store::new( @@ -62,6 +67,26 @@ fn test_limits() -> Result<()> { ); } + // Make a new store and instance to test memory grow through wasm + let mut store = Store::new( + &engine, + StoreLimitsBuilder::new() + .memory_size(10 * WASM_PAGE_SIZE) + .table_elements(5) + .build(), + ); + store.limiter(|s| s as &mut dyn ResourceLimiter); + let instance = Instance::new(&mut store, &module, &[])?; + let grow = instance.get_func(&mut store, "grow").unwrap(); + let grow = grow.typed::(&store).unwrap(); + + grow.call(&mut store, 3).unwrap(); + grow.call(&mut store, 5).unwrap(); + grow.call(&mut store, 2).unwrap(); + + // Wasm grow failure returns -1. + assert_eq!(grow.call(&mut store, 1).unwrap(), -1); + Ok(()) } @@ -937,6 +962,35 @@ fn panic_in_memory_limiter() { memory.grow(&mut store, 3).unwrap(); } +#[test] +#[should_panic(expected = "resource limiter memory growing")] +fn panic_in_memory_limiter_wasm_stack() { + // Like the test above, except the memory.grow happens in wasm code + // instead of a host function call. + let engine = Engine::default(); + let linker = Linker::new(&engine); + + let module = Module::new( + &engine, + r#" + (module + (memory $m (export "m") 0) + (func (export "grow") (param i32) (result i32) + (memory.grow $m (local.get 0))) + )"#, + ) + .unwrap(); + + let mut store = Store::new(&engine, Panic); + store.limiter(|s| s as &mut dyn ResourceLimiter); + let instance = linker.instantiate(&mut store, &module).unwrap(); + let grow = instance.get_func(&mut store, "grow").unwrap(); + let grow = grow.typed::(&store).unwrap(); + + // Grow the memory, which should panic + grow.call(&mut store, 3).unwrap(); +} + #[test] #[should_panic(expected = "resource limiter table growing")] fn panic_in_table_limiter() { @@ -973,6 +1027,37 @@ async fn panic_in_async_memory_limiter() { memory.grow_async(&mut store, 3).await.unwrap(); } +#[tokio::test] +#[should_panic(expected = "async resource limiter memory growing")] +async fn panic_in_async_memory_limiter_wasm_stack() { + // Like the test above, except the memory.grow happens in + // wasm code instead of a host function call. + let mut config = Config::new(); + config.async_support(true); + let engine = Engine::new(&config).unwrap(); + let linker = Linker::new(&engine); + + let module = Module::new( + &engine, + r#" + (module + (memory $m (export "m") 0) + (func (export "grow") (param i32) (result i32) + (memory.grow $m (local.get 0))) + )"#, + ) + .unwrap(); + + let mut store = Store::new(&engine, Panic); + store.limiter_async(|s| s as &mut dyn ResourceLimiterAsync); + let instance = linker.instantiate_async(&mut store, &module).await.unwrap(); + let grow = instance.get_func(&mut store, "grow").unwrap(); + let grow = grow.typed::(&store).unwrap(); + + // Grow the memory, which should panic + grow.call_async(&mut store, 3).await.unwrap(); +} + #[tokio::test] #[should_panic(expected = "async resource limiter table growing")] async fn panic_in_async_table_limiter() { From 996289725d69c3b1a920fa596f762ab42f2c50c5 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 22 Oct 2021 11:44:54 -0700 Subject: [PATCH 25/29] docs --- crates/wasmtime/src/externals.rs | 10 ++++++++++ crates/wasmtime/src/memory.rs | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 686b154472..d75b11c6c7 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -406,6 +406,12 @@ impl Table { /// Returns an error if `init` does not match the element type of the table, /// or if `init` does not belong to the `store` provided. /// + /// # Panics + /// + /// This function will panic when used with a [`Store`] which has a + /// [`ResourceLimiterAsync`] (see also: [`Store::limiter_async`]). When + /// using an async resource limiter, use [`Table::new_async`] instead. + /// /// # Examples /// /// ``` @@ -570,6 +576,10 @@ impl Table { /// # Panics /// /// Panics if `store` does not own this table. + /// + /// This function will panic when used with a [`Store`] which has a + /// [`ResourceLimiterAsync`] (see also: [`Store::limiter_async`]). When + /// using an async resource limiter, use [`Table::grow_async`] instead. pub fn grow(&self, mut store: impl AsContextMut, delta: u32, init: Val) -> Result { let store = store.as_context_mut().0; let ty = self.ty(&store).element().clone(); diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index 3f22f3dd2f..b01fa366b8 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -202,6 +202,12 @@ impl Memory { /// The `store` argument will be the owner of the returned [`Memory`]. All /// WebAssembly memory is initialized to zero. /// + /// # Panics + /// + /// This function will panic if the [`Store`] has a + /// [`ResourceLimiterAsync`] (see also: [`Store::limiter_async`]). When + /// using an async resource limiter, use [`Memory::new_async`] instead. + /// /// # Examples /// /// ``` @@ -459,6 +465,10 @@ impl Memory { /// /// Panics if this memory doesn't belong to `store`. /// + /// This function will panic if the [`Store`] has a + /// [`ResourceLimiterAsync`] (see also: [`Store::limiter_async`]). When + /// using an async resource limiter, use [`Memory::grow_async`] instead. + /// /// # Examples /// /// ``` From 681945908213a61ff137f5b607a9fa25b49ad6b9 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 22 Oct 2021 12:00:56 -0700 Subject: [PATCH 26/29] fix all docs links --- crates/wasmtime/src/externals.rs | 30 +++++++++++++++++++----------- crates/wasmtime/src/limits.rs | 7 ++++--- crates/wasmtime/src/memory.rs | 26 ++++++++++++++++---------- crates/wasmtime/src/store.rs | 24 +++++++++++++++--------- 4 files changed, 54 insertions(+), 33 deletions(-) diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index d75b11c6c7..a35216eb09 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -408,9 +408,11 @@ impl Table { /// /// # Panics /// - /// This function will panic when used with a [`Store`] which has a - /// [`ResourceLimiterAsync`] (see also: [`Store::limiter_async`]). When - /// using an async resource limiter, use [`Table::new_async`] instead. + /// This function will panic when used with a [`Store`](`crate::Store`) + /// which has a [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`) + /// (see also: [`Store::limiter_async`](`crate::Store::limiter_async`). + /// When using an async resource limiter, use [`Table::new_async`] + /// instead. /// /// # Examples /// @@ -442,12 +444,14 @@ impl Table { Table::_new(store.as_context_mut().0, ty, init) } - /// Async variant of [`Table::new`]. You must use this variant with [`Store`]s which have a - /// [`ResourceLimiterAsync`]. + /// Async variant of [`Table::new`]. You must use this variant with + /// [`Store`](`crate::Store`)s which have a + /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`). /// /// # Panics /// - /// This function will panic when used with a non-async [`Store`]. + /// This function will panic when used with a non-async + /// [`Store`](`crate::Store`) #[cfg(feature = "async")] pub async fn new_async( mut store: impl AsContextMut, @@ -577,9 +581,11 @@ impl Table { /// /// Panics if `store` does not own this table. /// - /// This function will panic when used with a [`Store`] which has a - /// [`ResourceLimiterAsync`] (see also: [`Store::limiter_async`]). When - /// using an async resource limiter, use [`Table::grow_async`] instead. + /// This function will panic when used with a [`Store`](`crate::Store`) + /// which has a [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`) + /// (see also: [`Store::limiter_async`](`crate::Store::limiter_async`)). + /// When using an async resource limiter, use [`Table::grow_async`] + /// instead. pub fn grow(&self, mut store: impl AsContextMut, delta: u32, init: Val) -> Result { let store = store.as_context_mut().0; let ty = self.ty(&store).element().clone(); @@ -597,11 +603,13 @@ impl Table { } } - /// Async variant of [`Table::grow`]. Required when using a [`ResourceLimiterAsync`]. + /// Async variant of [`Table::grow`]. Required when using a + /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`). /// /// # Panics /// - /// This function will panic when used with a non-async [`Store`]. + /// This function will panic when used with a non-async + /// [`Store`](`crate::Store`). #[cfg(feature = "async")] pub async fn grow_async( &self, diff --git a/crates/wasmtime/src/limits.rs b/crates/wasmtime/src/limits.rs index 7ca9cb07f1..687f01371c 100644 --- a/crates/wasmtime/src/limits.rs +++ b/crates/wasmtime/src/limits.rs @@ -88,10 +88,11 @@ pub trait ResourceLimiter { #[cfg(feature = "async")] /// Used by hosts to limit resource consumption of instances. Identical to /// [`ResourceLimiter`], except that the `memory_growing` and `table_growing` -/// functions are async. Must be used with an async [`Store`]. +/// functions are async. Must be used with an async [`Store`](`crate::Store`). /// -/// This trait is used with [`Store::limiter_async`]: see those docs for -/// restrictions on using other Wasmtime interfaces with an async resource +/// This trait is used with +/// [`Store::limiter_async`](`crate::Store::limiter_async`)`: see those docs +/// for restrictions on using other Wasmtime interfaces with an async resource /// limiter. #[async_trait::async_trait] pub trait ResourceLimiterAsync { diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index b01fa366b8..9165807cd7 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -204,8 +204,9 @@ impl Memory { /// /// # Panics /// - /// This function will panic if the [`Store`] has a - /// [`ResourceLimiterAsync`] (see also: [`Store::limiter_async`]). When + /// This function will panic if the [`Store`](`crate::Store`) has a + /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`) (see also: + /// [`Store::limiter_async`](`crate::Store::limiter_async`)). When /// using an async resource limiter, use [`Memory::new_async`] instead. /// /// # Examples @@ -229,12 +230,14 @@ impl Memory { Memory::_new(store.as_context_mut().0, ty) } - /// Async variant of [`Memory::new`]. You must use this variant with [`Store`]s which have a - /// [`ResourceLimiterAsync`]. + /// Async variant of [`Memory::new`]. You must use this variant with + /// [`Store`](`crate::Store`)s which have a + /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`). /// /// # Panics /// - /// This function will panic when used with a non-async [`Store`]. + /// This function will panic when used with a non-async + /// [`Store`](`crate::Store`). #[cfg(feature = "async")] pub async fn new_async( mut store: impl AsContextMut, @@ -465,9 +468,10 @@ impl Memory { /// /// Panics if this memory doesn't belong to `store`. /// - /// This function will panic if the [`Store`] has a - /// [`ResourceLimiterAsync`] (see also: [`Store::limiter_async`]). When - /// using an async resource limiter, use [`Memory::grow_async`] instead. + /// This function will panic if the [`Store`](`crate::Store`) has a + /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`) (see also: + /// [`Store::limiter_async`](`crate::Store::limiter_async`). When using an + /// async resource limiter, use [`Memory::grow_async`] instead. /// /// # Examples /// @@ -504,11 +508,13 @@ impl Memory { } } - /// Async variant of [`Memory::grow`]. Required when using a [`ResourceLimiterAsync`]. + /// Async variant of [`Memory::grow`]. Required when using a + /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`). /// /// # Panics /// - /// This function will panic when used with a non-async [`Store`]. + /// This function will panic when used with a non-async + /// [`Store`](`crate::Store`). #[cfg(feature = "async")] pub async fn grow_async( &self, diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 44ae7fe3f9..6c3c85ccc9 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -541,16 +541,22 @@ impl Store { /// resources in the future, this does not retroactively attempt to apply /// limits to the [`Store`]. /// - /// This variation on the [`ResourceLimiter`] makes the `memory_growing` - /// and `table_growing` functions `async`. This means that, as part of - /// your resource limiting strategy, the async resource limiter may yield - /// execution until a resource becomes available. + /// This variation on the [`ResourceLimiter`](`crate::ResourceLimiter`) + /// makes the `memory_growing` and `table_growing` functions `async`. This + /// means that, as part of your resource limiting strategy, the async + /// resource limiter may yield execution until a resource becomes + /// available. /// - /// By using a [`ResourceLimiterAsync`] with a [`Store`], you can no - /// longer use [`Memory::new`], [`Memory::grow`], [`Table::new`], and - /// [`Table::grow`]. Instead, you must use their `async` variants: - /// [`Memory::new_async`], [`Memory::grow_async`], [`Table::new_async`], - /// and [`Table::grow_async`]. + /// By using a [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`) + /// with a [`Store`], you can no longer use + /// [`Memory::new`](`crate::Memory::new`), + /// [`Memory::grow`](`crate::Memory::grow`), + /// [`Table::new`](`crate::Table::new`), and + /// [`Table::grow`](`crate::Table::grow`). Instead, you must use their + /// `async` variants: [`Memory::new_async`](`crate::Memory::new_async`), + /// [`Memory::grow_async`](`crate::Memory::grow_async`), + /// [`Table::new_async`](`crate::Table::new_async`), and + /// [`Table::grow_async`](`crate::Table::grow_async`). #[cfg(feature = "async")] pub fn limiter_async( &mut self, From 5f978dbfddd07c8facb97c90196b1a6c971c9521 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 22 Oct 2021 14:03:04 -0700 Subject: [PATCH 27/29] make feature requirement render in rustdoc for new apis --- crates/wasmtime/src/externals.rs | 2 ++ crates/wasmtime/src/memory.rs | 2 ++ crates/wasmtime/src/store.rs | 1 + 3 files changed, 5 insertions(+) diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index a35216eb09..047291ba34 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -444,6 +444,7 @@ impl Table { Table::_new(store.as_context_mut().0, ty, init) } + #[cfg_attr(nightlydoc, doc(cfg(feature = "async")))] /// Async variant of [`Table::new`]. You must use this variant with /// [`Store`](`crate::Store`)s which have a /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`). @@ -603,6 +604,7 @@ impl Table { } } + #[cfg_attr(nightlydoc, doc(cfg(feature = "async")))] /// Async variant of [`Table::grow`]. Required when using a /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`). /// diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index 9165807cd7..596e11537f 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -230,6 +230,7 @@ impl Memory { Memory::_new(store.as_context_mut().0, ty) } + #[cfg_attr(nightlydoc, doc(cfg(feature = "async")))] /// Async variant of [`Memory::new`]. You must use this variant with /// [`Store`](`crate::Store`)s which have a /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`). @@ -508,6 +509,7 @@ impl Memory { } } + #[cfg_attr(nightlydoc, doc(cfg(feature = "async")))] /// Async variant of [`Memory::grow`]. Required when using a /// [`ResourceLimiterAsync`](`crate::ResourceLimiterAsync`). /// diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 6c3c85ccc9..38bda4a734 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -533,6 +533,7 @@ impl Store { inner.limiter = Some(ResourceLimiterInner::Sync(Box::new(limiter))); } + #[cfg_attr(nightlydoc, doc(cfg(feature = "async")))] /// Configures the [`ResourceLimiterAsync`](crate::ResourceLimiterAsync) /// used to limit resource creation within this [`Store`]. Must be used /// with an async `Store`!. From fb549c6ddb7136403f699b6426c90c854f1c97d0 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 22 Oct 2021 16:12:07 -0700 Subject: [PATCH 28/29] actually do some awaiting in the async limiter, which doesnt work something tls-related is not right --- tests/all/limits.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 04cb0f5769..1515197ff9 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -510,6 +510,8 @@ impl ResourceLimiterAsync for MemoryContext { desired: usize, maximum: Option, ) -> bool { + // Show we can await in this async context: + tokio::time::sleep(std::time::Duration::from_millis(1)).await; // Check if the desired exceeds a maximum (either from Wasm or from the host) assert!(desired < maximum.unwrap_or(usize::MAX)); @@ -808,6 +810,8 @@ impl ResourceLimiterAsync for FailureDetector { desired: usize, _maximum: Option, ) -> bool { + // Show we can await in this async context: + tokio::time::sleep(std::time::Duration::from_millis(1)).await; self.memory_current = current; self.memory_desired = desired; true From 2f2c5231b46d14c1c9a9fef37c0df880d05c5d49 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 25 Oct 2021 10:04:31 -0700 Subject: [PATCH 29/29] Add Alex's solution for null handling in TlsRestore --- crates/runtime/src/traphandlers.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index 3a1bfac8ed..af60c2f4c3 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -431,9 +431,12 @@ mod tls { // null out our own previous field for safety in case it's // accidentally used later. let raw = raw::get(); - assert!(!raw.is_null()); - let prev = (*raw).prev.replace(ptr::null()); - raw::replace(prev)?; + if !raw.is_null() { + let prev = (*raw).prev.replace(ptr::null()); + raw::replace(prev)?; + } + // Null case: we aren't in a wasm context, so theres no tls + // to save for restoration. Ok(TlsRestore(raw)) } @@ -442,6 +445,11 @@ mod tls { /// This is unsafe because it's intended to only be used within the /// context of stack switching within wasmtime. pub unsafe fn replace(self) -> Result<(), Box> { + // Null case: we aren't in a wasm context, so theres no tls + // to restore. + if self.0.is_null() { + return Ok(()); + } // We need to configure our previous TLS pointer to whatever is in // TLS at this time, and then we set the current state to ourselves. let prev = raw::get();