Add explicit error type for operations on Memory

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,21 +344,21 @@ pub struct Memory {
 }
 
 impl Memory {
-    pub fn new(store: &Store, ty: MemoryType) -> Memory {
+    pub fn new(store: &Store, ty: MemoryType) -> Result<Memory, MemoryError> {
         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 {
@@ -398,7 +398,7 @@ impl Memory {
         unsafe { &*self.exported.from }
     }
 
-    pub fn grow(&self, delta: Pages) -> Option<Pages> {
+    pub fn grow(&self, delta: Pages) -> Result<Pages, MemoryError> {
         self.memory().grow(delta)
     }
 

lib/api/src/lib.rs

@@ -20,7 +20,7 @@ pub use crate::memory_view::MemoryView;
 pub use crate::module::Module;
 pub use crate::ptr::{Array, Item, WasmPtr};
 pub use crate::store::{Store, StoreObject};
-pub use crate::tunables::Tunables;
+pub use crate::tunables::{MemoryError, Tunables};
 pub use crate::types::{
     AnyRef, ExportType, ExternType, FunctionType, GlobalType, HostInfo, HostRef, ImportType,
     MemoryType, Mutability, TableType, Val, ValType,

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;
+pub 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<LinearMemory, String> {
+    fn create_memory(&self, plan: MemoryPlan) -> Result<LinearMemory, MemoryError> {
         LinearMemory::new(&plan)
     }
 

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(

lib/c-api/src/memory.rs

@@ -69,10 +69,19 @@ 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);
+    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
+        }
     }
 }
 

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, "Memory could not grow 10 more pages (12 pages currently) without exceeding the maximum of 15 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 given memory plan was invalid because the maximum allowed memory (10 pages) is less than the minimum required memory (15 pages)"));
     free(error_str2);
 
     printf("Destroy memory\n");

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<LinearMemory, String>;
+    fn create_memory(&self, memory_type: MemoryPlan) -> Result<LinearMemory, MemoryError>;
 
     /// Create a memory given a memory type
     fn create_table(&self, table_type: TablePlan) -> Result<Table, String>;
@@ -32,7 +33,11 @@ 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 memories: {}", e))
+                })?,
+            );
         }
         Ok(memories)
     }

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<Pages>
+    ) -> Result<Pages, MemoryError>
     where
         IntoPages: Into<Pages>,
     {
@@ -459,7 +459,7 @@ impl Instance {
         &self,
         memory_index: MemoryIndex,
         delta: IntoPages,
-    ) -> Option<Pages>
+    ) -> Result<Pages, MemoryError>
     where
         IntoPages: Into<Pages>,
     {
@@ -979,7 +979,7 @@ impl InstanceHandle {
         &self,
         memory_index: LocalMemoryIndex,
         delta: IntoPages,
-    ) -> Option<Pages>
+    ) -> Result<Pages, MemoryError>
     where
         IntoPages: Into<Pages>,
     {

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,

lib/runtime/src/memory.rs

@@ -7,7 +7,65 @@ 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 std::fmt;
+use wasm_common::{Bytes, Pages};
+
+/// Error type describing things that can go wrong when operating on Wasm Memories.
+#[derive(Debug, Clone, PartialEq, Hash)]
+pub enum MemoryError {
+    /// Low level error with mmap.
+    MmapError(String),
+    /// The operation would cause the size of the memory size to overflow or otherwise
+    /// create memory that is not indexable.
+    SizeOverflow {
+        /// 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.
+    SizeExceeded {
+        /// The current size in pages.
+        current: Pages,
+        /// The attempted amount to grow by in pages.
+        attempted_delta: Pages,
+        /// The maximum number of pages this memory may have.
+        maximum: Pages,
+    },
+    /// The operation would cause the size of the memory size exceed the maximum.
+    InvalidMemoryPlan {
+        /// The reason why the memory plan is invalid.
+        reason: String,
+    },
+    // TODO: review this; consider alternatives
+    /// A user defined error value, used for error cases not listed above.
+    Other(String),
+}
+
+impl std::error::Error for MemoryError {}
+
+impl fmt::Display for MemoryError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            MemoryError::MmapError(err) => {
+                write!(f, "There was an error when `mmap`ing memory: {}", err)
+            }
+            MemoryError::SizeOverflow {
+                current,
+                attempted_delta,
+            } => write!(f, "Memory could not grow {} more pages ({} pages currently) without overflowing the indexable space", attempted_delta.0, current.0),
+            MemoryError::SizeExceeded {
+                current,
+                attempted_delta,
+                maximum,
+            } => write!(f, "Memory could not grow {} more pages ({} pages currently) without exceeding the maximum of {} pages", attempted_delta.0, current.0, maximum.0),
+            MemoryError::InvalidMemoryPlan {
+                reason
+            } => write!(f, "The given memory plan was invalid because {}", reason),
+            MemoryError::Other(other) => write!(f, "A user-defined error occurred: {}", other),
+        }
+    }
+}
 
 /// A linear memory instance.
 #[derive(Debug)]
@@ -40,13 +98,19 @@ 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<Self, String> {
+    pub fn new(plan: &MemoryPlan) -> Result<Self, MemoryError> {
         // `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 allowed memory ({} pages) is less than the minimum required memory ({} 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 +135,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::MmapError)?,
             size: plan.memory.minimum,
         };
 
@@ -98,7 +163,7 @@ impl LinearMemory {
     ///
     /// Returns `None` if memory can't be grown by the specified amount
     /// of wasm pages.
-    pub fn grow<IntoPages>(&self, delta: IntoPages) -> Option<Pages>
+    pub fn grow<IntoPages>(&self, delta: IntoPages) -> Result<Pages, MemoryError>
     where
         IntoPages: Into<Pages>,
     {
@@ -106,20 +171,25 @@ 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::SizeOverflow {
+                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::SizeExceeded {
+                    current: mmap.size,
+                    attempted_delta: delta,
+                    maximum,
+                });
             }
         }
 
@@ -128,7 +198,10 @@ impl LinearMemory {
         // limit here.
         if new_pages >= Pages::max_value() {
             // Linear memory size would exceed the index range.
-            return None;
+            return Err(MemoryError::SizeOverflow {
+                current: mmap.size,
+                attempted_delta: delta,
+            });
         }
 
         let delta_bytes = delta.bytes().0;
@@ -139,9 +212,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::SizeOverflow {
+                        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::MmapError)?;
 
             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 +229,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::MmapError)?;
         }
 
         mmap.size = new_pages;
 
-        Some(prev_pages)
+        Ok(prev_pages)
     }
 
     /// Return a `VMMemoryDefinition` for exposing the memory to compiled wasm code.

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" => {

tests/misc.rs

@@ -1,15 +1,33 @@
 use test_utils::get_default_store;
-use wasmer::{Memory, MemoryType, Pages};
+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);
+    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::SizeExceeded {
+            current: 12.into(),
+            attempted_delta: 10.into(),
+            maximum: 16.into(),
+        })
+    );
+
+    let bad_desc = MemoryType::new(Pages(15), Some(Pages(10)), false);
+    let bad_result = Memory::new(&store, bad_desc);
+
+    assert!(matches!(
+        bad_result,
+        Err(MemoryError::InvalidMemoryPlan { .. })
+    ));
 }