Adjust check for too-large 1-byte-page memories (#13039)

Add tests for the embedder API as well as through wasm, and then
additionally ensure that embedder-API-based-growth is limited in the
same way as core wasm.

Author: alexcrichton

crates/wasmtime/src/runtime/vm/memory.rs

@@ -415,38 +415,6 @@ impl Memory {
         delta_pages: u64,
         limiter: Option<&mut StoreResourceLimiter<'_>>,
     ) -> Result<Option<usize>, Error> {
-        let new_size = delta_pages
-            .checked_mul(self.page_size())
-            .and_then(|new_bytes| {
-                let new_bytes = usize::try_from(new_bytes).ok()?;
-                self.byte_size().checked_add(new_bytes)
-            });
-        match new_size {
-            Some(new_size) => {
-                // Disallow growth to a value which this linear memory's type
-                // cannot represent. For example 1-byte-page memories cannot be
-                // 4GiB in size.
-                if !self.ty().allow_growth_to(new_size) {
-                    if let Some(limiter) = limiter {
-                        let err = crate::format_err!("memory growth exceeds memory type's limits");
-                        limiter.memory_grow_failed(err)?;
-                    }
-                    return Ok(None);
-                }
-            }
-
-            // If the new size in memory isn't representable in a `usize` then
-            // there's no need to actually try to grow it to that size. It's
-            // impossible to succeed so just fail it early.
-            None => {
-                if let Some(limiter) = limiter {
-                    let err = crate::format_err!("memory growth exceeds address space");
-                    limiter.memory_grow_failed(err)?;
-                }
-                return Ok(None);
-            }
-        }
-
         let result = match self {
             Memory::Local(mem) => mem.grow(delta_pages, limiter).await?,
             Memory::Shared(mem) => mem.grow(delta_pages)?,
@@ -665,6 +633,17 @@ impl LocalMemory {
             .ok()
             .and_then(|n| usize::try_from(n).ok());
 
+        // Disallow growth to a value which this linear memory's type
+        // cannot represent. For example 1-byte-page memories cannot be
+        // 4GiB in size.
+        if !self.ty().allow_growth_to(new_byte_size) {
+            if let Some(limiter) = limiter {
+                let err = crate::format_err!("memory growth exceeds memory type's limits");
+                limiter.memory_grow_failed(err)?;
+            }
+            return Ok(None);
+        }
+
         // Store limiter gets first chance to reject memory_growing.
         if let Some(limiter) = &mut limiter {
             if !limiter

tests/all/memory.rs

@@ -680,6 +680,66 @@ fn shared_memory_wait_notify() -> Result<()> {
     Ok(())
 }
 
+#[test]
+#[cfg_attr(miri, ignore)]
+fn custom_1byte_page_cant_grow() -> Result<()> {
+    let mut config = Config::new();
+    config.shared_memory(true);
+
+    let engine = Engine::new(&config)?;
+    let mut store = Store::new(&engine, ());
+
+    // unshared too big
+    Memory::new(
+        &mut store,
+        MemoryTypeBuilder::new()
+            .min(0xffff_ffff)
+            .max(Some(0xffff_ffff))
+            .page_size_log2(0)
+            .build()?,
+    )
+    .unwrap_err();
+
+    // shared too big
+    SharedMemory::new(
+        &engine,
+        MemoryTypeBuilder::new()
+            .min(0xffff_ffff)
+            .max(Some(0xffff_ffff))
+            .shared(true)
+            .page_size_log2(0)
+            .build()?,
+    )
+    .unwrap_err();
+
+    if cfg!(target_pointer_width = "64") {
+        // unshared growth too big
+        let memory = Memory::new(
+            &mut store,
+            MemoryTypeBuilder::new()
+                .min(0xffff_fffe)
+                .max(Some(0xffff_ffff))
+                .page_size_log2(0)
+                .build()?,
+        )?;
+        assert!(memory.grow(&mut store, 1).is_err());
+
+        // shared growth too big
+        let memory = SharedMemory::new(
+            &engine,
+            MemoryTypeBuilder::new()
+                .min(0xffff_fffe)
+                .max(Some(0xffff_ffff))
+                .shared(true)
+                .page_size_log2(0)
+                .build()?,
+        )?;
+        assert!(memory.grow(1).is_err());
+    }
+
+    Ok(())
+}
+
 #[wasmtime_test]
 #[cfg_attr(miri, ignore)]
 #[cfg(target_pointer_width = "64")] // requires large VM reservation

tests/misc_testsuite/custom-page-sizes/max-size-invalid-shared-memory.wast

@@ -0,0 +1,27 @@
+;;! custom_page_sizes = true
+;;! hogs_memory = true
+;;! threads = true
+
+(assert_trap
+  (module
+    (memory 0xffff_ffff 0xffff_ffff shared (pagesize 1))
+  )
+  "memory minimum size of 4294967295 pages exceeds memory limits")
+
+(module $m
+  (memory (export "memory") 0xffff_fffe 0xffff_ffff shared (pagesize 1))
+)
+
+(module
+  (import "m" "memory" (memory 0 0xffff_ffff shared (pagesize 1)))
+
+  (func (export "grow") (param i32) (result i32)
+    local.get 0
+    memory.grow)
+)
+
+(assert_return (invoke "grow" (i32.const 1)) (i32.const -1))
+(assert_return (invoke "grow" (i32.const 2)) (i32.const -1))
+(assert_return (invoke "grow" (i32.const 100)) (i32.const -1))
+(assert_return (invoke "grow" (i32.const -1)) (i32.const -1))
+(assert_return (invoke "grow" (i32.const 0)) (i32.const -2))

tests/misc_testsuite/custom-page-sizes/max-size-invalid.wast

@@ -1,13 +1,6 @@
 ;;! custom_page_sizes = true
 ;;! hogs_memory = true
 
-;; FIXME(WebAssembly/custom-page-sizes#45): unclear what the semantics of this
-;; test should be. For now Wasmtime traps so this tests that traps happen, but
-;; this may change in the specification itself. Either way the problem here is
-;; that with 1-byte pages an allocation of 0xffff_ffff bytes is
-;; indistinguishable from a growth failure which returns -1. Somehow this needs
-;; reconciliation and for now it's done as a trap.
-
 (assert_trap
   (module
     (memory 0xffff_ffff (pagesize 1))