Merge #1682

1682: Improve error messages around memory creation r=MarkMcCaskey a=MarkMcCaskey

Fixes issue reported in #1631 (comment)


Co-authored-by: Mark McCaskey <mark@wasmer.io>

lib/c-api/src/wasm_c_api/types/memory.rs

@@ -24,6 +24,7 @@ wasm_declare_vec!(memorytype);
 
 #[allow(non_camel_case_types)]
 #[derive(Copy, Clone, Debug)]
+#[repr(C)]
 pub struct wasm_limits_t {
     pub(crate) min: u32,
     pub(crate) max: u32,

lib/c-api/tests/wasm_c_api/CMakeLists.txt

@@ -19,6 +19,7 @@ add_executable(wasm-c-api-trap wasm-c-api/example/trap.c)
 # Our additional tests.
 add_executable(test-wasi test-wasi.c)
 add_executable(test-early-exit test-early-exit.c)
+add_executable(test-memory test-memory.c)
 
 include_directories(wasm-c-api/include)
 include_directories(../../)
@@ -141,3 +142,8 @@ set_property(TARGET test-early-exit PROPERTY C_STANDARD 11)
 target_link_libraries(test-early-exit general ${WASMER_LIB})
 target_compile_options(test-early-exit PRIVATE ${COMPILER_OPTIONS})
 add_test(test-early-exit test-early-exit)
+
+set_property(TARGET test-memory PROPERTY C_STANDARD 11)
+target_link_libraries(test-memory general ${WASMER_LIB})
+target_compile_options(test-memory PRIVATE ${COMPILER_OPTIONS})
+add_test(test-memory test-memory)

lib/c-api/tests/wasm_c_api/test-memory.c

@@ -0,0 +1,55 @@
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "wasmer_wasm.h"
+
+#define own
+
+// Use the last_error API to retrieve error messages
+own char* get_wasmer_error() {
+  int error_len = wasmer_last_error_length();
+  printf("Error len: `%d`\n", error_len);
+  char *error_str = malloc(error_len);
+  wasmer_last_error_message(error_str, error_len);
+  return error_str;
+}
+
+int main(int argc, const char *argv[]) {
+  printf("Initializing...\n");
+  own wasm_engine_t* engine = wasm_engine_new();
+  own wasm_store_t* store = wasm_store_new(engine);
+
+  wasm_limits_t limits1 = {
+    .min = 0,
+    .max = wasm_limits_max_default,
+  };
+  own wasm_memorytype_t* memtype1 = wasm_memorytype_new(&limits1);
+  own wasm_memory_t* memory1 = wasm_memory_new(store, memtype1);
+  assert(memory1 == NULL);
+  char* error = get_wasmer_error();
+  printf("Found error string: %s\n", error);
+  assert(0 == strcmp("The maximum requested memory (4294967295 pages) is greater than the maximum allowed memory (65536 pages)", error));
+  free(error);
+
+  wasm_memorytype_delete(memtype1);
+
+  wasm_limits_t limits2 = {
+    .min = 15,
+    .max = 25,
+  };
+  own wasm_memorytype_t* memtype2 = wasm_memorytype_new(&limits2);
+  own wasm_memory_t* memory2 = wasm_memory_new(store, memtype2);
+  assert(memory2 != NULL);
+
+  wasm_memorytype_delete(memtype2);
+  wasm_memory_delete(memory2);
+
+  printf("Shutting down...\n");
+  wasm_store_delete(store);
+  wasm_engine_delete(engine);
+
+  printf("Done.\n");
+  return 0;
+}

lib/c-api/wasmer_wasm.h

@@ -121,7 +121,7 @@ void wasi_env_set_memory(wasi_env_t *env, const wasm_memory_t *memory);
 /**
  * Takes ownership of `wasi_env_t`.
  */
-bool wasi_get_imports(wasm_store_t *store,
+bool wasi_get_imports(const wasm_store_t *store,
                       const wasm_module_t *module,
                       const wasi_env_t *wasi_env,
                       wasm_extern_t **imports);

lib/vm/src/memory.rs

@@ -7,7 +7,7 @@
 
 use crate::mmap::Mmap;
 use crate::vmcontext::VMMemoryDefinition;
-use more_asserts::{assert_ge, assert_le};
+use more_asserts::assert_ge;
 use serde::{Deserialize, Serialize};
 use std::borrow::BorrowMut;
 use std::cell::UnsafeCell;
@@ -39,6 +39,22 @@ pub enum MemoryError {
         /// The reason why the provided memory is invalid.
         reason: String,
     },
+    /// Caller asked for more minimum memory than we can give them.
+    #[error("The minimum requested ({} pages) memory is greater than the maximum allowed memory ({} pages)", min_requested.0, max_allowed.0)]
+    MinimumMemoryTooLarge {
+        /// The number of pages requested as the minimum amount of memory.
+        min_requested: Pages,
+        /// The maximum amount of memory we can allocate.
+        max_allowed: Pages,
+    },
+    /// Caller asked for a maximum memory greater than we can give them.
+    #[error("The maximum requested memory ({} pages) is greater than the maximum allowed memory ({} pages)", max_requested.0, max_allowed.0)]
+    MaximumMemoryTooLarge {
+        /// The number of pages requested as the maximum amount of memory.
+        max_requested: Pages,
+        /// The number of pages requested as the maximum amount of memory.
+        max_allowed: Pages,
+    },
     /// A user defined error value, used for error cases not listed above.
     #[error("A user-defined error occurred: {0}")]
     Generic(String),
@@ -140,19 +156,29 @@ struct WasmMmap {
 impl LinearMemory {
     /// Create a new linear memory instance with specified minimum and maximum number of wasm pages.
     pub fn new(memory: &MemoryType, style: &MemoryStyle) -> Result<Self, MemoryError> {
-        // `maximum` cannot be set to more than `65536` pages.
-        assert_le!(memory.minimum, Pages::max_value());
-        assert!(memory.maximum.is_none() || memory.maximum.unwrap() <= Pages::max_value());
-
-        if memory.maximum.is_some() && memory.maximum.unwrap() < memory.minimum {
-            return Err(MemoryError::InvalidMemory {
-                reason: format!(
-                    "the maximum ({} pages) is less than the minimum ({} pages)",
-                    memory.maximum.unwrap().0,
-                    memory.minimum.0
-                ),
+        if memory.minimum > Pages::max_value() {
+            return Err(MemoryError::MinimumMemoryTooLarge {
+                min_requested: memory.minimum,
+                max_allowed: Pages::max_value(),
             });
         }
+        // `maximum` cannot be set to more than `65536` pages.
+        if let Some(max) = memory.maximum {
+            if max > Pages::max_value() {
+                return Err(MemoryError::MaximumMemoryTooLarge {
+                    max_requested: max,
+                    max_allowed: Pages::max_value(),
+                });
+            }
+            if max < memory.minimum {
+                return Err(MemoryError::InvalidMemory {
+                    reason: format!(
+                        "the maximum ({} pages) is less than the minimum ({} pages)",
+                        max.0, memory.minimum.0
+                    ),
+                });
+            }
+        }
 
         let offset_guard_bytes = style.offset_guard_size() as usize;