diff --git a/Cargo.lock b/Cargo.lock index cd58cd219..1d3277a2e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1359,10 +1359,12 @@ dependencies = [ name = "test-utils" version = "0.16.2" dependencies = [ + "wasmer", "wasmer-compiler", "wasmer-compiler-cranelift", "wasmer-compiler-llvm", "wasmer-compiler-singlepass", + "wasmer-engine-jit", ] [[package]] diff --git a/lib/api/src/externals.rs b/lib/api/src/externals.rs index 3cbbc53e0..a793a15dd 100644 --- a/lib/api/src/externals.rs +++ b/lib/api/src/externals.rs @@ -11,8 +11,8 @@ use wasm_common::{Bytes, HostFunction, Pages, ValueType, WasmTypeList, WithEnv, use wasmer_engine::Engine as _; use wasmer_runtime::{ wasmer_call_trampoline, Export, ExportFunction, ExportGlobal, ExportMemory, ExportTable, - LinearMemory, Table as RuntimeTable, VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, - VMGlobalDefinition, VMMemoryDefinition, VMTrampoline, + LinearMemory, MemoryError, Table as RuntimeTable, VMCallerCheckedAnyfunc, VMContext, + VMFunctionBody, VMGlobalDefinition, VMMemoryDefinition, VMTrampoline, }; #[derive(Clone)] @@ -344,25 +344,25 @@ pub struct Memory { } impl Memory { - pub fn new(store: &Store, ty: MemoryType) -> Memory { + pub fn new(store: &Store, ty: MemoryType) -> Result { let tunables = store.engine().tunables(); let memory_plan = tunables.memory_plan(ty); - let memory = tunables.create_memory(memory_plan).unwrap(); + let memory = tunables.create_memory(memory_plan)?; let definition = memory.vmmemory(); - Memory { + Ok(Memory { store: store.clone(), owned_by_store: true, exported: ExportMemory { from: Box::leak(Box::new(memory)), definition: Box::leak(Box::new(definition)), }, - } + }) } - fn definition(&self) -> &VMMemoryDefinition { - unsafe { &*self.exported.definition } + fn definition(&self) -> VMMemoryDefinition { + self.memory().vmmemory() } pub fn ty(&self) -> &MemoryType { @@ -391,14 +391,14 @@ impl Memory { } pub fn size(&self) -> Pages { - Bytes(self.data_size()).into() + self.memory().size() } fn memory(&self) -> &LinearMemory { unsafe { &*self.exported.from } } - pub fn grow(&self, delta: Pages) -> Option { + pub fn grow(&self, delta: Pages) -> Result { self.memory().grow(delta) } diff --git a/lib/api/src/lib.rs b/lib/api/src/lib.rs index 39146b8ff..594d42556 100644 --- a/lib/api/src/lib.rs +++ b/lib/api/src/lib.rs @@ -33,6 +33,7 @@ pub use wasmer_compiler::{Features, Target}; pub use wasmer_engine::{ DeserializeError, Engine, InstantiationError, LinkError, RuntimeError, SerializeError, }; +pub use wasmer_runtime::MemoryError; // The compilers are mutually exclusive #[cfg(any( diff --git a/lib/api/src/tunables.rs b/lib/api/src/tunables.rs index 624237cfb..8eb097535 100644 --- a/lib/api/src/tunables.rs +++ b/lib/api/src/tunables.rs @@ -3,6 +3,7 @@ use std::cmp::min; use target_lexicon::{OperatingSystem, PointerWidth, Triple, HOST}; use wasm_common::{MemoryType, Pages, TableType}; use wasmer_engine::Tunables as BaseTunables; +use wasmer_runtime::MemoryError; use wasmer_runtime::{LinearMemory, Table}; use wasmer_runtime::{MemoryPlan, MemoryStyle, TablePlan, TableStyle}; @@ -93,7 +94,7 @@ impl BaseTunables for Tunables { } /// Create a memory given a memory type - fn create_memory(&self, plan: MemoryPlan) -> Result { + fn create_memory(&self, plan: MemoryPlan) -> Result { LinearMemory::new(&plan) } diff --git a/lib/c-api/src/import/wasi.rs b/lib/c-api/src/import/wasi.rs index 5aca465d4..e63f949f7 100644 --- a/lib/c-api/src/import/wasi.rs +++ b/lib/c-api/src/import/wasi.rs @@ -213,7 +213,7 @@ pub unsafe extern "C" fn wasmer_wasi_generate_default_import_object() -> *mut wa let mut wasi_env = wasi::WasiEnv::new(wasi_state); // this API will now leak a `Memory` let memory_type = MemoryType::new(0, None, false); - let memory = Memory::new(store, memory_type); + let memory = Memory::new(store, memory_type).expect("create memory"); wasi_env.set_memory(&memory); // TODO(mark): review lifetime of `Memory` here let import_object = Box::new(wasi::generate_import_object_from_env( diff --git a/lib/c-api/src/memory.rs b/lib/c-api/src/memory.rs index f73e7f9ee..10a98a90d 100644 --- a/lib/c-api/src/memory.rs +++ b/lib/c-api/src/memory.rs @@ -69,9 +69,18 @@ pub unsafe extern "C" fn wasmer_memory_new( }; let store = crate::get_global_store(); let desc = MemoryType::new(Pages(limits.min), max, false); - let new_memory = Memory::new(store, desc); - *memory = Box::into_raw(Box::new(new_memory)) as *mut wasmer_memory_t; - wasmer_result_t::WASMER_OK + match Memory::new(store, desc) { + Ok(new_memory) => { + *memory = Box::into_raw(Box::new(new_memory)) as *mut wasmer_memory_t; + wasmer_result_t::WASMER_OK + } + Err(err) => { + update_last_error(CApiError { + msg: err.to_string(), + }); + wasmer_result_t::WASMER_ERROR + } + } } /// Grows a memory by the given number of pages (of 65Kb each). @@ -105,8 +114,13 @@ pub extern "C" fn wasmer_memory_grow(memory: *mut wasmer_memory_t, delta: u32) - let grow_result = memory.grow(Pages(delta)); match grow_result { - Some(_) => wasmer_result_t::WASMER_OK, - _ => wasmer_result_t::WASMER_ERROR, + Ok(_) => wasmer_result_t::WASMER_OK, + Err(err) => { + update_last_error(CApiError { + msg: err.to_string(), + }); + wasmer_result_t::WASMER_ERROR + } } } diff --git a/lib/c-api/tests/CMakeLists.txt b/lib/c-api/tests/CMakeLists.txt index 6393cbe1f..7891a8a65 100644 --- a/lib/c-api/tests/CMakeLists.txt +++ b/lib/c-api/tests/CMakeLists.txt @@ -111,8 +111,7 @@ add_test(test-instantiate test-instantiate) target_link_libraries(test-memory general ${WASMER_LIB}) target_compile_options(test-memory PRIVATE ${COMPILER_OPTIONS}) -# TODO: reenable this test -#add_test(test-memory test-memory) +add_test(test-memory test-memory) target_link_libraries(test-module general ${WASMER_LIB}) target_compile_options(test-module PRIVATE ${COMPILER_OPTIONS}) diff --git a/lib/c-api/tests/test-memory.c b/lib/c-api/tests/test-memory.c index a2aca65a8..25737561a 100644 --- a/lib/c-api/tests/test-memory.c +++ b/lib/c-api/tests/test-memory.c @@ -40,7 +40,7 @@ int main() char *error_str = malloc(error_len); wasmer_last_error_message(error_str, error_len); printf("Error str: `%s`\n", error_str); - assert(0 == strcmp(error_str, "Failed to add pages because would exceed maximum number of pages for the memory. Left: 22, Added: 15")); + assert(0 == strcmp(error_str, "The memory could not grow: current size 12 pages, requested increase: 10 pages")); free(error_str); wasmer_memory_t *bad_memory = NULL; @@ -58,7 +58,7 @@ int main() char *error_str2 = malloc(error_len2); wasmer_last_error_message(error_str2, error_len2); printf("Error str 2: `%s`\n", error_str2); - assert(0 == strcmp(error_str2, "Unable to create because the supplied descriptor is invalid: \"Max number of memory pages is less than the minimum number of pages\"")); + assert(0 == strcmp(error_str2, "The memory plan is invalid because the maximum (10 pages) is less than the minimum (15 pages)")); free(error_str2); printf("Destroy memory\n"); diff --git a/lib/engine/src/tunables.rs b/lib/engine/src/tunables.rs index 080f44977..21c8bb646 100644 --- a/lib/engine/src/tunables.rs +++ b/lib/engine/src/tunables.rs @@ -4,6 +4,7 @@ use wasm_common::{ GlobalIndex, LocalGlobalIndex, LocalMemoryIndex, LocalTableIndex, MemoryIndex, MemoryType, TableIndex, TableType, }; +use wasmer_runtime::MemoryError; use wasmer_runtime::{LinearMemory, Module, Table, VMGlobalDefinition}; use wasmer_runtime::{MemoryPlan, TablePlan}; @@ -16,7 +17,7 @@ pub trait Tunables { fn table_plan(&self, table: TableType) -> TablePlan; /// Create a memory given a memory type - fn create_memory(&self, memory_type: MemoryPlan) -> Result; + fn create_memory(&self, memory_type: MemoryPlan) -> Result; /// Create a memory given a memory type fn create_table(&self, table_type: TablePlan) -> Result; @@ -32,7 +33,10 @@ pub trait Tunables { PrimaryMap::with_capacity(module.memories.len() - num_imports); for index in num_imports..module.memories.len() { let plan = memory_plans[MemoryIndex::new(index)].clone(); - memories.push(self.create_memory(plan).map_err(LinkError::Resource)?); + memories.push( + self.create_memory(plan) + .map_err(|e| LinkError::Resource(format!("Failed to create memory: {}", e)))?, + ); } Ok(memories) } diff --git a/lib/runtime/src/instance.rs b/lib/runtime/src/instance.rs index 4aa847d7d..888203766 100644 --- a/lib/runtime/src/instance.rs +++ b/lib/runtime/src/instance.rs @@ -3,7 +3,7 @@ //! `InstanceHandle` is a reference-counting handle for an `Instance`. use crate::export::Export; use crate::imports::Imports; -use crate::memory::LinearMemory; +use crate::memory::{LinearMemory, MemoryError}; use crate::table::Table; use crate::trap::{catch_traps, init_traps, Trap, TrapCode}; use crate::vmcontext::{ @@ -431,7 +431,7 @@ impl Instance { &self, memory_index: LocalMemoryIndex, delta: IntoPages, - ) -> Option + ) -> Result where IntoPages: Into, { @@ -459,7 +459,7 @@ impl Instance { &self, memory_index: MemoryIndex, delta: IntoPages, - ) -> Option + ) -> Result where IntoPages: Into, { @@ -979,7 +979,7 @@ impl InstanceHandle { &self, memory_index: LocalMemoryIndex, delta: IntoPages, - ) -> Option + ) -> Result where IntoPages: Into, { diff --git a/lib/runtime/src/lib.rs b/lib/runtime/src/lib.rs index bad8455d5..0a2711aa2 100644 --- a/lib/runtime/src/lib.rs +++ b/lib/runtime/src/lib.rs @@ -39,7 +39,7 @@ pub mod libcalls; pub use crate::export::*; pub use crate::imports::Imports; pub use crate::instance::InstanceHandle; -pub use crate::memory::LinearMemory; +pub use crate::memory::{LinearMemory, MemoryError}; pub use crate::mmap::Mmap; pub use crate::module::{ ExportsIterator, ImportsIterator, MemoryPlan, MemoryStyle, Module, TableElements, TablePlan, diff --git a/lib/runtime/src/memory.rs b/lib/runtime/src/memory.rs index 4a96256ca..d3906a88b 100644 --- a/lib/runtime/src/memory.rs +++ b/lib/runtime/src/memory.rs @@ -7,7 +7,34 @@ use crate::module::{MemoryPlan, MemoryStyle}; use crate::vmcontext::VMMemoryDefinition; use more_asserts::{assert_ge, assert_le}; use std::cell::RefCell; -use wasm_common::Pages; +use thiserror::Error; +use wasm_common::{Bytes, Pages}; + +/// Error type describing things that can go wrong when operating on Wasm Memories. +#[derive(Error, Debug, Clone, PartialEq, Hash)] +pub enum MemoryError { + /// Low level error with mmap. + #[error("Error when allocating memory: {0}")] + Region(String), + /// The operation would cause the size of the memory to exceed the maximum or would cause + /// an overflow leading to unindexable memory. + #[error("The memory could not grow: current size {} pages, requested increase: {} pages", current.0, attempted_delta.0)] + CouldNotGrow { + /// The current size in pages. + current: Pages, + /// The attempted amount to grow by in pages. + attempted_delta: Pages, + }, + /// The operation would cause the size of the memory size exceed the maximum. + #[error("The memory plan is invalid because {}", reason)] + InvalidMemoryPlan { + /// The reason why the memory plan is invalid. + reason: String, + }, + /// A user defined error value, used for error cases not listed above. + #[error("A user-defined error occurred: {0}")] + Generic(String), +} /// A linear memory instance. #[derive(Debug)] @@ -40,13 +67,23 @@ struct WasmMmap { impl LinearMemory { /// Create a new linear memory instance with specified minimum and maximum number of wasm pages. - pub fn new(plan: &MemoryPlan) -> Result { + pub fn new(plan: &MemoryPlan) -> Result { // `maximum` cannot be set to more than `65536` pages. assert_le!(plan.memory.minimum, Pages::max_value()); assert!( plan.memory.maximum.is_none() || plan.memory.maximum.unwrap() <= Pages::max_value() ); + if plan.memory.maximum.is_some() && plan.memory.maximum.unwrap() < plan.memory.minimum { + return Err(MemoryError::InvalidMemoryPlan { + reason: format!( + "the maximum ({} pages) is less than the minimum ({} pages)", + plan.memory.maximum.unwrap().0, + plan.memory.minimum.0 + ), + }); + } + let offset_guard_bytes = plan.offset_guard_size as usize; // If we have an offset guard, or if we're doing the static memory @@ -71,7 +108,8 @@ impl LinearMemory { let mapped_bytes = mapped_pages.bytes(); let mmap = WasmMmap { - alloc: Mmap::accessible_reserved(mapped_bytes.0, request_bytes)?, + alloc: Mmap::accessible_reserved(mapped_bytes.0, request_bytes) + .map_err(MemoryError::Region)?, size: plan.memory.minimum, }; @@ -98,7 +136,7 @@ impl LinearMemory { /// /// Returns `None` if memory can't be grown by the specified amount /// of wasm pages. - pub fn grow(&self, delta: IntoPages) -> Option + pub fn grow(&self, delta: IntoPages) -> Result where IntoPages: Into, { @@ -106,20 +144,24 @@ impl LinearMemory { let delta: Pages = delta.into(); let mut mmap = self.mmap.borrow_mut(); if delta.0 == 0 { - return Some(mmap.size); + return Ok(mmap.size); } - let new_pages = match mmap.size.checked_add(delta) { - Some(new_pages) => new_pages, - // Linear memory size overflow. - None => return None, - }; + let new_pages = mmap + .size + .checked_add(delta) + .ok_or_else(|| MemoryError::CouldNotGrow { + current: mmap.size, + attempted_delta: delta, + })?; let prev_pages = mmap.size; if let Some(maximum) = self.maximum { if new_pages > maximum { - // Linear memory size would exceed the declared maximum. - return None; + return Err(MemoryError::CouldNotGrow { + current: mmap.size, + attempted_delta: delta, + }); } } @@ -128,7 +170,10 @@ impl LinearMemory { // limit here. if new_pages >= Pages::max_value() { // Linear memory size would exceed the index range. - return None; + return Err(MemoryError::CouldNotGrow { + current: mmap.size, + attempted_delta: delta, + }); } let delta_bytes = delta.bytes().0; @@ -139,9 +184,16 @@ impl LinearMemory { // If the new size is within the declared maximum, but needs more memory than we // have on hand, it's a dynamic heap and it can move. let guard_bytes = self.offset_guard_size; - let request_bytes = new_bytes.checked_add(guard_bytes)?; + let request_bytes = + new_bytes + .checked_add(guard_bytes) + .ok_or_else(|| MemoryError::CouldNotGrow { + current: new_pages, + attempted_delta: Bytes(guard_bytes).into(), + })?; - let mut new_mmap = Mmap::accessible_reserved(new_bytes, request_bytes).ok()?; + let mut new_mmap = + Mmap::accessible_reserved(new_bytes, request_bytes).map_err(MemoryError::Region)?; let copy_len = mmap.alloc.len() - self.offset_guard_size; new_mmap.as_mut_slice()[..copy_len].copy_from_slice(&mmap.alloc.as_slice()[..copy_len]); @@ -149,12 +201,14 @@ impl LinearMemory { mmap.alloc = new_mmap; } else if delta_bytes > 0 { // Make the newly allocated pages accessible. - mmap.alloc.make_accessible(prev_bytes, delta_bytes).ok()?; + mmap.alloc + .make_accessible(prev_bytes, delta_bytes) + .map_err(MemoryError::Region)?; } mmap.size = new_pages; - Some(prev_pages) + Ok(prev_pages) } /// Return a `VMMemoryDefinition` for exposing the memory to compiled wasm code. diff --git a/tests/lib/test-utils/Cargo.toml b/tests/lib/test-utils/Cargo.toml index c3787696a..a694dff62 100644 --- a/tests/lib/test-utils/Cargo.toml +++ b/tests/lib/test-utils/Cargo.toml @@ -13,6 +13,8 @@ wasmer-compiler = { path = "../../../lib/compiler", version = "0.16.2" } wasmer-compiler-singlepass = { path = "../../../lib/compiler-singlepass", version = "0.16.2", optional = true } wasmer-compiler-cranelift = { path = "../../../lib/compiler-cranelift", version = "0.16.2", optional = true } wasmer-compiler-llvm = { path = "../../../lib/compiler-llvm", version = "0.16.2", optional = true } +wasmer-engine-jit = { path = "../../../lib/engine-jit", version = "0.16.2" } +wasmer = { path = "../../../lib/api", version = "0.16.2", deafult-features = false } [features] compiler = [] diff --git a/tests/lib/test-utils/src/lib.rs b/tests/lib/test-utils/src/lib.rs index 4a9dd834c..e99171459 100644 --- a/tests/lib/test-utils/src/lib.rs +++ b/tests/lib/test-utils/src/lib.rs @@ -1,6 +1,9 @@ #![cfg(feature = "compiler")] +use std::sync::Arc; +use wasmer::{Store, Tunables}; use wasmer_compiler::{CompilerConfig, Features, Target}; +use wasmer_engine_jit::JITEngine; pub fn get_compiler_config_from_str( compiler_name: &str, @@ -34,3 +37,10 @@ pub fn get_compiler_config_from_str( _ => panic!("Compiler {} not supported", compiler_name), } } + +/// for when you need a store but you don't care about the details +pub fn get_default_store() -> Store { + let compiler_config = get_compiler_config_from_str("cranelift", false, Features::default()); + let tunables = Tunables::for_target(compiler_config.target().triple()); + Store::new(Arc::new(JITEngine::new(&*compiler_config, tunables))) +} diff --git a/tests/lib/wast/src/spectest.rs b/tests/lib/wast/src/spectest.rs index 4eadfc492..be90476d6 100644 --- a/tests/lib/wast/src/spectest.rs +++ b/tests/lib/wast/src/spectest.rs @@ -26,7 +26,7 @@ pub fn spectest_importobject(store: &Store) -> ImportObject { let table = Table::new(store, ty, Val::AnyRef(AnyRef::Null)).unwrap(); let ty = MemoryType::new(1, Some(2), false); - let memory = Memory::new(store, ty); + let memory = Memory::new(store, ty).unwrap(); imports! { "spectest" => { diff --git a/tests/misc.rs b/tests/misc.rs new file mode 100644 index 000000000..fbf1b724b --- /dev/null +++ b/tests/misc.rs @@ -0,0 +1,41 @@ +use test_utils::get_default_store; +use wasmer::{Memory, MemoryError, MemoryType, Pages}; + +#[test] +fn growing_memory_with_api() { + let desc = MemoryType::new(Pages(10), Some(Pages(16)), false); + let store = get_default_store(); + + let memory = Memory::new(&store, desc).unwrap(); + + assert_eq!(memory.size(), Pages(10)); + let result = memory.grow(Pages(2)).unwrap(); + assert_eq!(result, Pages(10)); + assert_eq!(memory.size(), Pages(12)); + + let result = memory.grow(Pages(10)); + assert_eq!( + result, + Err(MemoryError::CouldNotGrow { + current: 12.into(), + attempted_delta: 10.into(), + }) + ); + + let bad_desc = MemoryType::new(Pages(15), Some(Pages(10)), false); + let bad_result = Memory::new(&store, bad_desc); + + // due to stack overflow with a modern nightly, we can't update CI to use a version of nightly which will make this work + /*assert!(matches!( + bad_result, + Err(MemoryError::InvalidMemoryPlan { .. }) + ));*/ + + assert!( + if let Err(MemoryError::InvalidMemoryPlan { .. }) = bad_result { + true + } else { + false + } + ); +}