From 48de0207f92a0222fb56a78de8ed2d452fa621f1 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 5 Nov 2024 16:32:06 -0800 Subject: [PATCH 01/11] Refactor internals of `vm::Memory` Instead of storing just a trait object store instead an `enum` between "local" memory and shared memory. This helps remove redundant trait impls on shared memories, removes the need for `dyn Any`, and removes the possibility of wrapping a shared memory as a shared memory. This is additionally intended to make it a bit easier to see what's nested where and reduce some layers of indirection. --- crates/wasmtime/src/runtime/memory.rs | 2 +- .../wasmtime/src/runtime/trampoline/memory.rs | 4 - crates/wasmtime/src/runtime/vm/cow.rs | 16 --- crates/wasmtime/src/runtime/vm/memory.rs | 104 +++++++++++------- .../src/runtime/vm/threads/shared_memory.rs | 78 ++++--------- .../vm/threads/shared_memory_disabled.rs | 30 +---- 6 files changed, 89 insertions(+), 145 deletions(-) diff --git a/crates/wasmtime/src/runtime/memory.rs b/crates/wasmtime/src/runtime/memory.rs index 3a93de349222..efc705e6419d 100644 --- a/crates/wasmtime/src/runtime/memory.rs +++ b/crates/wasmtime/src/runtime/memory.rs @@ -1,5 +1,5 @@ use crate::prelude::*; -use crate::runtime::vm::{RuntimeLinearMemory, VMMemoryImport}; +use crate::runtime::vm::VMMemoryImport; use crate::store::{StoreData, StoreOpaque, Stored}; use crate::trampoline::generate_memory_export; use crate::Trap; diff --git a/crates/wasmtime/src/runtime/trampoline/memory.rs b/crates/wasmtime/src/runtime/trampoline/memory.rs index 2287102edac3..42aa2171a2b6 100644 --- a/crates/wasmtime/src/runtime/trampoline/memory.rs +++ b/crates/wasmtime/src/runtime/trampoline/memory.rs @@ -107,10 +107,6 @@ impl RuntimeLinearMemory for LinearMemoryProxy { true } - fn as_any_mut(&mut self) -> &mut dyn core::any::Any { - self - } - fn wasm_accessible(&self) -> Range { self.mem.wasm_accessible() } diff --git a/crates/wasmtime/src/runtime/vm/cow.rs b/crates/wasmtime/src/runtime/vm/cow.rs index af53d15d0075..01f2e07ee3c8 100644 --- a/crates/wasmtime/src/runtime/vm/cow.rs +++ b/crates/wasmtime/src/runtime/vm/cow.rs @@ -348,22 +348,6 @@ impl MemoryImageSlot { } } - #[cfg(feature = "pooling-allocator")] - pub(crate) fn dummy() -> MemoryImageSlot { - MemoryImageSlot { - // This pointer isn't ever actually used so its value doesn't - // matter but we need to satisfy `NonNull` requirement so create a - // `dangling` pointer as a sentinel that should cause problems if - // it's actually used. - base: NonNull::dangling().into(), - static_size: 0, - image: None, - accessible: 0, - dirty: false, - clear_on_drop: false, - } - } - /// Inform the MemoryImageSlot that it should *not* clear the underlying /// address space when dropped. This should be used only when the /// caller will clear or reuse the address space in some other diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index 98de7ca9854b..6660e0477475 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -164,14 +164,15 @@ pub trait RuntimeLinearMemory: Send + Sync { /// `RuntimeMemoryCreator::new_memory()`. fn needs_init(&self) -> bool; - /// Used for optional dynamic downcasting. - fn as_any_mut(&mut self) -> &mut dyn core::any::Any; - /// Returns the range of addresses that may be reached by WebAssembly. /// /// This starts at the base of linear memory and ends at the end of the /// guard pages, if any. fn wasm_accessible(&self) -> Range; + + fn unwrap_static_image(self: Box) -> MemoryImageSlot { + panic!("not a static memory") + } } /// A linear memory instance. @@ -441,10 +442,6 @@ impl RuntimeLinearMemory for MmapMemory { self.memory_image.is_none() } - fn as_any_mut(&mut self) -> &mut dyn core::any::Any { - self - } - fn wasm_accessible(&self) -> Range { let base = self.mmap.as_ptr() as usize + self.pre_guard_size; let end = base + (self.mmap.len() - self.pre_guard_size); @@ -553,19 +550,22 @@ impl RuntimeLinearMemory for StaticMemory { !self.memory_image.has_image() } - fn as_any_mut(&mut self) -> &mut dyn core::any::Any { - self - } - fn wasm_accessible(&self) -> Range { let base = self.base.as_ptr() as usize; let end = base + self.memory_and_guard_size; base..end } + + fn unwrap_static_image(self: Box) -> MemoryImageSlot { + self.memory_image + } } /// Representation of a runtime wasm linear memory. -pub struct Memory(pub(crate) Box); +pub enum Memory { + Local(Box), + Shared(SharedMemory), +} impl Memory { /// Create a new dynamic (movable) memory instance for the specified plan. @@ -578,11 +578,6 @@ impl Memory { ) -> Result { let (minimum, maximum) = Self::limit_new(ty, Some(store))?; let allocation = creator.new_memory(ty, tunables, minimum, maximum, memory_image)?; - let allocation = if ty.shared { - Box::new(SharedMemory::wrap(ty, allocation)?) - } else { - allocation - }; // Double-check that the created memory respects the safety invariant of // whether the memory may move or not at runtime. @@ -600,7 +595,11 @@ impl Memory { "this linear memory should not be able to move its base pointer", ); - Ok(Memory(allocation)) + Ok(if ty.shared { + Memory::Shared(SharedMemory::wrap(ty, allocation)?) + } else { + Memory::Local(allocation) + }) } /// Create a new static (immovable) memory instance for the specified plan. @@ -623,15 +622,14 @@ impl Memory { memory_and_guard_size, )?; let allocation = Box::new(pooled_memory); - let allocation: Box = if ty.shared { + Ok(if ty.shared { // FIXME(#4244): not supported with the pooling allocator (which // `new_static` is always used with), see `MemoryPool::validate` as // well). todo!("using shared memory with the pooling allocator is a work in progress"); } else { - allocation - }; - Ok(Memory(allocation)) + Memory::Local(allocation) + }) } /// Calls the `store`'s limiter to optionally prevent a memory from being allocated. @@ -714,19 +712,28 @@ impl Memory { /// Returns this memory's page size, in bytes. pub fn page_size(&self) -> u64 { - self.0.page_size() + match self { + Memory::Local(mem) => mem.page_size(), + Memory::Shared(mem) => mem.page_size(), + } } /// Returns the number of allocated wasm pages. pub fn byte_size(&self) -> usize { - self.0.byte_size() + match self { + Memory::Local(mem) => mem.byte_size(), + Memory::Shared(mem) => mem.byte_size(), + } } /// Returns whether or not this memory needs initialization. It /// may not if it already has initial content thanks to a CoW /// mechanism. pub(crate) fn needs_init(&self) -> bool { - self.0.needs_init() + match self { + Memory::Local(mem) => mem.needs_init(), + Memory::Shared(mem) => mem.needs_init(), + } } /// Grow memory by the specified amount of wasm pages. @@ -751,39 +758,51 @@ impl Memory { delta_pages: u64, store: Option<&mut dyn VMStore>, ) -> Result, Error> { - self.0 - .grow(delta_pages, store) - .map(|opt| opt.map(|(old, _new)| old)) + let result = match self { + Memory::Local(mem) => mem.grow(delta_pages, store)?, + Memory::Shared(mem) => mem.grow(delta_pages, store)?, + }; + match result { + Some((old, _new)) => Ok(Some(old)), + None => Ok(None), + } } /// Return a `VMMemoryDefinition` for exposing the memory to compiled wasm code. pub fn vmmemory(&mut self) -> VMMemoryDefinition { - self.0.vmmemory() + match self { + Memory::Local(mem) => mem.vmmemory(), + // `vmmemory()` is used for writing the `VMMemoryDefinition` of a + // memory into its `VMContext`; this should never be possible for a + // shared memory because the only `VMMemoryDefinition` for it should + // be stored in its own `def` field. + Memory::Shared(_) => unreachable!(), + } } /// Consume the memory, returning its [`MemoryImageSlot`] if any is present. /// The image should only be present for a subset of memories created with /// [`Memory::new_static()`]. #[cfg(feature = "pooling-allocator")] - pub fn unwrap_static_image(mut self) -> MemoryImageSlot { - let mem = self.0.as_any_mut().downcast_mut::().unwrap(); - core::mem::replace(&mut mem.memory_image, MemoryImageSlot::dummy()) + pub fn unwrap_static_image(self) -> MemoryImageSlot { + match self { + Memory::Local(mem) => mem.unwrap_static_image(), + Memory::Shared(_) => panic!("expected a local memory"), + } } /// If the [Memory] is a [SharedMemory], unwrap it and return a clone to /// that shared memory. pub fn as_shared_memory(&mut self) -> Option<&mut SharedMemory> { - let as_any = self.0.as_any_mut(); - if let Some(m) = as_any.downcast_mut::() { - Some(m) - } else { - None + match self { + Memory::Local(_) => None, + Memory::Shared(mem) => Some(mem), } } /// Implementation of `memory.atomic.notify` for all memories. pub fn atomic_notify(&mut self, addr: u64, count: u32) -> Result { - match self.0.as_any_mut().downcast_mut::() { + match self.as_shared_memory() { Some(m) => m.atomic_notify(addr, count), None => { validate_atomic_addr(&self.vmmemory(), addr, 4, 4)?; @@ -799,7 +818,7 @@ impl Memory { expected: u32, timeout: Option, ) -> Result { - match self.0.as_any_mut().downcast_mut::() { + match self.as_shared_memory() { Some(m) => m.atomic_wait32(addr, expected, timeout), None => { validate_atomic_addr(&self.vmmemory(), addr, 4, 4)?; @@ -815,7 +834,7 @@ impl Memory { expected: u64, timeout: Option, ) -> Result { - match self.0.as_any_mut().downcast_mut::() { + match self.as_shared_memory() { Some(m) => m.atomic_wait64(addr, expected, timeout), None => { validate_atomic_addr(&self.vmmemory(), addr, 8, 8)?; @@ -828,7 +847,10 @@ impl Memory { /// this linear memory. Note that this includes guard pages which wasm can /// hit. pub fn wasm_accessible(&self) -> Range { - self.0.wasm_accessible() + match self { + Memory::Local(mem) => mem.wasm_accessible(), + Memory::Shared(mem) => mem.wasm_accessible(), + } } } diff --git a/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs b/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs index 7afe5f1e4a87..72c25d554634 100644 --- a/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs +++ b/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs @@ -1,8 +1,8 @@ use crate::prelude::*; -use crate::runtime::vm::memory::{validate_atomic_addr, MmapMemory}; +use crate::runtime::vm::memory::{validate_atomic_addr, MmapMemory, RuntimeLinearMemory}; use crate::runtime::vm::threads::parking_spot::{ParkingSpot, Waiter}; use crate::runtime::vm::vmcontext::VMMemoryDefinition; -use crate::runtime::vm::{Memory, RuntimeLinearMemory, VMStore, WaitResult}; +use crate::runtime::vm::{Memory, VMStore, WaitResult}; use std::cell::RefCell; use std::ops::Range; use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; @@ -47,10 +47,6 @@ impl SharedMemory { if memory.memory_may_move() { bail!("shared memory cannot use a memory which may relocate the base pointer") } - assert!( - memory.as_any_mut().type_id() != std::any::TypeId::of::(), - "cannot re-wrap a shared memory" - ); Ok(Self(Arc::new(SharedMemoryInner { ty: *ty, spot: ParkingSpot::default(), @@ -66,7 +62,7 @@ impl SharedMemory { /// Convert this shared memory into a [`Memory`]. pub fn as_memory(self) -> Memory { - Memory(Box::new(self)) + Memory::Shared(self) } /// Return a pointer to the shared memory's [VMMemoryDefinition]. @@ -166,6 +162,22 @@ impl SharedMemory { Ok(self.0.spot.wait64(atomic, expected, deadline, &mut waiter)) }) } + + pub(crate) fn page_size(&self) -> u64 { + self.0.ty.page_size() + } + + pub(crate) fn byte_size(&self) -> usize { + self.0.memory.read().unwrap().byte_size() + } + + pub(crate) fn needs_init(&self) -> bool { + self.0.memory.read().unwrap().needs_init() + } + + pub(crate) fn wasm_accessible(&self) -> Range { + self.0.memory.read().unwrap().wasm_accessible() + } } thread_local! { @@ -185,55 +197,3 @@ thread_local! { struct LongTermVMMemoryDefinition(VMMemoryDefinition); unsafe impl Send for LongTermVMMemoryDefinition {} unsafe impl Sync for LongTermVMMemoryDefinition {} - -/// Proxy all calls through the [`RwLock`]. -impl RuntimeLinearMemory for SharedMemory { - fn page_size_log2(&self) -> u8 { - self.0.memory.read().unwrap().page_size_log2() - } - - fn byte_size(&self) -> usize { - self.0.memory.read().unwrap().byte_size() - } - - fn maximum_byte_size(&self) -> Option { - self.0.memory.read().unwrap().maximum_byte_size() - } - - fn grow( - &mut self, - delta_pages: u64, - store: Option<&mut dyn VMStore>, - ) -> Result, Error> { - SharedMemory::grow(self, delta_pages, store) - } - - fn grow_to(&mut self, size: usize) -> Result<()> { - self.0.memory.write().unwrap().grow_to(size) - } - - fn vmmemory(&mut self) -> VMMemoryDefinition { - // `vmmemory()` is used for writing the `VMMemoryDefinition` of a memory - // into its `VMContext`; this should never be possible for a shared - // memory because the only `VMMemoryDefinition` for it should be stored - // in its own `def` field. - unreachable!() - } - - fn needs_init(&self) -> bool { - self.0.memory.read().unwrap().needs_init() - } - - fn as_any_mut(&mut self) -> &mut dyn std::any::Any { - self - } - - fn wasm_accessible(&self) -> Range { - self.0.memory.read().unwrap().wasm_accessible() - } - - fn memory_may_move(&self) -> bool { - debug_assert!(!self.0.memory.read().unwrap().memory_may_move()); - false - } -} diff --git a/crates/wasmtime/src/runtime/vm/threads/shared_memory_disabled.rs b/crates/wasmtime/src/runtime/vm/threads/shared_memory_disabled.rs index 74e7c8725c8e..4fc78a1ee585 100644 --- a/crates/wasmtime/src/runtime/vm/threads/shared_memory_disabled.rs +++ b/crates/wasmtime/src/runtime/vm/threads/shared_memory_disabled.rs @@ -58,46 +58,28 @@ impl SharedMemory { ) -> Result { match *self {} } -} - -impl RuntimeLinearMemory for SharedMemory { - fn page_size_log2(&self) -> u8 { - match *self {} - } - - fn byte_size(&self) -> usize { - match *self {} - } - fn maximum_byte_size(&self) -> Option { - match *self {} - } - - fn grow( - &mut self, - _delta_pages: u64, - _store: Option<&mut dyn VMStore>, - ) -> Result> { + pub(crate) fn page_size(&self) -> u64 { match *self {} } - fn grow_to(&mut self, _size: usize) -> Result<()> { + pub(crate) fn byte_size(&self) -> usize { match *self {} } - fn vmmemory(&mut self) -> VMMemoryDefinition { + pub(crate) fn grow_to(&mut self, _size: usize) -> Result<()> { match *self {} } - fn needs_init(&self) -> bool { + pub(crate) fn vmmemory(&mut self) -> VMMemoryDefinition { match *self {} } - fn as_any_mut(&mut self) -> &mut dyn core::any::Any { + pub(crate) fn needs_init(&self) -> bool { match *self {} } - fn wasm_accessible(&self) -> Range { + pub(crate) fn wasm_accessible(&self) -> Range { match *self {} } From b960ae31052f5c6286d415cc7956ead0a3a2f8af Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 5 Nov 2024 16:55:20 -0800 Subject: [PATCH 02/11] Start implementing a `LocalMemory` abstraction This is intended to be a non-shared implementation of a linear memory with various bits and pieces tracking state. The end goal is to simplify the `RuntimeLinearMemory` trait to its bare bones necessary to faithfully implement wasm linear memory. --- .../wasmtime/src/runtime/trampoline/memory.rs | 12 +- crates/wasmtime/src/runtime/vm/memory.rs | 227 ++++++++++-------- .../src/runtime/vm/threads/shared_memory.rs | 11 +- 3 files changed, 126 insertions(+), 124 deletions(-) diff --git a/crates/wasmtime/src/runtime/trampoline/memory.rs b/crates/wasmtime/src/runtime/trampoline/memory.rs index 42aa2171a2b6..5b654a5ccd5f 100644 --- a/crates/wasmtime/src/runtime/trampoline/memory.rs +++ b/crates/wasmtime/src/runtime/trampoline/memory.rs @@ -76,14 +76,9 @@ pub fn create_memory( struct LinearMemoryProxy { mem: Box, - page_size_log2: u8, } impl RuntimeLinearMemory for LinearMemoryProxy { - fn page_size_log2(&self) -> u8 { - self.page_size_log2 - } - fn byte_size(&self) -> usize { self.mem.byte_size() } @@ -146,12 +141,7 @@ impl RuntimeMemoryCreator for MemoryCreatorProxy { reserved_size_in_bytes, usize::try_from(tunables.memory_guard_size).unwrap(), ) - .map(|mem| { - Box::new(LinearMemoryProxy { - mem, - page_size_log2: ty.page_size_log2, - }) as Box - }) + .map(|mem| Box::new(LinearMemoryProxy { mem }) as Box) .map_err(|e| anyhow!(e)) } } diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index 6660e0477475..deedc72714c8 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -54,16 +54,6 @@ impl RuntimeMemoryCreator for DefaultMemoryCreator { /// A linear memory and its backing storage. pub trait RuntimeLinearMemory: Send + Sync { - /// Returns the log2 of this memory's page size, in bytes. - fn page_size_log2(&self) -> u8; - - /// Returns this memory's page size, in bytes. - fn page_size(&self) -> u64 { - let log2 = self.page_size_log2(); - debug_assert!(log2 == 16 || log2 == 0); - 1 << self.page_size_log2() - } - /// Returns the number of allocated bytes. fn byte_size(&self) -> usize; @@ -75,80 +65,6 @@ pub trait RuntimeLinearMemory: Send + Sync { /// time to accommodate requests for growth. fn memory_may_move(&self) -> bool; - /// Grows a memory by `delta_pages`. - /// - /// This performs the necessary checks on the growth before delegating to - /// the underlying `grow_to` implementation. A default implementation of - /// this memory is provided here since this is assumed to be the same for - /// most kinds of memory; one exception is shared memory, which must perform - /// all the steps of the default implementation *plus* the required locking. - /// - /// The `store` is used only for error reporting. - fn grow( - &mut self, - delta_pages: u64, - mut store: Option<&mut dyn VMStore>, - ) -> 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 Ok(Some((old_byte_size, old_byte_size))); - } - - let page_size = usize::try_from(self.page_size()).unwrap(); - - // The largest wasm-page-aligned region of memory is possible to - // represent in a `usize`. This will be impossible for the system to - // actually allocate. - let absolute_max = 0usize.wrapping_sub(page_size); - - // Calculate the byte size of the new allocation. Let it overflow up to - // `usize::MAX`, then clamp it down to `absolute_max`. - let new_byte_size = usize::try_from(delta_pages) - .unwrap_or(usize::MAX) - .saturating_mul(page_size) - .saturating_add(old_byte_size) - .min(absolute_max); - - let maximum = self.maximum_byte_size(); - - // Store limiter gets first chance to reject memory_growing. - if let Some(store) = &mut store { - 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 { - if let Some(store) = store { - // FIXME: shared memories may not have an associated store - // to report the growth failure to but the error should not - // be dropped - // (https://github.com/bytecodealliance/wasmtime/issues/4240). - store.memory_grow_failed(format_err!("Memory maximum size exceeded"))?; - } - return Ok(None); - } - } - - match self.grow_to(new_byte_size) { - Ok(_) => Ok(Some((old_byte_size, new_byte_size))), - Err(e) => { - // FIXME: shared memories may not have an associated store to - // report the growth failure to but the error should not be - // dropped - // (https://github.com/bytecodealliance/wasmtime/issues/4240). - if let Some(store) = store { - store.memory_grow_failed(e)?; - } - Ok(None) - } - } - } - /// Grow memory to the specified amount of bytes. /// /// Returns an error if memory can't be grown by the specified amount @@ -201,9 +117,6 @@ pub struct MmapMemory { // page size. maximum: Option, - // The log2 of this Wasm memory's page size, in bytes. - page_size_log2: u8, - // The amount of extra bytes to reserve whenever memory grows. This is // specified so that the cost of repeated growth is amortized. extra_to_reserve_on_growth: usize, @@ -311,7 +224,6 @@ impl MmapMemory { mmap, len: minimum, maximum, - page_size_log2: ty.page_size_log2, pre_guard_size: pre_guard_bytes, offset_guard_size: offset_guard_bytes, extra_to_reserve_on_growth, @@ -332,10 +244,6 @@ impl MmapMemory { } impl RuntimeLinearMemory for MmapMemory { - fn page_size_log2(&self) -> u8 { - self.page_size_log2 - } - fn byte_size(&self) -> usize { self.len } @@ -462,9 +370,6 @@ struct StaticMemory { /// The current size, in bytes, of this memory. size: usize, - /// The log2 of this memory's page size. - page_size_log2: u8, - /// The size, in bytes, of the virtual address allocation starting at `base` /// and going to the end of the guard pages at the end of the linear memory. memory_and_guard_size: usize, @@ -480,7 +385,6 @@ impl StaticMemory { base_capacity: usize, initial_size: usize, maximum_size: Option, - page_size_log2: u8, memory_image: MemoryImageSlot, memory_and_guard_size: usize, ) -> Result { @@ -503,7 +407,6 @@ impl StaticMemory { base: SendSyncPtr::new(NonNull::new(base_ptr).unwrap()), capacity: base_capacity, size: initial_size, - page_size_log2, memory_image, memory_and_guard_size, }) @@ -511,10 +414,6 @@ impl StaticMemory { } impl RuntimeLinearMemory for StaticMemory { - fn page_size_log2(&self) -> u8 { - self.page_size_log2 - } - fn byte_size(&self) -> usize { self.size } @@ -563,7 +462,7 @@ impl RuntimeLinearMemory for StaticMemory { /// Representation of a runtime wasm linear memory. pub enum Memory { - Local(Box), + Local(LocalMemory), Shared(SharedMemory), } @@ -595,10 +494,11 @@ impl Memory { "this linear memory should not be able to move its base pointer", ); + let memory = LocalMemory::new(ty, allocation); Ok(if ty.shared { - Memory::Shared(SharedMemory::wrap(ty, allocation)?) + Memory::Shared(SharedMemory::wrap(ty, memory)?) } else { - Memory::Local(allocation) + Memory::Local(memory) }) } @@ -617,18 +517,18 @@ impl Memory { base_capacity, minimum, maximum, - ty.page_size_log2, memory_image, memory_and_guard_size, )?; let allocation = Box::new(pooled_memory); + let memory = LocalMemory::new(ty, allocation); Ok(if ty.shared { // FIXME(#4244): not supported with the pooling allocator (which // `new_static` is always used with), see `MemoryPool::validate` as // well). todo!("using shared memory with the pooling allocator is a work in progress"); } else { - Memory::Local(allocation) + Memory::Local(memory) }) } @@ -854,6 +754,121 @@ impl Memory { } } +/// An owned allocation of a wasm linear memory. +/// +/// This might be part of a `Memory` via `Memory::Local` but it might also be +/// the implementation basis for a `SharedMemory` behind an `RwLock` for +/// example. +pub struct LocalMemory { + alloc: Box, + ty: wasmtime_environ::Memory, +} + +impl LocalMemory { + pub fn new(ty: &wasmtime_environ::Memory, alloc: Box) -> LocalMemory { + LocalMemory { ty: *ty, alloc } + } + + pub fn page_size(&self) -> u64 { + self.ty.page_size() + } + + /// Grows a memory by `delta_pages`. + /// + /// This performs the necessary checks on the growth before delegating to + /// the underlying `grow_to` implementation. + /// + /// The `store` is used only for error reporting. + pub fn grow( + &mut self, + delta_pages: u64, + mut store: Option<&mut dyn VMStore>, + ) -> Result, Error> { + let old_byte_size = self.alloc.byte_size(); + + // Wasm spec: when growing by 0 pages, always return the current size. + if delta_pages == 0 { + return Ok(Some((old_byte_size, old_byte_size))); + } + + let page_size = usize::try_from(self.page_size()).unwrap(); + + // The largest wasm-page-aligned region of memory is possible to + // represent in a `usize`. This will be impossible for the system to + // actually allocate. + let absolute_max = 0usize.wrapping_sub(page_size); + + // Calculate the byte size of the new allocation. Let it overflow up to + // `usize::MAX`, then clamp it down to `absolute_max`. + let new_byte_size = usize::try_from(delta_pages) + .unwrap_or(usize::MAX) + .saturating_mul(page_size) + .saturating_add(old_byte_size) + .min(absolute_max); + + let maximum = self.alloc.maximum_byte_size(); + + // Store limiter gets first chance to reject memory_growing. + if let Some(store) = &mut store { + 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 { + if let Some(store) = store { + // FIXME: shared memories may not have an associated store + // to report the growth failure to but the error should not + // be dropped + // (https://github.com/bytecodealliance/wasmtime/issues/4240). + store.memory_grow_failed(format_err!("Memory maximum size exceeded"))?; + } + return Ok(None); + } + } + + match self.alloc.grow_to(new_byte_size) { + Ok(_) => Ok(Some((old_byte_size, new_byte_size))), + Err(e) => { + // FIXME: shared memories may not have an associated store to + // report the growth failure to but the error should not be + // dropped + // (https://github.com/bytecodealliance/wasmtime/issues/4240). + if let Some(store) = store { + store.memory_grow_failed(e)?; + } + Ok(None) + } + } + } + + pub fn vmmemory(&mut self) -> VMMemoryDefinition { + self.alloc.vmmemory() + } + + pub fn byte_size(&self) -> usize { + self.alloc.byte_size() + } + + pub fn needs_init(&self) -> bool { + self.alloc.needs_init() + } + + pub fn memory_may_move(&self) -> bool { + self.alloc.memory_may_move() + } + + pub fn wasm_accessible(&self) -> Range { + self.alloc.wasm_accessible() + } + + pub fn unwrap_static_image(self) -> MemoryImageSlot { + self.alloc.unwrap_static_image() + } +} + /// In the configurations where bounds checks were elided in JIT code (because /// we are using static memories with virtual memory guard pages) this manual /// check is here so we don't segfault from Rust. For other configurations, diff --git a/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs b/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs index 72c25d554634..db5e7be3605f 100644 --- a/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs +++ b/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs @@ -1,5 +1,5 @@ use crate::prelude::*; -use crate::runtime::vm::memory::{validate_atomic_addr, MmapMemory, RuntimeLinearMemory}; +use crate::runtime::vm::memory::{validate_atomic_addr, LocalMemory, MmapMemory}; use crate::runtime::vm::threads::parking_spot::{ParkingSpot, Waiter}; use crate::runtime::vm::vmcontext::VMMemoryDefinition; use crate::runtime::vm::{Memory, VMStore, WaitResult}; @@ -22,7 +22,7 @@ use wasmtime_environ::{Trap, Tunables}; pub struct SharedMemory(Arc); struct SharedMemoryInner { - memory: RwLock>, + memory: RwLock, spot: ParkingSpot, ty: wasmtime_environ::Memory, def: LongTermVMMemoryDefinition, @@ -33,14 +33,11 @@ impl SharedMemory { pub fn new(ty: &wasmtime_environ::Memory, tunables: &Tunables) -> Result { let (minimum_bytes, maximum_bytes) = Memory::limit_new(ty, None)?; let mmap_memory = MmapMemory::new(ty, tunables, minimum_bytes, maximum_bytes, None)?; - Self::wrap(ty, Box::new(mmap_memory)) + Self::wrap(ty, LocalMemory::new(ty, Box::new(mmap_memory))) } /// Wrap an existing [Memory] with the locking provided by a [SharedMemory]. - pub fn wrap( - ty: &wasmtime_environ::Memory, - mut memory: Box, - ) -> Result { + pub fn wrap(ty: &wasmtime_environ::Memory, mut memory: LocalMemory) -> Result { if !ty.shared { bail!("shared memory must have a `shared` memory type"); } From 533712ce82e8cb539a3e5b48306c9108b852dea5 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 5 Nov 2024 17:16:32 -0800 Subject: [PATCH 03/11] Move `vmmemory` to `LocalMemory` --- .../wasmtime/src/runtime/trampoline/memory.rs | 9 +++---- crates/wasmtime/src/runtime/vm/memory.rs | 24 ++++++++----------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/crates/wasmtime/src/runtime/trampoline/memory.rs b/crates/wasmtime/src/runtime/trampoline/memory.rs index 5b654a5ccd5f..8e337cfec6ed 100644 --- a/crates/wasmtime/src/runtime/trampoline/memory.rs +++ b/crates/wasmtime/src/runtime/trampoline/memory.rs @@ -5,7 +5,7 @@ use crate::runtime::vm::{ CompiledModuleId, GcHeapAllocationIndex, Imports, InstanceAllocationRequest, InstanceAllocator, InstanceAllocatorImpl, Memory, MemoryAllocationIndex, MemoryImage, ModuleRuntimeInfo, OnDemandInstanceAllocator, RuntimeLinearMemory, RuntimeMemoryCreator, SharedMemory, StorePtr, - Table, TableAllocationIndex, VMMemoryDefinition, + Table, TableAllocationIndex, }; use crate::store::{InstanceId, StoreOpaque}; use crate::MemoryType; @@ -91,11 +91,8 @@ impl RuntimeLinearMemory for LinearMemoryProxy { self.mem.grow_to(new_size) } - fn vmmemory(&mut self) -> VMMemoryDefinition { - VMMemoryDefinition { - base: self.mem.as_ptr(), - current_length: self.mem.byte_size().into(), - } + fn base_ptr(&mut self) -> *mut u8 { + self.mem.as_ptr() } fn needs_init(&self) -> bool { diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index deedc72714c8..011fc149f9b8 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -71,9 +71,8 @@ pub trait RuntimeLinearMemory: Send + Sync { /// of bytes. fn grow_to(&mut self, size: usize) -> Result<()>; - /// Return a `VMMemoryDefinition` for exposing the memory to compiled wasm - /// code. - fn vmmemory(&mut self) -> VMMemoryDefinition; + /// Returns a pointer to the base of this linear memory allocation. + fn base_ptr(&mut self) -> *mut u8; /// Does this memory need initialization? It may not if it already /// has initial contents courtesy of the `MemoryImage` passed to @@ -337,11 +336,8 @@ impl RuntimeLinearMemory for MmapMemory { Ok(()) } - fn vmmemory(&mut self) -> VMMemoryDefinition { - VMMemoryDefinition { - base: unsafe { self.mmap.as_mut_ptr().add(self.pre_guard_size) }, - current_length: self.len.into(), - } + fn base_ptr(&mut self) -> *mut u8 { + unsafe { self.mmap.as_mut_ptr().add(self.pre_guard_size) } } fn needs_init(&self) -> bool { @@ -438,11 +434,8 @@ impl RuntimeLinearMemory for StaticMemory { Ok(()) } - fn vmmemory(&mut self) -> VMMemoryDefinition { - VMMemoryDefinition { - base: self.base.as_ptr(), - current_length: self.size.into(), - } + fn base_ptr(&mut self) -> *mut u8 { + self.base.as_ptr() } fn needs_init(&self) -> bool { @@ -845,7 +838,10 @@ impl LocalMemory { } pub fn vmmemory(&mut self) -> VMMemoryDefinition { - self.alloc.vmmemory() + VMMemoryDefinition { + base: self.alloc.base_ptr(), + current_length: self.alloc.byte_size().into(), + } } pub fn byte_size(&self) -> usize { From 457f63b3f0e7e818440f061e658e72c0f3c837c2 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 6 Nov 2024 13:16:07 -0800 Subject: [PATCH 04/11] Move CoW/max limits into `LocalMemory` Further simplify `RuntimeLinearMemory` by moving these responsibilities up a layer into the `LocalMemory` structure. --- crates/wasmtime/src/runtime/memory.rs | 8 +- .../wasmtime/src/runtime/trampoline/memory.rs | 11 +- .../instance/allocator/pooling/memory_pool.rs | 1 + crates/wasmtime/src/runtime/vm/memory.rs | 223 +++++++++--------- .../src/runtime/vm/threads/shared_memory.rs | 7 +- tests/all/memory_creator.rs | 4 +- 6 files changed, 132 insertions(+), 122 deletions(-) diff --git a/crates/wasmtime/src/runtime/memory.rs b/crates/wasmtime/src/runtime/memory.rs index efc705e6419d..d7b48583d8a4 100644 --- a/crates/wasmtime/src/runtime/memory.rs +++ b/crates/wasmtime/src/runtime/memory.rs @@ -676,11 +676,11 @@ pub unsafe trait LinearMemory: Send + Sync + 'static { /// Returns the number of allocated bytes which are accessible at this time. fn byte_size(&self) -> usize; - /// Returns the maximum number of bytes the memory can grow to. + /// Returns byte capacity of this linear memory's current allocation. /// - /// Returns `None` if the memory is unbounded, or `Some` if memory cannot - /// grow beyond a specified limit. - fn maximum_byte_size(&self) -> Option; + /// Growth up to this value should not relocate the linear memory base + /// pointer. + fn byte_capacity(&self) -> usize; /// Grows this memory to have the `new_size`, in bytes, specified. /// diff --git a/crates/wasmtime/src/runtime/trampoline/memory.rs b/crates/wasmtime/src/runtime/trampoline/memory.rs index 8e337cfec6ed..9de841e6e2a0 100644 --- a/crates/wasmtime/src/runtime/trampoline/memory.rs +++ b/crates/wasmtime/src/runtime/trampoline/memory.rs @@ -3,7 +3,7 @@ use crate::prelude::*; use crate::runtime::vm::mpk::ProtectionKey; use crate::runtime::vm::{ CompiledModuleId, GcHeapAllocationIndex, Imports, InstanceAllocationRequest, InstanceAllocator, - InstanceAllocatorImpl, Memory, MemoryAllocationIndex, MemoryImage, ModuleRuntimeInfo, + InstanceAllocatorImpl, Memory, MemoryAllocationIndex, ModuleRuntimeInfo, OnDemandInstanceAllocator, RuntimeLinearMemory, RuntimeMemoryCreator, SharedMemory, StorePtr, Table, TableAllocationIndex, }; @@ -83,8 +83,8 @@ impl RuntimeLinearMemory for LinearMemoryProxy { self.mem.byte_size() } - fn maximum_byte_size(&self) -> Option { - self.mem.maximum_byte_size() + fn byte_capacity(&self) -> usize { + self.mem.byte_capacity() } fn grow_to(&mut self, new_size: usize) -> Result<()> { @@ -95,10 +95,6 @@ impl RuntimeLinearMemory for LinearMemoryProxy { self.mem.as_ptr() } - fn needs_init(&self) -> bool { - true - } - fn wasm_accessible(&self) -> Range { self.mem.wasm_accessible() } @@ -121,7 +117,6 @@ impl RuntimeMemoryCreator for MemoryCreatorProxy { tunables: &Tunables, minimum: usize, maximum: Option, - _: Option<&Arc>, ) -> Result> { let style = MemoryStyle::for_memory(*ty, tunables); let reserved_size_in_bytes = match style { diff --git a/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs b/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs index 7e8dfa36c948..596948cd1578 100644 --- a/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs +++ b/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs @@ -376,6 +376,7 @@ impl MemoryPool { Memory::new_static( ty, + tunables, base_ptr, base_capacity, slot, diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index 011fc149f9b8..a88d6eaf3dab 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -24,8 +24,6 @@ pub trait RuntimeMemoryCreator: Send + Sync { tunables: &Tunables, minimum: usize, maximum: Option, - // Optionally, a memory image for CoW backing. - memory_image: Option<&Arc>, ) -> Result>; } @@ -40,26 +38,20 @@ impl RuntimeMemoryCreator for DefaultMemoryCreator { tunables: &Tunables, minimum: usize, maximum: Option, - memory_image: Option<&Arc>, ) -> Result> { - Ok(Box::new(MmapMemory::new( - ty, - tunables, - minimum, - maximum, - memory_image, - )?)) + Ok(Box::new(MmapMemory::new(ty, tunables, minimum, maximum)?)) } } /// A linear memory and its backing storage. pub trait RuntimeLinearMemory: Send + Sync { - /// Returns the number of allocated bytes. + /// Returns the number bytes that this linear memory can access. fn byte_size(&self) -> usize; - /// Returns the maximum number of bytes the memory can grow to. - /// Returns `None` if the memory is unbounded. - fn maximum_byte_size(&self) -> Option; + /// Returns the maximal number of bytes the current allocation can access. + /// + /// Growth up to this value should not relocate the base pointer. + fn byte_capacity(&self) -> usize; /// Returns whether the base pointer of this memory may be relocated over /// time to accommodate requests for growth. @@ -74,19 +66,22 @@ pub trait RuntimeLinearMemory: Send + Sync { /// Returns a pointer to the base of this linear memory allocation. fn base_ptr(&mut self) -> *mut u8; - /// Does this memory need initialization? It may not if it already - /// has initial contents courtesy of the `MemoryImage` passed to - /// `RuntimeMemoryCreator::new_memory()`. - fn needs_init(&self) -> bool; - /// Returns the range of addresses that may be reached by WebAssembly. /// /// This starts at the base of linear memory and ends at the end of the /// guard pages, if any. fn wasm_accessible(&self) -> Range; - fn unwrap_static_image(self: Box) -> MemoryImageSlot { - panic!("not a static memory") + /// Internal method for Wasmtime when used in conjunction with CoW images. + /// This is used to inform the underlying memory that the size of memory has + /// changed. + /// + /// Note that this is hidden and panics by default as embedders using custom + /// memory without CoW images shouldn't have to worry about this. + #[doc(hidden)] + fn set_byte_size(&mut self, len: usize) { + let _ = len; + panic!("CoW images used with this memory and it doesn't support it"); } } @@ -127,10 +122,6 @@ pub struct MmapMemory { // optimize loads and stores with constant offsets. pre_guard_size: usize, offset_guard_size: usize, - - // An optional CoW mapping that provides the initial content of this - // MmapMemory, if mapped. - memory_image: Option, } impl MmapMemory { @@ -141,7 +132,6 @@ impl MmapMemory { tunables: &Tunables, minimum: usize, maximum: Option, - memory_image: Option<&Arc>, ) -> Result { // It's a programmer error for these two configuration values to exceed // the host available address space, so panic if such a configuration is @@ -199,26 +189,6 @@ impl MmapMemory { mmap.make_accessible(pre_guard_bytes, accessible)?; } - // If a memory image was specified, try to create the MemoryImageSlot on - // top of our mmap. - let memory_image = match memory_image { - Some(image) => { - let base = unsafe { mmap.as_mut_ptr().add(pre_guard_bytes) }; - let mut slot = MemoryImageSlot::create( - base.cast(), - minimum, - alloc_bytes + extra_to_reserve_on_growth, - ); - slot.instantiate(minimum, Some(image), ty, tunables)?; - // On drop, we will unmap our mmap'd range that this slot was - // mapped on top of, so there is no need for the slot to wipe - // it with an anonymous mapping first. - slot.no_clear_on_drop(); - Some(slot) - } - None => None, - }; - Ok(Self { mmap, len: minimum, @@ -226,7 +196,6 @@ impl MmapMemory { pre_guard_size: pre_guard_bytes, offset_guard_size: offset_guard_bytes, extra_to_reserve_on_growth, - memory_image, memory_may_move: ty.memory_may_move(tunables), }) } @@ -247,8 +216,8 @@ impl RuntimeLinearMemory for MmapMemory { self.len } - fn maximum_byte_size(&self) -> Option { - self.maximum + fn byte_capacity(&self) -> usize { + self.mmap.len() - self.offset_guard_size - self.pre_guard_size } fn memory_may_move(&self) -> bool { @@ -291,18 +260,7 @@ impl RuntimeLinearMemory for MmapMemory { dst.copy_from_slice(src); } - // Now drop the MemoryImageSlot, if any. We've lost the CoW - // advantages by explicitly copying all data, but we have - // preserved all of its content; so we no longer need the - // mapping. We need to do this before we (implicitly) drop the - // `mmap` field by overwriting it below. - drop(self.memory_image.take()); - self.mmap = new_mmap; - } else if let Some(image) = self.memory_image.as_mut() { - // MemoryImageSlot has its own growth mechanisms; defer to its - // implementation. - image.set_heap_limit(new_size)?; } else { // If the new size of this heap fits within the existing allocation // then all we need to do is to make the new pages accessible. This @@ -336,14 +294,12 @@ impl RuntimeLinearMemory for MmapMemory { Ok(()) } - fn base_ptr(&mut self) -> *mut u8 { - unsafe { self.mmap.as_mut_ptr().add(self.pre_guard_size) } + fn set_byte_size(&mut self, len: usize) { + self.len = len; } - fn needs_init(&self) -> bool { - // If we're using a CoW mapping, then no initialization - // is needed. - self.memory_image.is_none() + fn base_ptr(&mut self) -> *mut u8 { + unsafe { self.mmap.as_mut_ptr().add(self.pre_guard_size) } } fn wasm_accessible(&self) -> Range { @@ -369,10 +325,6 @@ struct StaticMemory { /// The size, in bytes, of the virtual address allocation starting at `base` /// and going to the end of the guard pages at the end of the linear memory. memory_and_guard_size: usize, - - /// The image management, if any, for this memory. Owned here and - /// returned to the pooling allocator when termination occurs. - memory_image: MemoryImageSlot, } impl StaticMemory { @@ -381,7 +333,6 @@ impl StaticMemory { base_capacity: usize, initial_size: usize, maximum_size: Option, - memory_image: MemoryImageSlot, memory_and_guard_size: usize, ) -> Result { if base_capacity < initial_size { @@ -403,7 +354,6 @@ impl StaticMemory { base: SendSyncPtr::new(NonNull::new(base_ptr).unwrap()), capacity: base_capacity, size: initial_size, - memory_image, memory_and_guard_size, }) } @@ -414,8 +364,8 @@ impl RuntimeLinearMemory for StaticMemory { self.size } - fn maximum_byte_size(&self) -> Option { - Some(self.capacity) + fn byte_capacity(&self) -> usize { + self.capacity } fn memory_may_move(&self) -> bool { @@ -427,19 +377,17 @@ impl RuntimeLinearMemory for StaticMemory { // prior to arriving here. assert!(new_byte_size <= self.capacity); - self.memory_image.set_heap_limit(new_byte_size)?; - // Update our accounting of the available size. self.size = new_byte_size; Ok(()) } - fn base_ptr(&mut self) -> *mut u8 { - self.base.as_ptr() + fn set_byte_size(&mut self, len: usize) { + self.size = len; } - fn needs_init(&self) -> bool { - !self.memory_image.has_image() + fn base_ptr(&mut self) -> *mut u8 { + self.base.as_ptr() } fn wasm_accessible(&self) -> Range { @@ -447,10 +395,6 @@ impl RuntimeLinearMemory for StaticMemory { let end = base + self.memory_and_guard_size; base..end } - - fn unwrap_static_image(self: Box) -> MemoryImageSlot { - self.memory_image - } } /// Representation of a runtime wasm linear memory. @@ -469,7 +413,7 @@ impl Memory { memory_image: Option<&Arc>, ) -> Result { let (minimum, maximum) = Self::limit_new(ty, Some(store))?; - let allocation = creator.new_memory(ty, tunables, minimum, maximum, memory_image)?; + let allocation = creator.new_memory(ty, tunables, minimum, maximum)?; // Double-check that the created memory respects the safety invariant of // whether the memory may move or not at runtime. @@ -487,7 +431,7 @@ impl Memory { "this linear memory should not be able to move its base pointer", ); - let memory = LocalMemory::new(ty, allocation); + let memory = LocalMemory::new(ty, tunables, allocation, memory_image)?; Ok(if ty.shared { Memory::Shared(SharedMemory::wrap(ty, memory)?) } else { @@ -498,6 +442,7 @@ impl Memory { /// Create a new static (immovable) memory instance for the specified plan. pub fn new_static( ty: &wasmtime_environ::Memory, + tunables: &Tunables, base_ptr: *mut u8, base_capacity: usize, memory_image: MemoryImageSlot, @@ -510,11 +455,12 @@ impl Memory { base_capacity, minimum, maximum, - memory_image, memory_and_guard_size, )?; let allocation = Box::new(pooled_memory); - let memory = LocalMemory::new(ty, allocation); + let mut memory = LocalMemory::new(ty, tunables, allocation, None)?; + assert!(memory.memory_image.is_none()); + memory.memory_image = Some(memory_image); Ok(if ty.shared { // FIXME(#4244): not supported with the pooling allocator (which // `new_static` is always used with), see `MemoryPool::validate` as @@ -755,11 +701,42 @@ impl Memory { pub struct LocalMemory { alloc: Box, ty: wasmtime_environ::Memory, + + /// An optional CoW mapping that provides the initial content of this + /// memory. + memory_image: Option, } impl LocalMemory { - pub fn new(ty: &wasmtime_environ::Memory, alloc: Box) -> LocalMemory { - LocalMemory { ty: *ty, alloc } + pub fn new( + ty: &wasmtime_environ::Memory, + tunables: &Tunables, + mut alloc: Box, + memory_image: Option<&Arc>, + ) -> Result { + // If a memory image was specified, try to create the MemoryImageSlot on + // top of our mmap. + let memory_image = match memory_image { + Some(image) => { + let mut slot = MemoryImageSlot::create( + alloc.base_ptr().cast(), + alloc.byte_size(), + alloc.byte_capacity(), + ); + // On drop, we will unmap our mmap'd range that this slot was + // mapped on top of, so there is no need for the slot to wipe + // it with an anonymous mapping first. + slot.no_clear_on_drop(); + slot.instantiate(alloc.byte_size(), Some(image), ty, tunables)?; + Some(slot) + } + None => None, + }; + Ok(LocalMemory { + ty: *ty, + alloc, + memory_image, + }) } pub fn page_size(&self) -> u64 { @@ -799,7 +776,11 @@ impl LocalMemory { .saturating_add(old_byte_size) .min(absolute_max); - let maximum = self.alloc.maximum_byte_size(); + let maximum = self + .ty + .maximum_byte_size() + .ok() + .and_then(|n| usize::try_from(n).ok()); // Store limiter gets first chance to reject memory_growing. if let Some(store) = &mut store { @@ -808,22 +789,49 @@ impl LocalMemory { } } - // Never exceed maximum, even if limiter permitted it. - if let Some(max) = maximum { - if new_byte_size > max { - if let Some(store) = store { - // FIXME: shared memories may not have an associated store - // to report the growth failure to but the error should not - // be dropped - // (https://github.com/bytecodealliance/wasmtime/issues/4240). - store.memory_grow_failed(format_err!("Memory maximum size exceeded"))?; + let result = (|| -> Result<()> { + // Never exceed maximum, even if limiter permitted it. + if let Some(max) = maximum { + if new_byte_size > max { + bail!("Memory maximum size exceeded"); } - return Ok(None); } - } - match self.alloc.grow_to(new_byte_size) { - Ok(_) => Ok(Some((old_byte_size, new_byte_size))), + // If memory isn't allowed to move then don't let growth happen + // beyond the initial capacity + if !self.alloc.memory_may_move() && new_byte_size > self.alloc.byte_capacity() { + bail!("Memory maximum size exceeded"); + } + + // If we have a CoW image overlay then let it manage accessible + // bytes. Once the heap limit is modified inform the underlying + // allocation that the size has changed. + // + // If the growth is going beyond the size of the heap image then + // discard it. This should only happen for `MmapMemory` where + // `no_clear_on_drop` is set so the destructor doesn't do anything. + // For now be maximally sure about this by asserting that memory can + // indeed move and that we're on unix. If this wants to run + // somewhere else like Windows or with other allocations this may + // need adjusting. + if let Some(image) = &mut self.memory_image { + if new_byte_size <= self.alloc.byte_capacity() { + image.set_heap_limit(new_byte_size)?; + self.alloc.set_byte_size(new_byte_size); + return Ok(()); + } + assert!(cfg!(unix)); + assert!(self.alloc.memory_may_move()); + self.memory_image = None; + } + + // And failing all that fall back to the underlying allocation to + // grow it. + self.alloc.grow_to(new_byte_size) + })(); + + match result { + Ok(()) => Ok(Some((old_byte_size, new_byte_size))), Err(e) => { // FIXME: shared memories may not have an associated store to // report the growth failure to but the error should not be @@ -849,7 +857,10 @@ impl LocalMemory { } pub fn needs_init(&self) -> bool { - self.alloc.needs_init() + match &self.memory_image { + Some(image) => !image.has_image(), + None => true, + } } pub fn memory_may_move(&self) -> bool { @@ -861,7 +872,7 @@ impl LocalMemory { } pub fn unwrap_static_image(self) -> MemoryImageSlot { - self.alloc.unwrap_static_image() + self.memory_image.unwrap() } } diff --git a/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs b/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs index db5e7be3605f..316ac5d3a391 100644 --- a/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs +++ b/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs @@ -32,8 +32,11 @@ impl SharedMemory { /// Construct a new [`SharedMemory`]. pub fn new(ty: &wasmtime_environ::Memory, tunables: &Tunables) -> Result { let (minimum_bytes, maximum_bytes) = Memory::limit_new(ty, None)?; - let mmap_memory = MmapMemory::new(ty, tunables, minimum_bytes, maximum_bytes, None)?; - Self::wrap(ty, LocalMemory::new(ty, Box::new(mmap_memory))) + let mmap_memory = MmapMemory::new(ty, tunables, minimum_bytes, maximum_bytes)?; + Self::wrap( + ty, + LocalMemory::new(ty, tunables, Box::new(mmap_memory), None)?, + ) } /// Wrap an existing [Memory] with the locking provided by a [SharedMemory]. diff --git a/tests/all/memory_creator.rs b/tests/all/memory_creator.rs index 6d79e3c0d3db..b71331bde710 100644 --- a/tests/all/memory_creator.rs +++ b/tests/all/memory_creator.rs @@ -57,8 +57,8 @@ mod not_for_windows { self.used_wasm_bytes } - fn maximum_byte_size(&self) -> Option { - Some(self.size - self.guard_size) + fn byte_capacity(&self) -> usize { + self.size - self.guard_size } fn grow_to(&mut self, new_size: usize) -> wasmtime::Result<()> { From 9a01031cb97d1bd20e26bf80ca01a596d7cdb048 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 6 Nov 2024 13:33:33 -0800 Subject: [PATCH 05/11] Simplify `memory_may_move` handling in `LocalMemory` No need to plumb it through all the traits any more. Now it's possible to only have it handled in `LocalMemory` and other pieces don't have to worry about it. --- .../wasmtime/src/runtime/trampoline/memory.rs | 7 -- crates/wasmtime/src/runtime/vm/memory.rs | 69 +++++++------------ .../src/runtime/vm/threads/shared_memory.rs | 3 - 3 files changed, 25 insertions(+), 54 deletions(-) diff --git a/crates/wasmtime/src/runtime/trampoline/memory.rs b/crates/wasmtime/src/runtime/trampoline/memory.rs index 9de841e6e2a0..76ac94b6fca1 100644 --- a/crates/wasmtime/src/runtime/trampoline/memory.rs +++ b/crates/wasmtime/src/runtime/trampoline/memory.rs @@ -98,13 +98,6 @@ impl RuntimeLinearMemory for LinearMemoryProxy { fn wasm_accessible(&self) -> Range { self.mem.wasm_accessible() } - - fn memory_may_move(&self) -> bool { - // FIXME(#9568): should propagate this up to the `LinearMemory` trait, - // but for now pessimistically assume that consumers might move linear - // memory. - true - } } #[derive(Clone)] diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index a88d6eaf3dab..81519bbdfd2f 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -53,10 +53,6 @@ pub trait RuntimeLinearMemory: Send + Sync { /// Growth up to this value should not relocate the base pointer. fn byte_capacity(&self) -> usize; - /// Returns whether the base pointer of this memory may be relocated over - /// time to accommodate requests for growth. - fn memory_may_move(&self) -> bool; - /// Grow memory to the specified amount of bytes. /// /// Returns an error if memory can't be grown by the specified amount @@ -115,9 +111,6 @@ pub struct MmapMemory { // specified so that the cost of repeated growth is amortized. extra_to_reserve_on_growth: usize, - // Whether or not this memory is allowed to relocate the base pointer. - memory_may_move: bool, - // Size in bytes of extra guard pages before the start and after the end to // optimize loads and stores with constant offsets. pre_guard_size: usize, @@ -196,7 +189,6 @@ impl MmapMemory { pre_guard_size: pre_guard_bytes, offset_guard_size: offset_guard_bytes, extra_to_reserve_on_growth, - memory_may_move: ty.memory_may_move(tunables), }) } @@ -220,10 +212,6 @@ impl RuntimeLinearMemory for MmapMemory { self.mmap.len() - self.offset_guard_size - self.pre_guard_size } - fn memory_may_move(&self) -> bool { - self.memory_may_move - } - fn grow_to(&mut self, new_size: usize) -> Result<()> { assert!(usize_is_multiple_of_host_page_size(self.offset_guard_size)); assert!(usize_is_multiple_of_host_page_size(self.pre_guard_size)); @@ -231,10 +219,6 @@ impl RuntimeLinearMemory for MmapMemory { let new_accessible = round_usize_up_to_host_pages(new_size)?; if new_accessible > self.mmap.len() - self.offset_guard_size - self.pre_guard_size { - if !self.memory_may_move { - bail!("disallowing growth as base pointer of memory cannot move"); - } - // If the new size of this heap exceeds the current size of the // allocation we have, then this must be a dynamic heap. Use // `new_size` to calculate a new size of an allocation, allocate it, @@ -368,10 +352,6 @@ impl RuntimeLinearMemory for StaticMemory { self.capacity } - fn memory_may_move(&self) -> bool { - false - } - fn grow_to(&mut self, new_byte_size: usize) -> Result<()> { // Never exceed the static memory size; this check should have been made // prior to arriving here. @@ -414,23 +394,6 @@ impl Memory { ) -> Result { let (minimum, maximum) = Self::limit_new(ty, Some(store))?; let allocation = creator.new_memory(ty, tunables, minimum, maximum)?; - - // Double-check that the created memory respects the safety invariant of - // whether the memory may move or not at runtime. - // - // * If the memory is allowed to move, that's ok. - // * If the allocation doesn't allow the memory to move, that's ok. - // * If the heap has a static size meaning the min is the same as the - // max, that's ok since it'll never be requested to move. - // - // Otherwise something went wrong so trigger an assert. - assert!( - ty.memory_may_move(tunables) - || !allocation.memory_may_move() - || ty.static_heap_size().is_some(), - "this linear memory should not be able to move its base pointer", - ); - let memory = LocalMemory::new(ty, tunables, allocation, memory_image)?; Ok(if ty.shared { Memory::Shared(SharedMemory::wrap(ty, memory)?) @@ -458,9 +421,16 @@ impl Memory { memory_and_guard_size, )?; let allocation = Box::new(pooled_memory); + + // Configure some defaults a bit differently for this memory within the + // `LocalMemory` structure created, notably we already have + // `memory_image` and regardless of configuration settings this memory + // can't move its base pointer since it's a fixed allocation. let mut memory = LocalMemory::new(ty, tunables, allocation, None)?; assert!(memory.memory_image.is_none()); memory.memory_image = Some(memory_image); + memory.memory_may_move = false; + Ok(if ty.shared { // FIXME(#4244): not supported with the pooling allocator (which // `new_static` is always used with), see `MemoryPool::validate` as @@ -701,6 +671,7 @@ impl Memory { pub struct LocalMemory { alloc: Box, ty: wasmtime_environ::Memory, + memory_may_move: bool, /// An optional CoW mapping that provides the initial content of this /// memory. @@ -735,6 +706,7 @@ impl LocalMemory { Ok(LocalMemory { ty: *ty, alloc, + memory_may_move: ty.memory_may_move(tunables), memory_image, }) } @@ -789,6 +761,11 @@ impl LocalMemory { } } + // Save the original base pointer to assert the invariant that growth up + // to the byte capacity never relocates the base pointer. + let base_ptr_before = self.alloc.base_ptr(); + let required_to_not_move_memory = new_byte_size <= self.alloc.byte_capacity(); + let result = (|| -> Result<()> { // Never exceed maximum, even if limiter permitted it. if let Some(max) = maximum { @@ -799,7 +776,7 @@ impl LocalMemory { // If memory isn't allowed to move then don't let growth happen // beyond the initial capacity - if !self.alloc.memory_may_move() && new_byte_size > self.alloc.byte_capacity() { + if !self.memory_may_move && new_byte_size > self.alloc.byte_capacity() { bail!("Memory maximum size exceeded"); } @@ -821,7 +798,7 @@ impl LocalMemory { return Ok(()); } assert!(cfg!(unix)); - assert!(self.alloc.memory_may_move()); + assert!(self.memory_may_move); self.memory_image = None; } @@ -831,7 +808,15 @@ impl LocalMemory { })(); match result { - Ok(()) => Ok(Some((old_byte_size, new_byte_size))), + Ok(()) => { + // On successful growth double-check that the base pointer + // didn't move if it shouldn't have. + if required_to_not_move_memory { + assert_eq!(base_ptr_before, self.alloc.base_ptr()); + } + + Ok(Some((old_byte_size, new_byte_size))) + } Err(e) => { // FIXME: shared memories may not have an associated store to // report the growth failure to but the error should not be @@ -863,10 +848,6 @@ impl LocalMemory { } } - pub fn memory_may_move(&self) -> bool { - self.alloc.memory_may_move() - } - pub fn wasm_accessible(&self) -> Range { self.alloc.wasm_accessible() } diff --git a/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs b/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs index 316ac5d3a391..05642223c6ec 100644 --- a/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs +++ b/crates/wasmtime/src/runtime/vm/threads/shared_memory.rs @@ -44,9 +44,6 @@ impl SharedMemory { if !ty.shared { bail!("shared memory must have a `shared` memory type"); } - if memory.memory_may_move() { - bail!("shared memory cannot use a memory which may relocate the base pointer") - } Ok(Self(Arc::new(SharedMemoryInner { ty: *ty, spot: ParkingSpot::default(), From 2f0180c7b6f73f00b3cf448d6254cd4c1b1910db Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 6 Nov 2024 13:46:54 -0800 Subject: [PATCH 06/11] Move `wasm_accessible` into `LocalMemory` Further simplify custom traits and mmap/static memory by moving more responsibility into `LocalMemory`. --- crates/wasmtime/src/runtime/memory.rs | 5 -- .../wasmtime/src/runtime/trampoline/memory.rs | 7 +-- .../instance/allocator/pooling/memory_pool.rs | 28 +++------ crates/wasmtime/src/runtime/vm/memory.rs | 59 +++++++------------ crates/wasmtime/src/runtime/vm/mmap.rs | 2 +- .../src/runtime/vm/sys/custom/mmap.rs | 2 +- .../wasmtime/src/runtime/vm/sys/miri/mmap.rs | 2 +- .../wasmtime/src/runtime/vm/sys/unix/mmap.rs | 2 +- .../src/runtime/vm/sys/windows/mmap.rs | 2 +- tests/all/memory_creator.rs | 7 --- 10 files changed, 36 insertions(+), 80 deletions(-) diff --git a/crates/wasmtime/src/runtime/memory.rs b/crates/wasmtime/src/runtime/memory.rs index d7b48583d8a4..5c1bda33b424 100644 --- a/crates/wasmtime/src/runtime/memory.rs +++ b/crates/wasmtime/src/runtime/memory.rs @@ -6,7 +6,6 @@ use crate::Trap; use crate::{AsContext, AsContextMut, Engine, MemoryType, StoreContext, StoreContextMut}; use core::cell::UnsafeCell; use core::fmt; -use core::ops::Range; use core::slice; use core::time::Duration; @@ -691,10 +690,6 @@ pub unsafe trait LinearMemory: Send + Sync + 'static { /// Return the allocated memory as a mutable pointer to u8. fn as_ptr(&self) -> *mut u8; - - /// Returns the range of native addresses that WebAssembly can natively - /// access from this linear memory, including guard pages. - fn wasm_accessible(&self) -> Range; } /// A memory creator. Can be used to provide a memory creator diff --git a/crates/wasmtime/src/runtime/trampoline/memory.rs b/crates/wasmtime/src/runtime/trampoline/memory.rs index 76ac94b6fca1..76c46bd3182b 100644 --- a/crates/wasmtime/src/runtime/trampoline/memory.rs +++ b/crates/wasmtime/src/runtime/trampoline/memory.rs @@ -10,7 +10,6 @@ use crate::runtime::vm::{ use crate::store::{InstanceId, StoreOpaque}; use crate::MemoryType; use alloc::sync::Arc; -use core::ops::Range; use wasmtime_environ::{ DefinedMemoryIndex, DefinedTableIndex, EntityIndex, HostPtr, MemoryStyle, Module, Tunables, VMOffsets, @@ -91,13 +90,9 @@ impl RuntimeLinearMemory for LinearMemoryProxy { self.mem.grow_to(new_size) } - fn base_ptr(&mut self) -> *mut u8 { + fn base_ptr(&self) -> *mut u8 { self.mem.as_ptr() } - - fn wasm_accessible(&self) -> Range { - self.mem.wasm_accessible() - } } #[derive(Clone)] diff --git a/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs b/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs index 596948cd1578..54ce459042f9 100644 --- a/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs +++ b/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs @@ -63,7 +63,7 @@ use crate::{prelude::*, vm::round_usize_up_to_host_pages}; use std::ffi::c_void; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Mutex; -use wasmtime_environ::{DefinedMemoryIndex, MemoryStyle, Module, Tunables}; +use wasmtime_environ::{DefinedMemoryIndex, Module, Tunables}; /// A set of allocator slots. /// @@ -338,16 +338,10 @@ impl MemoryPool { // satisfied by the configuration of this pooling allocator. This // should be returned as an error through `validate_memory_plans` // but double-check here to be sure. - let style = MemoryStyle::for_memory(*ty, tunables); - match style { - MemoryStyle::Static { byte_reservation } => { - assert!( - byte_reservation - <= u64::try_from(self.layout.bytes_to_next_stripe_slot()).unwrap() - ); - } - MemoryStyle::Dynamic { .. } => {} - } + assert!( + tunables.memory_reservation + tunables.memory_guard_size + <= u64::try_from(self.layout.bytes_to_next_stripe_slot()).unwrap() + ); let base_ptr = self.get_base(allocation_index); let base_capacity = self.layout.max_memory_bytes; @@ -374,15 +368,9 @@ impl MemoryPool { let initial_size = usize::try_from(initial_size).unwrap(); slot.instantiate(initial_size, image, ty, tunables)?; - Memory::new_static( - ty, - tunables, - base_ptr, - base_capacity, - slot, - self.layout.bytes_to_next_stripe_slot(), - unsafe { &mut *request.store.get().unwrap() }, - ) + Memory::new_static(ty, tunables, base_ptr, base_capacity, slot, unsafe { + &mut *request.store.get().unwrap() + }) })() { Ok(memory) => Ok((allocation_index, memory)), Err(e) => { diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index 81519bbdfd2f..ab64cf3ebd75 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -60,13 +60,7 @@ pub trait RuntimeLinearMemory: Send + Sync { fn grow_to(&mut self, size: usize) -> Result<()>; /// Returns a pointer to the base of this linear memory allocation. - fn base_ptr(&mut self) -> *mut u8; - - /// Returns the range of addresses that may be reached by WebAssembly. - /// - /// This starts at the base of linear memory and ends at the end of the - /// guard pages, if any. - fn wasm_accessible(&self) -> Range; + fn base_ptr(&self) -> *mut u8; /// Internal method for Wasmtime when used in conjunction with CoW images. /// This is used to inform the underlying memory that the size of memory has @@ -282,15 +276,9 @@ impl RuntimeLinearMemory for MmapMemory { self.len = len; } - fn base_ptr(&mut self) -> *mut u8 { + fn base_ptr(&self) -> *mut u8 { unsafe { self.mmap.as_mut_ptr().add(self.pre_guard_size) } } - - fn wasm_accessible(&self) -> Range { - let base = self.mmap.as_ptr() as usize + self.pre_guard_size; - let end = base + (self.mmap.len() - self.pre_guard_size); - base..end - } } /// A "static" memory where the lifetime of the backing memory is managed @@ -305,10 +293,6 @@ struct StaticMemory { /// The current size, in bytes, of this memory. size: usize, - - /// The size, in bytes, of the virtual address allocation starting at `base` - /// and going to the end of the guard pages at the end of the linear memory. - memory_and_guard_size: usize, } impl StaticMemory { @@ -317,7 +301,6 @@ impl StaticMemory { base_capacity: usize, initial_size: usize, maximum_size: Option, - memory_and_guard_size: usize, ) -> Result { if base_capacity < initial_size { bail!( @@ -338,7 +321,6 @@ impl StaticMemory { base: SendSyncPtr::new(NonNull::new(base_ptr).unwrap()), capacity: base_capacity, size: initial_size, - memory_and_guard_size, }) } } @@ -366,15 +348,9 @@ impl RuntimeLinearMemory for StaticMemory { self.size = len; } - fn base_ptr(&mut self) -> *mut u8 { + fn base_ptr(&self) -> *mut u8 { self.base.as_ptr() } - - fn wasm_accessible(&self) -> Range { - let base = self.base.as_ptr() as usize; - let end = base + self.memory_and_guard_size; - base..end - } } /// Representation of a runtime wasm linear memory. @@ -409,17 +385,10 @@ impl Memory { base_ptr: *mut u8, base_capacity: usize, memory_image: MemoryImageSlot, - memory_and_guard_size: usize, store: &mut dyn VMStore, ) -> Result { let (minimum, maximum) = Self::limit_new(ty, Some(store))?; - let pooled_memory = StaticMemory::new( - base_ptr, - base_capacity, - minimum, - maximum, - memory_and_guard_size, - )?; + let pooled_memory = StaticMemory::new(base_ptr, base_capacity, minimum, maximum)?; let allocation = Box::new(pooled_memory); // Configure some defaults a bit differently for this memory within the @@ -672,6 +641,8 @@ pub struct LocalMemory { alloc: Box, ty: wasmtime_environ::Memory, memory_may_move: bool, + memory_guard_size: usize, + memory_reservation: usize, /// An optional CoW mapping that provides the initial content of this /// memory. @@ -682,7 +653,7 @@ impl LocalMemory { pub fn new( ty: &wasmtime_environ::Memory, tunables: &Tunables, - mut alloc: Box, + alloc: Box, memory_image: Option<&Arc>, ) -> Result { // If a memory image was specified, try to create the MemoryImageSlot on @@ -708,6 +679,8 @@ impl LocalMemory { alloc, memory_may_move: ty.memory_may_move(tunables), memory_image, + memory_guard_size: tunables.memory_guard_size.try_into().unwrap(), + memory_reservation: tunables.memory_reservation.try_into().unwrap(), }) } @@ -849,7 +822,19 @@ impl LocalMemory { } pub fn wasm_accessible(&self) -> Range { - self.alloc.wasm_accessible() + let base = self.alloc.base_ptr() as usize; + // From the base add: + // + // * max(capacity, reservation) -- all memory is guaranteed to have at + // least `memory_reservation`, but capacity may go beyond that. + // * memory_guard_size - wasm is allowed to hit the guard page for + // sigsegv for example. + // + // and this computes the range that wasm is allowed to load from and + // deterministically trap or succeed. + let end = + base + self.alloc.byte_capacity().max(self.memory_reservation) + self.memory_guard_size; + base..end } pub fn unwrap_static_image(self) -> MemoryImageSlot { diff --git a/crates/wasmtime/src/runtime/vm/mmap.rs b/crates/wasmtime/src/runtime/vm/mmap.rs index 2d955d5ebddc..ef0ead8e7507 100644 --- a/crates/wasmtime/src/runtime/vm/mmap.rs +++ b/crates/wasmtime/src/runtime/vm/mmap.rs @@ -143,7 +143,7 @@ impl Mmap { /// Return the allocated memory as a mutable pointer to u8. #[inline] - pub fn as_mut_ptr(&mut self) -> *mut u8 { + pub fn as_mut_ptr(&self) -> *mut u8 { self.sys.as_mut_ptr() } diff --git a/crates/wasmtime/src/runtime/vm/sys/custom/mmap.rs b/crates/wasmtime/src/runtime/vm/sys/custom/mmap.rs index 22844d6a873b..5a647ea1d868 100644 --- a/crates/wasmtime/src/runtime/vm/sys/custom/mmap.rs +++ b/crates/wasmtime/src/runtime/vm/sys/custom/mmap.rs @@ -61,7 +61,7 @@ impl Mmap { } #[inline] - pub fn as_mut_ptr(&mut self) -> *mut u8 { + pub fn as_mut_ptr(&self) -> *mut u8 { self.memory.as_ptr().cast() } diff --git a/crates/wasmtime/src/runtime/vm/sys/miri/mmap.rs b/crates/wasmtime/src/runtime/vm/sys/miri/mmap.rs index 964d19fda6f3..e7fba6c503ea 100644 --- a/crates/wasmtime/src/runtime/vm/sys/miri/mmap.rs +++ b/crates/wasmtime/src/runtime/vm/sys/miri/mmap.rs @@ -63,7 +63,7 @@ impl Mmap { self.memory.as_ptr() as *const u8 } - pub fn as_mut_ptr(&mut self) -> *mut u8 { + pub fn as_mut_ptr(&self) -> *mut u8 { self.memory.as_ptr().cast() } diff --git a/crates/wasmtime/src/runtime/vm/sys/unix/mmap.rs b/crates/wasmtime/src/runtime/vm/sys/unix/mmap.rs index 231449265b3b..fa28be983662 100644 --- a/crates/wasmtime/src/runtime/vm/sys/unix/mmap.rs +++ b/crates/wasmtime/src/runtime/vm/sys/unix/mmap.rs @@ -126,7 +126,7 @@ impl Mmap { } #[inline] - pub fn as_mut_ptr(&mut self) -> *mut u8 { + pub fn as_mut_ptr(&self) -> *mut u8 { self.memory.as_ptr().cast() } diff --git a/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs b/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs index 33bac0258f95..63dab82b1e7c 100644 --- a/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs +++ b/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs @@ -158,7 +158,7 @@ impl Mmap { } #[inline] - pub fn as_mut_ptr(&mut self) -> *mut u8 { + pub fn as_mut_ptr(&self) -> *mut u8 { self.memory.as_ptr().cast() } diff --git a/tests/all/memory_creator.rs b/tests/all/memory_creator.rs index b71331bde710..ca14f74995d4 100644 --- a/tests/all/memory_creator.rs +++ b/tests/all/memory_creator.rs @@ -5,7 +5,6 @@ mod not_for_windows { use rustix::mm::{mmap_anonymous, mprotect, munmap, MapFlags, MprotectFlags, ProtFlags}; - use std::ops::Range; use std::ptr::null_mut; use std::sync::{Arc, Mutex}; @@ -78,12 +77,6 @@ mod not_for_windows { fn as_ptr(&self) -> *mut u8 { self.mem as *mut u8 } - - fn wasm_accessible(&self) -> Range { - let base = self.mem; - let end = base + self.size; - base..end - } } struct CustomMemoryCreator { From 2011904ec9cbcdecee533a5da1e3b2a49109881f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 6 Nov 2024 14:01:12 -0800 Subject: [PATCH 07/11] Improve documentation of memories --- crates/wasmtime/src/runtime/vm/memory.rs | 74 +++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index ab64cf3ebd75..ebf70051b7d3 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -1,6 +1,78 @@ //! Memory management for linear memories. //! -//! `RuntimeLinearMemory` is to WebAssembly linear memories what `Table` is to WebAssembly tables. +//! This module implements the runtime data structures that manage linear +//! memories for WebAssembly. There's a number of types here each with various +//! purposes, and this is the high level relationships between types where an +//! arrow here means "builds on top of". +//! +//! ```text +//! ┌─────────────────────┐ +//! │ │ +//! │ Memory ├─────────────┐ +//! │ │ │ +//! └──────────┬──────────┘ │ +//! │ │ +//! │ │ +//! ▼ ▼ +//! ┌─────────────────────┐ ┌──────────────┐ +//! │ │ │ │ +//! │ LocalMemory │◄────┤ SharedMemory │ +//! │ │ │ │ +//! └──────────┬──────────┘ └──────────────┘ +//! │ +//! │ +//! ▼ +//! ┌─────────────────────┐ +//! │ │ +//! │ RuntimeLinearMemory ├─────┬───────┐ +//! │ │ │ │ +//! └──────────┬──────────┘ └───────┼───────────────┐ +//! │ │ │ +//! │ │ │ +//! ▼ ▼ ▼ +//! ┌─────────────────────┐ ┌──────────────┐ ┌─────┐ +//! │ │ │ │ │ │ +//! │ MmapMemory │ │ StaticMemory │ │ ... │ +//! │ │ │ │ │ │ +//! └─────────────────────┘ └──────────────┘ └─────┘ +//! ``` +//! +//! In more detail: +//! +//! * `Memory` - the root of what's actually stored in a wasm instance. This +//! implements the high-level embedder APIs one would expect from a wasm +//! linear memory. +//! +//! * `SharedMemory` - this is one of the variants of a local memory. A shared +//! memory contains `RwLock` where all the real bits happen +//! within the lock. +//! +//! * `LocalMemory` - this is an owned allocation of a linear memory which +//! maintains low-level state that's shared between `SharedMemory` and the +//! instance-local state of `Memory`. One example is that `LocalMemory::grow` +//! has most of the logic around memory growth. +//! +//! * `RuntimeLinearMemory` - this is a trait which `LocalMemory` delegates to. +//! This trait is intentionally relatively simple to be exposed in Wasmtime's +//! embedder API. This is exposed all the way through `wasmtime::Config` so +//! embedders can provide arbitrary implementations. +//! +//! * `MmapMemory` - this is an implementation of `RuntimeLinearMemory` in terms +//! of the platform's mmap primitive. +//! +//! * `StaticMemory` - this is an implementation of `RuntimeLinearMemory` +//! for the pooling allocator where the base pointer is already allocated +//! and contents are managed through `MemoryImageSlot`. +//! +//! Other important types for memories are `MemoryImage` and `MemoryImageSlot` +//! which manage CoW state for memories. This is implemented at the +//! `LocalMemory` layer. +//! +//! FIXME: don't have both RuntimeLinearMemory and wasmtime::LinearMemory, they +//! should be merged together. +//! +//! FIXME: don't have both RuntimeMemoryCreator and wasmtime::MemoryCreator, +//! they should be merged together. use crate::prelude::*; use crate::runtime::vm::mmap::Mmap; From 93ab7667227fce637bff603cd43efc4187510020 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 6 Nov 2024 14:22:40 -0800 Subject: [PATCH 08/11] Fix some CI failures --- crates/c-api/include/wasmtime/config.h | 2 +- crates/c-api/src/config.rs | 30 +++++-------------- .../vm/threads/shared_memory_disabled.rs | 12 ++------ 3 files changed, 12 insertions(+), 32 deletions(-) diff --git a/crates/c-api/include/wasmtime/config.h b/crates/c-api/include/wasmtime/config.h index 21df3271ebec..bafdb85e2631 100644 --- a/crates/c-api/include/wasmtime/config.h +++ b/crates/c-api/include/wasmtime/config.h @@ -446,7 +446,7 @@ WASMTIME_CONFIG_PROP(void, macos_use_mach_ports, bool) * https://docs.wasmtime.dev/api/wasmtime/trait.LinearMemory.html */ typedef uint8_t *(*wasmtime_memory_get_callback_t)(void *env, size_t *byte_size, - size_t *maximum_byte_size); + size_t *byte_capacity); /** * Grow the memory to the `new_size` in bytes. diff --git a/crates/c-api/src/config.rs b/crates/c-api/src/config.rs index 96dd3384a7a9..1648d1d67edf 100644 --- a/crates/c-api/src/config.rs +++ b/crates/c-api/src/config.rs @@ -322,37 +322,23 @@ struct CHostLinearMemory { unsafe impl LinearMemory for CHostLinearMemory { fn byte_size(&self) -> usize { let mut byte_size = 0; - let mut maximum_byte_size = 0; + let mut byte_capacity = 0; let cb = self.get_memory; - cb(self.foreign.data, &mut byte_size, &mut maximum_byte_size); + cb(self.foreign.data, &mut byte_size, &mut byte_capacity); return byte_size; } - fn maximum_byte_size(&self) -> Option { + fn byte_capacity(&self) -> usize { let mut byte_size = 0; - let mut maximum_byte_size = 0; + let mut byte_capacity = 0; let cb = self.get_memory; - cb(self.foreign.data, &mut byte_size, &mut maximum_byte_size); - if maximum_byte_size == 0 { - None - } else { - Some(maximum_byte_size) - } + cb(self.foreign.data, &mut byte_size, &mut byte_capacity); + byte_capacity } fn as_ptr(&self) -> *mut u8 { let mut byte_size = 0; - let mut maximum_byte_size = 0; - let cb = self.get_memory; - cb(self.foreign.data, &mut byte_size, &mut maximum_byte_size) - } - fn wasm_accessible(&self) -> Range { - let mut byte_size = 0; - let mut maximum_byte_size = 0; + let mut byte_capacity = 0; let cb = self.get_memory; - let ptr = cb(self.foreign.data, &mut byte_size, &mut maximum_byte_size); - Range { - start: ptr as usize, - end: ptr as usize + byte_size, - } + cb(self.foreign.data, &mut byte_size, &mut byte_capacity) } fn grow_to(&mut self, new_size: usize) -> Result<()> { let cb = self.grow_memory; diff --git a/crates/wasmtime/src/runtime/vm/threads/shared_memory_disabled.rs b/crates/wasmtime/src/runtime/vm/threads/shared_memory_disabled.rs index 4fc78a1ee585..fd88d1c070cd 100644 --- a/crates/wasmtime/src/runtime/vm/threads/shared_memory_disabled.rs +++ b/crates/wasmtime/src/runtime/vm/threads/shared_memory_disabled.rs @@ -1,7 +1,8 @@ #![allow(missing_docs)] use crate::prelude::*; -use crate::runtime::vm::{RuntimeLinearMemory, VMMemoryDefinition, VMStore, WaitResult}; +use crate::runtime::vm::memory::LocalMemory; +use crate::runtime::vm::{VMMemoryDefinition, VMStore, WaitResult}; use core::ops::Range; use core::time::Duration; use wasmtime_environ::{Trap, Tunables}; @@ -10,10 +11,7 @@ use wasmtime_environ::{Trap, Tunables}; pub enum SharedMemory {} impl SharedMemory { - pub fn wrap( - _ty: &wasmtime_environ::Memory, - _memory: Box, - ) -> Result { + pub fn wrap(_ty: &wasmtime_environ::Memory, _memory: LocalMemory) -> Result { bail!("support for shared memories was disabled at compile time"); } @@ -82,8 +80,4 @@ impl SharedMemory { pub(crate) fn wasm_accessible(&self) -> Range { match *self {} } - - fn memory_may_move(&self) -> bool { - match *self {} - } } From 1297ec86e4c27c3949c5c67275839037555c29d1 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 6 Nov 2024 14:38:06 -0800 Subject: [PATCH 09/11] Handle more CI failures --- crates/c-api/src/config.rs | 1 - crates/fuzzing/src/generators/memory.rs | 15 +++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/crates/c-api/src/config.rs b/crates/c-api/src/config.rs index 1648d1d67edf..82d9496b3534 100644 --- a/crates/c-api/src/config.rs +++ b/crates/c-api/src/config.rs @@ -3,7 +3,6 @@ #![cfg_attr(not(feature = "cache"), allow(unused_imports))] use crate::{handle_result, wasm_memorytype_t, wasmtime_error_t}; -use std::ops::Range; use std::os::raw::c_char; use std::ptr; use std::{ffi::CStr, sync::Arc}; diff --git a/crates/fuzzing/src/generators/memory.rs b/crates/fuzzing/src/generators/memory.rs index 6b005d7e3026..d6e21a47108d 100644 --- a/crates/fuzzing/src/generators/memory.rs +++ b/crates/fuzzing/src/generators/memory.rs @@ -2,7 +2,6 @@ use anyhow::Result; use arbitrary::{Arbitrary, Unstructured}; -use std::ops::Range; use wasmtime::{LinearMemory, MemoryCreator, MemoryType}; /// A description of a memory config, image, etc... that can be used to test @@ -232,7 +231,6 @@ pub struct UnalignedMemory { /// This memory is always one byte larger than the actual size of linear /// memory. src: Vec, - maximum: Option, } unsafe impl LinearMemory for UnalignedMemory { @@ -242,8 +240,8 @@ unsafe impl LinearMemory for UnalignedMemory { self.src.len() - 1 } - fn maximum_byte_size(&self) -> Option { - self.maximum + fn byte_capacity(&self) -> usize { + self.src.capacity() - 1 } fn grow_to(&mut self, new_size: usize) -> Result<()> { @@ -257,12 +255,6 @@ unsafe impl LinearMemory for UnalignedMemory { // of memory is always unaligned. self.src[1..].as_ptr() as *mut _ } - - fn wasm_accessible(&self) -> Range { - let base = self.as_ptr() as usize; - let len = self.byte_size(); - base..base + len - } } /// A mechanism to generate [`UnalignedMemory`] at runtime. @@ -273,7 +265,7 @@ unsafe impl MemoryCreator for UnalignedMemoryCreator { &self, _ty: MemoryType, minimum: usize, - maximum: Option, + _maximum: Option, reserved_size_in_bytes: Option, guard_size_in_bytes: usize, ) -> Result, String> { @@ -281,7 +273,6 @@ unsafe impl MemoryCreator for UnalignedMemoryCreator { assert!(reserved_size_in_bytes.is_none() || reserved_size_in_bytes == Some(0)); Ok(Box::new(UnalignedMemory { src: vec![0; minimum + 1], - maximum, })) } } From 7e15030416cec5f14f48371c4b4dfafa6231830c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 6 Nov 2024 14:47:50 -0800 Subject: [PATCH 10/11] More fixes for CI --- crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs b/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs index 63dab82b1e7c..a2c7a3b6140b 100644 --- a/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs +++ b/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs @@ -118,7 +118,7 @@ impl Mmap { let memory = std::ptr::slice_from_raw_parts_mut(ptr.cast(), len); let memory = SendSyncPtr::new(NonNull::new(memory).unwrap()); - let mut ret = Self { + let ret = Self { memory, is_file: true, }; From 6c482e98a8c6feac510cc859a7a217c77f8d81df Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 7 Nov 2024 12:28:19 -0600 Subject: [PATCH 11/11] Update crates/wasmtime/src/runtime/vm/memory.rs Co-authored-by: Nick Fitzgerald --- crates/wasmtime/src/runtime/vm/memory.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index ebf70051b7d3..284be6b0c6ed 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -24,9 +24,9 @@ //! ▼ //! ┌─────────────────────┐ //! │ │ -//! │ RuntimeLinearMemory ├─────┬───────┐ -//! │ │ │ │ -//! └──────────┬──────────┘ └───────┼───────────────┐ +//! │ RuntimeLinearMemory ├─────────────┬───────────────┐ +//! │ │ │ │ +//! └──────────┬──────────┘ │ │ //! │ │ │ //! │ │ │ //! ▼ ▼ ▼