diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index 11ddf2ca32db..cac9a6df93cf 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -162,6 +162,7 @@ impl ResourceLimiter for StoreLimits { current: usize, desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { Ok(self.alloc(desired - current)) } diff --git a/crates/wasmtime/src/runtime/limits.rs b/crates/wasmtime/src/runtime/limits.rs index f92eb4b4500d..e83ad7ac9edc 100644 --- a/crates/wasmtime/src/runtime/limits.rs +++ b/crates/wasmtime/src/runtime/limits.rs @@ -1,5 +1,41 @@ use crate::prelude::*; +/// Whether a growing heap backs a WebAssembly linear memory or one of +/// Wasmtime's internal GC heaps. +/// +/// This is passed to [`ResourceLimiter::memory_growing`] and +/// [`ResourceLimiter::memory_grown`] so that embedders which account for guest +/// memory separately from runtime overhead can tell the two apart. GC heap +/// capacity is an implementation detail of Wasmtime's garbage collector rather +/// than memory the guest module declared. +/// +/// # When `GcHeap` can be observed +/// +/// A store only ever allocates a GC heap if the `gc` crate feature is compiled +/// in *and* [`Config::wasm_gc`](crate::Config::wasm_gc) (or another +/// GC-dependent proposal) is enabled *and* something in the store actually +/// allocates a GC object. An embedder that leaves GC disabled will never see +/// `GcHeap` and can treat every callback as `LinearMemory`. Embedders that do +/// enable GC must not bill GC heap capacity as guest linear memory: the two are +/// separate pools, and the GC heap grows on the collector's schedule rather +/// than in response to a guest `memory.grow`. +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum MemoryKind { + /// A WebAssembly linear memory declared or imported by a guest module. + LinearMemory, + /// A heap that Wasmtime's garbage collector allocates GC objects out of. + GcHeap, +} + +impl From for MemoryKind { + fn from(kind: wasmtime_environ::MemoryKind) -> MemoryKind { + match kind { + wasmtime_environ::MemoryKind::LinearMemory => MemoryKind::LinearMemory, + wasmtime_environ::MemoryKind::GcHeap => MemoryKind::GcHeap, + } + } +} + /// Value returned by [`ResourceLimiter::instances`] default method pub const DEFAULT_INSTANCE_LIMIT: usize = 10000; /// Value returned by [`ResourceLimiter::tables`] default method @@ -38,6 +74,9 @@ pub trait ResourceLimiter: Send { /// * `maximum` is either the linear memory's maximum or a maximum from an /// instance allocator, also in bytes. A value of `None` /// indicates that the linear memory is unbounded. + /// * `kind` distinguishes a guest linear memory from one of Wasmtime's + /// internal GC heaps. Embedders that attribute memory to the guest should + /// check this before counting the request as guest memory. /// /// The `current` and `desired` amounts are guaranteed to always be /// multiples of the WebAssembly page size, 64KiB. @@ -71,32 +110,50 @@ pub trait ResourceLimiter: Send { current: usize, desired: usize, maximum: Option, + kind: MemoryKind, ) -> Result; - /// Notifies the resource limiter that growing a linear memory, permitted by - /// the `memory_growing` method, has failed. + /// Notifies the resource limiter that growing a heap has failed. /// /// Note that this method is not called if `memory_growing` returns an - /// error. + /// error. It *can* be called without a preceding `memory_growing`, when the + /// request is rejected before the limiter is ever consulted — for instance + /// a growth that the memory's own type cannot represent. /// /// Reasons for failure include: the growth exceeds the `maximum` passed to /// `memory_growing`, or the operating system failed to allocate additional /// memory. In that case, `error` might be downcastable to a `std::io::Error`. /// + /// `kind` is the same kind that was passed to the corresponding + /// `memory_growing` call, so an embedder that reserved something there + /// knows which reservation to release. + /// /// See the details on the return values for `memory_growing` for what the /// return value of this function indicates. - fn memory_grow_failed(&mut self, error: crate::Error) -> Result<()> { + fn memory_grow_failed(&mut self, error: crate::Error, _kind: MemoryKind) -> Result<()> { log::debug!("ignoring memory growth failure error: {error:?}"); Ok(()) } - /// Notifies the resource limiter that a linear-memory growth permitted by + /// Notifies the resource limiter that a growth permitted by /// `memory_growing` has successfully committed. /// - /// This is not called for a memory's initial allocation or for shared - /// memories. `current` and `desired` are the memory's old and new sizes in - /// bytes. - fn memory_grown(&mut self, _current: usize, _desired: usize) {} + /// Every `memory_growing` call that returns `Ok(true)` for a growth (as + /// opposed to an initial allocation) is followed by exactly one of this + /// method or `memory_grow_failed`, with the same `kind` that was passed to + /// `memory_growing`, unless one of Wasmtime's own internal invariants is + /// violated and it panics in between. This is not called for a memory's + /// initial allocation or for shared memories. + /// + /// `current` and `desired` are the heap's old and new sizes in bytes, and + /// `kind` distinguishes a guest linear memory from an internal GC heap. + /// + /// This runs after the committed base pointer and length have been + /// published to the owning `VMContext` — and, for a GC heap, after the grown + /// memory and its new capacity have been handed back to the collector. + /// Unwinding out of this method therefore leaves the store and its instances + /// in a consistent, reusable state. + fn memory_grown(&mut self, _current: usize, _desired: usize, _kind: MemoryKind) {} /// Notifies the resource limiter that an instance's table has been /// requested to grow. @@ -185,16 +242,17 @@ pub trait ResourceLimiterAsync: Send { current: usize, desired: usize, maximum: Option, + kind: MemoryKind, ) -> Result; /// Identical to [`ResourceLimiter::memory_grow_failed`] - fn memory_grow_failed(&mut self, error: crate::Error) -> Result<()> { + fn memory_grow_failed(&mut self, error: crate::Error, _kind: MemoryKind) -> Result<()> { log::debug!("ignoring memory growth failure error: {error:?}"); Ok(()) } /// Identical to [`ResourceLimiter::memory_grown`]. - fn memory_grown(&mut self, _current: usize, _desired: usize) {} + fn memory_grown(&mut self, _current: usize, _desired: usize, _kind: MemoryKind) {} /// Asynchronous version of [`ResourceLimiter::table_growing`] async fn table_growing( @@ -350,6 +408,7 @@ impl ResourceLimiter for StoreLimits { _current: usize, desired: usize, maximum: Option, + _kind: MemoryKind, ) -> Result { let allow = match self.memory_size { Some(limit) if desired > limit => false, @@ -365,7 +424,7 @@ impl ResourceLimiter for StoreLimits { } } - fn memory_grow_failed(&mut self, error: crate::Error) -> Result<()> { + fn memory_grow_failed(&mut self, error: crate::Error, _kind: MemoryKind) -> Result<()> { if self.trap_on_grow_failure { Err(error.context("forcing a memory growth failure to be a trap")) } else { @@ -415,3 +474,609 @@ impl ResourceLimiter for StoreLimits { self.memories } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{Config, Engine, Instance, Module, Store}; + use alloc::vec::Vec; + + /// One limiter callback, recorded in the order it was observed. + #[derive(Debug, PartialEq, Eq, Clone, Copy)] + enum Event { + Growing(usize, usize, MemoryKind), + Grown(usize, usize, MemoryKind), + GrowFailed(MemoryKind), + } + + impl Event { + fn kind(&self) -> MemoryKind { + match self { + Event::Growing(_, _, k) | Event::Grown(_, _, k) | Event::GrowFailed(k) => *k, + } + } + } + + /// A limiter that records every callback and can be told to reject growth + /// or to panic from `memory_grown`. + struct Recorder { + events: Vec, + /// Kinds whose growth requests are rejected by `memory_growing`. + reject: Option, + /// If set, the *next* `memory_grown` for this kind panics after + /// recording itself. It fires once so that the store can be exercised + /// afterwards to prove it survived the unwind. + panic_on_grown: Option, + } + + impl Recorder { + fn new() -> Recorder { + Recorder { + events: Vec::new(), + reject: None, + panic_on_grown: None, + } + } + + #[cfg(feature = "gc")] + fn rejecting(kind: MemoryKind) -> Recorder { + Recorder { + reject: Some(kind), + ..Recorder::new() + } + } + + fn panicking_on_grown(kind: MemoryKind) -> Recorder { + Recorder { + panic_on_grown: Some(kind), + ..Recorder::new() + } + } + + fn of_kind(&self, kind: MemoryKind) -> Vec { + self.events + .iter() + .copied() + .filter(|e| e.kind() == kind) + .collect() + } + + /// Asserts the callback protocol holds: after discarding the leading + /// `initial_allocations` requests (initial allocations are deliberately + /// never followed by `memory_grown`), every permitted `memory_growing` + /// is immediately resolved by exactly one `memory_grown` or + /// `memory_grow_failed` carrying the same kind and sizes. + /// + /// A *rejected* growth resolves itself and gets no follow-up: the + /// limiter said no, so it never reserved anything that needs releasing. + /// + /// Note this deliberately rejects a `GrowFailed` with no preceding + /// `Growing`. Such a sequence is legal in general — a request the + /// memory's type cannot represent is refused before the limiter is + /// consulted, see `unrepresentable_growth_fails_without_a_request` — but + /// no test using this helper should be provoking that path, so seeing it + /// here means the test is not exercising what it claims to. + fn assert_growth_protocol(&self, initial_allocations: usize) { + let mut remaining_initial = initial_allocations; + let mut events = self.events.iter().copied().peekable(); + while let Some(event) = events.next() { + let Event::Growing(current, desired, kind) = event else { + panic!("unpaired {event:?} in {:?}", self.events); + }; + if remaining_initial > 0 { + remaining_initial -= 1; + continue; + } + if self.reject == Some(kind) { + continue; + } + match events.next() { + Some(Event::Grown(c, d, k)) => { + assert_eq!( + (c, d, k), + (current, desired, kind), + "mismatched resolution in {:?}", + self.events + ); + } + Some(Event::GrowFailed(k)) => assert_eq!( + k, kind, + "`memory_grow_failed` reported the wrong kind in {:?}", + self.events + ), + other => panic!( + "permitted growth {event:?} was resolved by {other:?} in {:?}", + self.events + ), + } + } + assert_eq!( + remaining_initial, 0, + "expected {initial_allocations} initial allocations in {:?}", + self.events + ); + } + } + + impl ResourceLimiter for Recorder { + fn memory_growing( + &mut self, + current: usize, + desired: usize, + _maximum: Option, + kind: MemoryKind, + ) -> Result { + self.events.push(Event::Growing(current, desired, kind)); + if self.reject == Some(kind) { + return Ok(false); + } + Ok(true) + } + + fn memory_grown(&mut self, current: usize, desired: usize, kind: MemoryKind) { + self.events.push(Event::Grown(current, desired, kind)); + if self.panic_on_grown == Some(kind) { + self.panic_on_grown = None; + panic!("memory_grown panicked on purpose"); + } + } + + fn memory_grow_failed(&mut self, _error: crate::Error, kind: MemoryKind) -> Result<()> { + self.events.push(Event::GrowFailed(kind)); + Ok(()) + } + + fn table_growing( + &mut self, + _current: usize, + _desired: usize, + _maximum: Option, + ) -> Result { + Ok(true) + } + } + + const MEM_WAT: &str = r#" + (module + (memory (export "m") 1) + (func (export "load") (param i32) (result i32) + local.get 0 + i32.load) + (func (export "store") (param i32) (param i32) + local.get 0 + local.get 1 + i32.store)) + "#; + + /// A config whose linear memories are backed by a virtual reservation large + /// enough for the growth these tests perform, so the base pointer never + /// moves. Set explicitly rather than relying on the default. + fn fixed_base_config() -> Config { + let mut config = Config::new(); + config.memory_reservation(16 << 20).memory_may_move(false); + config + } + + /// A config with no spare reservation, forcing growth to reallocate and move + /// the memory's base pointer. + fn moving_base_config() -> Config { + let mut config = Config::new(); + config + .memory_reservation(0) + .memory_reservation_for_growth(0) + .memory_guard_size(0) + .memory_may_move(true); + config + } + + /// A config with a GC heap small enough that a handful of allocations + /// force it to grow. + #[cfg(feature = "gc")] + fn gc_config() -> Config { + let mut config = Config::new(); + config.wasm_gc(true).gc_heap_reservation(1 << 16); + config + } + + const PAGE: usize = 64 * 1024; + /// An address that only exists once the memory has grown to two pages. + const GROWN_ADDR: i32 = PAGE as i32 + 128; + + /// GOL-424: a `memory_grown` callback that panics must not be able to leave + /// a `VMContext` holding a stale base pointer or length. After catching the + /// panic the memory reports its new size and compiled wasm can read and + /// write the newly committed region. + fn memory_grown_panic_is_unwind_safe(mut config: Config, expect_base_move: bool) { + let engine = Engine::new(&config.wasm_multi_memory(true)).unwrap(); + let module = Module::new(&engine, MEM_WAT).unwrap(); + + let mut store = Store::new( + &engine, + Recorder::panicking_on_grown(MemoryKind::LinearMemory), + ); + store.limiter(|r| r); + + let instance = Instance::new(&mut store, &module, &[]).unwrap(); + let memory = instance.get_memory(&mut store, "m").unwrap(); + let load = instance + .get_typed_func::(&mut store, "load") + .unwrap(); + let store_fn = instance + .get_typed_func::<(i32, i32), ()>(&mut store, "store") + .unwrap(); + + let base_before = memory.data_ptr(&store); + + let panicked = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let _ = memory.grow(&mut store, 1); + })) + .is_err(); + assert!(panicked, "expected the limiter's panic to propagate"); + + // The growth committed, so the memory must report the new size. + assert_eq!(memory.size(&store), 2); + assert_eq!(memory.data_size(&store), 2 * PAGE); + + let base_after = memory.data_ptr(&store); + if expect_base_move { + assert_ne!( + base_before, base_after, + "expected this configuration to relocate the memory on growth" + ); + } + + // The instance must still be usable, and crucially the compiled wasm + // must see the *new* base and bounds: this store traps if the VMContext + // still holds the pre-growth length, and corrupts freed memory if it + // still holds the pre-growth base. + store_fn + .call(&mut store, (GROWN_ADDR, 0x1234_5678)) + .unwrap(); + assert_eq!(load.call(&mut store, GROWN_ADDR).unwrap(), 0x1234_5678); + + // The host's view of the memory must agree with what wasm just wrote, + // which is only true if both are looking at the same allocation. + let data = memory.data(&store); + let addr = GROWN_ADDR as usize; + assert_eq!( + u32::from_le_bytes(data[addr..addr + 4].try_into().unwrap()), + 0x1234_5678 + ); + + // And the store is healthy enough to grow again. The limiter's panic is + // one-shot, so this growth runs its `memory_grown` normally. + assert_eq!(memory.grow(&mut store, 1).unwrap(), 2); + assert_eq!(memory.size(&store), 3); + assert_eq!(load.call(&mut store, GROWN_ADDR).unwrap(), 0x1234_5678); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn memory_grown_panic_is_unwind_safe_with_fixed_base() { + memory_grown_panic_is_unwind_safe(fixed_base_config(), false); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn memory_grown_panic_is_unwind_safe_with_moving_base() { + memory_grown_panic_is_unwind_safe(moving_base_config(), true); + } + + /// Ordinary linear-memory growth still reports `LinearMemory` and still + /// pairs each permitted growth with exactly one resolution. + #[test] + #[cfg_attr(miri, ignore)] + fn linear_memory_growth_callback_sequence() { + let engine = Engine::default(); + let module = Module::new(&engine, r#"(module (memory (export "m") 1 2))"#).unwrap(); + let mut store = Store::new(&engine, Recorder::new()); + store.limiter(|r| r); + + let instance = Instance::new(&mut store, &module, &[]).unwrap(); + let memory = instance.get_memory(&mut store, "m").unwrap(); + + assert_eq!(memory.grow(&mut store, 1).unwrap(), 1); + // Growing past the declared maximum is permitted by this limiter but + // fails in the allocator, which must report `memory_grow_failed`. + assert!(memory.grow(&mut store, 1).is_err()); + + let events = store.data().of_kind(MemoryKind::LinearMemory); + assert_eq!( + events, + [ + Event::Growing(0, PAGE, MemoryKind::LinearMemory), + Event::Growing(PAGE, 2 * PAGE, MemoryKind::LinearMemory), + Event::Grown(PAGE, 2 * PAGE, MemoryKind::LinearMemory), + Event::Growing(2 * PAGE, 3 * PAGE, MemoryKind::LinearMemory), + Event::GrowFailed(MemoryKind::LinearMemory), + ] + ); + store.data().assert_growth_protocol(1); + assert!(store.data().of_kind(MemoryKind::GcHeap).is_empty()); + } + + /// A growth the memory's own type cannot represent is refused before the + /// limiter is ever asked, so `memory_grow_failed` arrives with no preceding + /// `memory_growing`. This is the one sequence `memory_grow_failed`'s docs + /// call out as unpaired; assert it really happens so the docs and + /// `assert_growth_protocol`'s strictness stay honest. + #[test] + #[cfg_attr(miri, ignore)] + fn unrepresentable_growth_fails_without_a_request() { + // A 1-byte-page memory cannot address the whole 32-bit range, so growth + // towards it is rejected by the memory type itself. + let mut config = Config::new(); + config.wasm_custom_page_sizes(true); + let engine = Engine::new(&config).unwrap(); + let module = + Module::new(&engine, r#"(module (memory (export "m") 1 (pagesize 1)))"#).unwrap(); + + let mut store = Store::new(&engine, Recorder::new()); + store.limiter(|r| r); + + let instance = Instance::new(&mut store, &module, &[]).unwrap(); + let memory = instance.get_memory(&mut store, "m").unwrap(); + let before = store.data().events.len(); + + assert!(memory.grow(&mut store, 1 << 32).is_err()); + + assert_eq!( + &store.data().events[before..], + [Event::GrowFailed(MemoryKind::LinearMemory)], + "expected a lone failure with no growth request; saw {:?}", + store.data().events, + ); + } + + /// Growing by zero pages is a no-op for the limiter: nothing was requested, + /// so nothing is reported. + #[test] + #[cfg_attr(miri, ignore)] + fn zero_page_growth_is_not_reported() { + let engine = Engine::default(); + let module = Module::new(&engine, r#"(module (memory (export "m") 1 2))"#).unwrap(); + let mut store = Store::new(&engine, Recorder::new()); + store.limiter(|r| r); + + let instance = Instance::new(&mut store, &module, &[]).unwrap(); + let memory = instance.get_memory(&mut store, "m").unwrap(); + let before = store.data().events.len(); + + assert_eq!(memory.grow(&mut store, 0).unwrap(), 1); + assert_eq!(store.data().events.len(), before); + } + + /// GOL-425: a GC heap that grows successfully must resolve the + /// `memory_growing` that permitted it, and must be labelled `GcHeap` rather + /// than passed off as guest linear memory. + #[cfg(feature = "gc")] + #[test] + #[cfg_attr(miri, ignore)] + fn gc_heap_growth_reports_gc_heap_kind() { + use crate::ExternRef; + + let engine = Engine::new(&gc_config()).unwrap(); + + let mut store = Store::new(&engine, Recorder::new()); + store.limiter(|r| r); + + // Keep allocating rooted GC objects until the heap has to grow. + let mut roots = Vec::new(); + for i in 0..10_000u32 { + roots.push(ExternRef::new(&mut store, i).unwrap()); + if store + .data() + .events + .iter() + .any(|e| matches!(e, Event::Grown(_, _, MemoryKind::GcHeap))) + { + break; + } + } + + let events = store.data().of_kind(MemoryKind::GcHeap); + assert!( + events.iter().any(|e| matches!(e, Event::Grown(..))) + && events.iter().any(|e| matches!(e, Event::Growing(..))), + "expected the GC heap to grow at least once; saw {events:?}", + ); + for event in &events { + if let Event::Grown(current, desired, _) = event { + assert!(desired > current, "reported a non-growth: {event:?}"); + } + } + store.data().assert_growth_protocol(1); + + // GC heap capacity must never be attributed to guest linear memory. + assert!( + !store + .data() + .events + .iter() + .any(|e| matches!(e, Event::Grown(_, _, MemoryKind::LinearMemory))), + "GC heap growth must not be reported as linear-memory growth", + ); + } + + /// GOL-424 for the GC heap: a `memory_grown` that panics unwinds through + /// `TakenGcHeap::drop`, which is what hands the grown memory and its size + /// delta back to the collector. If the notification ran before that delta + /// was computed the collector would be told the heap grew by zero bytes. + /// The deferred reference-counting collector believes that delta — it feeds + /// it straight to `FreeList::add_capacity` — so it would never use the new + /// capacity and would have to grow all over again on the next allocation. + /// (The copying and null collectors recompute from the memory's size, so + /// they mask this; hence the collector is pinned rather than defaulted.) + #[cfg(feature = "gc-drc")] + #[test] + #[cfg_attr(miri, ignore)] + fn gc_heap_grown_panic_is_unwind_safe() { + use crate::{Collector, ExternRef}; + + let mut config = Config::new(); + config + .wasm_gc(true) + .collector(Collector::DeferredReferenceCounting) + .gc_heap_reservation(1 << 16); + let engine = Engine::new(&config).unwrap(); + + let mut store = Store::new(&engine, Recorder::panicking_on_grown(MemoryKind::GcHeap)); + store.limiter(|r| r); + + // Allocate until the one-shot panic fires out of the first GC heap + // growth. + let mut roots = Vec::new(); + let mut panicked = false; + for i in 0..10_000u32 { + let caught = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + ExternRef::new(&mut store, i) + })); + match caught { + Ok(Ok(r)) => roots.push(r), + Ok(Err(e)) => panic!("GC allocation failed before the heap grew: {e:?}"), + Err(_) => { + panicked = true; + break; + } + } + } + assert!(panicked, "expected the limiter's panic to propagate"); + + let growths = |store: &Store| { + store + .data() + .events + .iter() + .filter(|e| matches!(e, Event::Grown(_, _, MemoryKind::GcHeap))) + .count() + }; + let growths_at_panic = growths(&store); + assert_eq!( + growths_at_panic, + 1, + "expected the panic to come from the first GC heap growth; saw {:?}", + store.data().events, + ); + + // The growth committed, so the collector must have been handed the new + // capacity before the unwind. If it was not, its free list gained zero + // bytes and the very next allocation has to grow the heap all over + // again — so allocating well within the capacity just added must not + // trigger another growth. + for i in 0..50u32 { + roots.push( + ExternRef::new(&mut store, i) + .expect("store must still allocate after the caught panic"), + ); + } + assert_eq!( + growths(&store), + growths_at_panic, + "the capacity from the growth that panicked was lost: the collector \ + had to grow again immediately; saw {:?}", + store.data().events, + ); + } + + /// GOL-425: a GC heap growth the limiter *permits* but that then fails in + /// the allocator must resolve with `memory_grow_failed` carrying `GcHeap`, + /// not `LinearMemory`. Without the kind on the failure callback an embedder + /// could not tell which reservation to release. + #[cfg(feature = "gc")] + #[test] + #[cfg_attr(miri, ignore)] + fn failed_gc_heap_growth_reports_gc_heap_kind() { + use crate::ExternRef; + + // Pin the heap to a single reservation it may not move out of, so + // growing beyond that capacity is permitted by the limiter and then + // fails in the allocator. + let mut config = Config::new(); + config + .wasm_gc(true) + .gc_heap_reservation(1 << 16) + .gc_heap_reservation_for_growth(0) + .gc_heap_may_move(false); + let engine = Engine::new(&config).unwrap(); + + let mut store = Store::new(&engine, Recorder::new()); + store.limiter(|r| r); + + let mut roots = Vec::new(); + for i in 0..100_000u32 { + match ExternRef::new(&mut store, i) { + Ok(r) => roots.push(r), + Err(_) => break, + } + if store + .data() + .events + .iter() + .any(|e| matches!(e, Event::GrowFailed(_))) + { + break; + } + } + + let events = store.data().of_kind(MemoryKind::GcHeap); + assert!( + events.iter().any(|e| matches!(e, Event::GrowFailed(..))), + "expected a permitted GC heap growth to fail in the allocator; saw {events:?}", + ); + assert!( + !store + .data() + .events + .iter() + .any(|e| matches!(e, Event::GrowFailed(MemoryKind::LinearMemory))), + "a GC heap growth failure must not be reported as a linear-memory \ + failure; saw {:?}", + store.data().events, + ); + store.data().assert_growth_protocol(1); + } + + /// A limiter that refuses GC heap growth sees the request and no + /// resolution, because a refusal resolves itself. It must never see a + /// `Grown` for a growth it rejected. + #[cfg(feature = "gc")] + #[test] + #[cfg_attr(miri, ignore)] + fn rejected_gc_heap_growth_reports_no_commit() { + use crate::ExternRef; + + let engine = Engine::new(&gc_config()).unwrap(); + + let mut store = Store::new(&engine, Recorder::rejecting(MemoryKind::GcHeap)); + store.limiter(|r| r); + + // Allocate until the heap is exhausted; growth is refused, so this must + // eventually fail rather than silently succeed. + let mut roots = Vec::new(); + let mut hit_limit = false; + for i in 0..10_000u32 { + match ExternRef::new(&mut store, i) { + Ok(r) => roots.push(r), + Err(_) => { + hit_limit = true; + break; + } + } + } + assert!( + hit_limit, + "expected GC allocation to fail once growth was refused" + ); + + let events = store.data().of_kind(MemoryKind::GcHeap); + assert!( + events.iter().any(|e| matches!(e, Event::Growing(..))), + "expected at least one GC heap growth request; saw {events:?}", + ); + assert!( + !events.iter().any(|e| matches!(e, Event::Grown(..))), + "a rejected growth must never be reported as committed: {events:?}", + ); + store.data().assert_growth_protocol(1); + } +} diff --git a/crates/wasmtime/src/runtime/memory.rs b/crates/wasmtime/src/runtime/memory.rs index 8f0155d8d342..25e41060dbb5 100644 --- a/crates/wasmtime/src/runtime/memory.rs +++ b/crates/wasmtime/src/runtime/memory.rs @@ -1118,7 +1118,7 @@ mod tests { use alloc::vec::Vec; #[derive(Default)] - struct SuccessfulGrowths(Vec<(usize, usize)>); + struct SuccessfulGrowths(Vec<(usize, usize, MemoryKind)>); impl ResourceLimiter for SuccessfulGrowths { fn memory_growing( @@ -1126,12 +1126,13 @@ mod tests { _current: usize, _desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { Ok(true) } - fn memory_grown(&mut self, current: usize, desired: usize) { - self.0.push((current, desired)); + fn memory_grown(&mut self, current: usize, desired: usize, kind: MemoryKind) { + self.0.push((current, desired, kind)); } fn table_growing( @@ -1155,10 +1156,16 @@ mod tests { let memory = instance.get_memory(&mut store, "m").unwrap(); assert_eq!(memory.grow(&mut store, 1)?, 1); - assert_eq!(store.data().0, [(65536, 2 * 65536)]); + assert_eq!( + store.data().0, + [(65536, 2 * 65536, MemoryKind::LinearMemory)] + ); assert!(memory.grow(&mut store, 1).is_err()); - assert_eq!(store.data().0, [(65536, 2 * 65536)]); + assert_eq!( + store.data().0, + [(65536, 2 * 65536, MemoryKind::LinearMemory)] + ); Ok(()) } diff --git a/crates/wasmtime/src/runtime/store.rs b/crates/wasmtime/src/runtime/store.rs index a7b2a3a41c7f..04f58f93d58d 100644 --- a/crates/wasmtime/src/runtime/store.rs +++ b/crates/wasmtime/src/runtime/store.rs @@ -330,27 +330,32 @@ impl StoreResourceLimiter<'_> { current: usize, desired: usize, maximum: Option, + kind: crate::MemoryKind, ) -> Result { match self { - Self::Sync(s) => s.memory_growing(current, desired, maximum), + Self::Sync(s) => s.memory_growing(current, desired, maximum, kind), #[cfg(feature = "async")] - Self::Async(s) => s.memory_growing(current, desired, maximum).await, + Self::Async(s) => s.memory_growing(current, desired, maximum, kind).await, } } - pub(crate) fn memory_grow_failed(&mut self, error: crate::Error) -> Result<()> { + pub(crate) fn memory_grow_failed( + &mut self, + error: crate::Error, + kind: crate::MemoryKind, + ) -> Result<()> { match self { - Self::Sync(s) => s.memory_grow_failed(error), + Self::Sync(s) => s.memory_grow_failed(error, kind), #[cfg(feature = "async")] - Self::Async(s) => s.memory_grow_failed(error), + Self::Async(s) => s.memory_grow_failed(error, kind), } } - pub(crate) fn memory_grown(&mut self, current: usize, desired: usize) { + pub(crate) fn memory_grown(&mut self, current: usize, desired: usize, kind: crate::MemoryKind) { match self { - Self::Sync(s) => s.memory_grown(current, desired), + Self::Sync(s) => s.memory_grown(current, desired, kind), #[cfg(feature = "async")] - Self::Async(s) => s.memory_grown(current, desired), + Self::Async(s) => s.memory_grown(current, desired, kind), } } diff --git a/crates/wasmtime/src/runtime/store/gc.rs b/crates/wasmtime/src/runtime/store/gc.rs index 63fa437b829f..0beb6f299b35 100644 --- a/crates/wasmtime/src/runtime/store/gc.rs +++ b/crates/wasmtime/src/runtime/store/gc.rs @@ -224,7 +224,7 @@ impl StoreOpaque { /// Returns an error if growing the GC heap fails. pub(crate) async fn grow_gc_heap( &mut self, - limiter: Option<&mut StoreResourceLimiter<'_>>, + mut limiter: Option<&mut StoreResourceLimiter<'_>>, bytes_needed: u64, asyncness: Asyncness, ) -> Result<()> { @@ -289,13 +289,13 @@ impl StoreOpaque { // Safety: we pair growing the GC heap with updating its associated // `VMMemoryDefinition` in the `VMStoreContext` immediately // afterwards. - unsafe { + let (old_byte_size, new_byte_size) = unsafe { heap.memory - .grow(delta_pages_for_alloc, limiter) + .grow(delta_pages_for_alloc, limiter.as_deref_mut()) .await .context(GcHeapGrowthFailed)? - .ok_or(GcHeapGrowthFailed)?; - } + .ok_or(GcHeapGrowthFailed)? + }; *heap.store.vm_store_context.gc_heap.get_mut() = heap.memory.vmmemory(); let new_size_in_bytes = u64::try_from(heap.memory.byte_size())?; @@ -311,6 +311,25 @@ impl StoreOpaque { " -> grew GC heap by {:#x} bytes: new size is {new_size_in_bytes:#x} bytes", heap.delta_bytes_grown ); + + // Put the grown memory, and the size delta the collector needs in order + // to use the new capacity, back into the `GcStore` before notifying the + // embedder. Running the notification while `heap` is still alive would + // hand control to embedder code at a point where the store's GC heap has + // no memory at all and its free list has not been extended. + drop(heap); + + // Resolve the `memory_growing` that permitted this growth. See + // `ResourceLimiter::memory_grown` for the contract; the ordering above + // is what upholds its unwind-safety guarantee. This reports the kind + // directly rather than going through `Memory::notify_grown` because the + // memory now lives inside the `GcStore`. Growth here is always nonzero: + // a `bytes_needed` of zero returned early and the delta was asserted + // above. + if let Some(limiter) = limiter { + limiter.memory_grown(old_byte_size, new_byte_size, crate::MemoryKind::GcHeap); + } + return Ok(()); struct TakenGcHeap<'a> { diff --git a/crates/wasmtime/src/runtime/vm/instance.rs b/crates/wasmtime/src/runtime/vm/instance.rs index 06ba7e44551c..38eac4b86447 100644 --- a/crates/wasmtime/src/runtime/vm/instance.rs +++ b/crates/wasmtime/src/runtime/vm/instance.rs @@ -787,7 +787,7 @@ impl Instance { /// successful. pub(crate) async fn memory_grow( mut self: Pin<&mut Self>, - limiter: Option<&mut StoreResourceLimiter<'_>>, + mut limiter: Option<&mut StoreResourceLimiter<'_>>, idx: DefinedMemoryIndex, delta: u64, ) -> Result, Error> { @@ -796,16 +796,28 @@ impl Instance { // SAFETY: this is the safe wrapper around `Memory::grow` because it // automatically updates the `VMMemoryDefinition` in this instance after // a growth operation below. - let result = unsafe { memory.grow(delta, limiter).await }; + let result = unsafe { memory.grow(delta, limiter.as_deref_mut()).await }; // Update the state used by a non-shared Wasm memory in case the base // pointer and/or the length changed. if memory.as_shared_memory().is_none() { let vmmemory = memory.vmmemory(); - self.set_memory(idx, vmmemory); + self.as_mut().set_memory(idx, vmmemory); } - result + // Only now that the `VMContext` holds the committed base pointer and + // length is it safe to hand control to the embedder's post-growth + // notification. It may panic, and if that panic is caught this instance + // must still be consistent and reusable. + match result { + Ok(Some((old, new))) => { + let memory = &self.as_mut().memories_mut()[idx].1; + memory.notify_grown(limiter, old, new); + Ok(Some(old)) + } + Ok(None) => Ok(None), + Err(e) => Err(e), + } } /// Performs a grow operation on the `table_index` specified using `grow`. diff --git a/crates/wasmtime/src/runtime/vm/memory.rs b/crates/wasmtime/src/runtime/vm/memory.rs index 5077182c4298..50bc0f14832b 100644 --- a/crates/wasmtime/src/runtime/vm/memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory.rs @@ -246,7 +246,7 @@ impl Memory { limiter: Option<&mut StoreResourceLimiter<'_>>, kind: MemoryKind, ) -> Result { - let (minimum, maximum) = Self::limit_new(ty, limiter).await?; + let (minimum, maximum) = Self::limit_new(ty, kind, limiter).await?; let tunables = engine.tunables(); let memory_tunables = MemoryTunables::new(tunables, kind); let allocation = creator.new_memory(ty, &memory_tunables, minimum, maximum)?; @@ -270,7 +270,7 @@ impl Memory { memory_image: MemoryImageSlot, limiter: Option<&mut StoreResourceLimiter<'_>>, ) -> Result { - let (minimum, maximum) = Self::limit_new(ty, limiter).await?; + let (minimum, maximum) = Self::limit_new(ty, kind, limiter).await?; let pooled_memory = StaticMemory::new(base, base_capacity, minimum, maximum)?; let allocation = try_new::>(pooled_memory)?; @@ -299,6 +299,7 @@ impl Memory { /// size) of the memory, all in bytes. pub(crate) async fn limit_new( ty: &wasmtime_environ::Memory, + kind: MemoryKind, limiter: Option<&mut StoreResourceLimiter<'_>>, ) -> Result<(usize, Option)> { let page_size = usize::try_from(ty.page_size()).unwrap(); @@ -344,7 +345,7 @@ impl Memory { // now the expected uses of limiter means that's ok. if let Some(limiter) = limiter { if !limiter - .memory_growing(0, minimum.unwrap_or(absolute_max), maximum) + .memory_growing(0, minimum.unwrap_or(absolute_max), maximum, kind.into()) .await? { bail!( @@ -399,8 +400,14 @@ impl Memory { /// Grow memory by the specified amount of wasm pages. /// /// Returns `None` if memory can't be grown by the specified amount - /// of wasm pages. Returns `Some` with the old size of memory, in bytes, on - /// successful growth. + /// of wasm pages. Returns `Some` with the old and new sizes of memory, in + /// bytes, on successful growth. + /// + /// Note that this does *not* emit the limiter's `memory_grown` + /// notification: at the point this returns the backing allocation has + /// committed but the owning `VMContext` has not yet been refreshed. Callers + /// are responsible for publishing the new [`VMMemoryDefinition`] and then + /// calling [`Self::notify_grown`]. /// /// # Safety /// @@ -417,14 +424,35 @@ impl Memory { &mut self, delta_pages: u64, limiter: Option<&mut StoreResourceLimiter<'_>>, - ) -> Result, Error> { - let result = match self { - Memory::Local(mem) => mem.grow(delta_pages, limiter).await?, - Memory::Shared(mem) => mem.grow(delta_pages)?, - }; - match result { - Some((old, _new)) => Ok(Some(old)), - None => Ok(None), + ) -> Result, Error> { + match self { + Memory::Local(mem) => mem.grow(delta_pages, limiter).await, + Memory::Shared(mem) => mem.grow(delta_pages), + } + } + + /// Emits the limiter's `memory_grown` notification for a growth that + /// [`Self::grow`] just reported as successful. + /// + /// This must be called only *after* the refreshed [`VMMemoryDefinition`] + /// has been published to whatever owns this memory, so that embedder code + /// which panics cannot leave a `VMContext` pointing at a stale base or + /// length. + /// + /// The notification is suppressed for shared memories, which have no + /// associated limiter, and for growth of zero bytes. + pub fn notify_grown( + &self, + limiter: Option<&mut StoreResourceLimiter<'_>>, + old_byte_size: usize, + new_byte_size: usize, + ) { + let Memory::Local(mem) = self else { return }; + if new_byte_size <= old_byte_size { + return; + } + if let Some(limiter) = limiter { + limiter.memory_grown(old_byte_size, new_byte_size, mem.limiter_kind()); } } @@ -600,6 +628,14 @@ impl LocalMemory { &self.ty } + /// This memory's kind as the embedder-facing [`crate::MemoryKind`]. + /// + /// Note that `MemoryKind` in this module refers to the `wasmtime_environ` + /// type; this converts to the public one the limiter callbacks take. + fn limiter_kind(&self) -> crate::MemoryKind { + self.kind.into() + } + /// Grows a memory by `delta_pages`. /// /// This performs the necessary checks on the growth before delegating to @@ -645,7 +681,7 @@ impl LocalMemory { 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)?; + limiter.memory_grow_failed(err, self.limiter_kind())?; } return Ok(None); } @@ -653,7 +689,7 @@ impl LocalMemory { // Store limiter gets first chance to reject memory_growing. if let Some(limiter) = &mut limiter { if !limiter - .memory_growing(old_byte_size, new_byte_size, maximum) + .memory_growing(old_byte_size, new_byte_size, maximum, self.limiter_kind()) .await? { return Ok(None); @@ -714,12 +750,13 @@ impl LocalMemory { assert_eq!(base_ptr_before, self.alloc.base().as_mut_ptr()); } - if matches!(self.kind, MemoryKind::LinearMemory) - && let Some(limiter) = limiter - { - limiter.memory_grown(old_byte_size, new_byte_size); - } - + // NB: the `memory_grown` notification is deliberately *not* + // emitted here. The backing allocation has committed but the + // owning `VMContext` still holds the pre-growth base pointer + // and length, so running embedder code at this point would let + // a panic escape with compiled wasm observing stale bounds or a + // freed base. Callers emit the notification once they have + // published the refreshed `VMMemoryDefinition`. Ok(Some((old_byte_size, new_byte_size))) } Err(e) => { @@ -728,7 +765,7 @@ impl LocalMemory { // dropped // (https://github.com/bytecodealliance/wasmtime/issues/4240). if let Some(limiter) = limiter { - limiter.memory_grow_failed(e)?; + limiter.memory_grow_failed(e, self.limiter_kind())?; } Ok(None) } diff --git a/crates/wasmtime/src/runtime/vm/memory/shared_memory.rs b/crates/wasmtime/src/runtime/vm/memory/shared_memory.rs index 1bff2586167c..21d430eb5c96 100644 --- a/crates/wasmtime/src/runtime/vm/memory/shared_memory.rs +++ b/crates/wasmtime/src/runtime/vm/memory/shared_memory.rs @@ -43,7 +43,11 @@ impl SharedMemory { ); // Note that without a limiter being passed to `limit_new` this // `assert_ready` should never panic. - let (minimum_bytes, maximum_bytes) = vm::assert_ready(Memory::limit_new(ty, None))?; + let (minimum_bytes, maximum_bytes) = vm::assert_ready(Memory::limit_new( + ty, + wasmtime_environ::MemoryKind::LinearMemory, + None, + ))?; let mmap_memory = MmapMemory::new(ty, &memory_tunables, minimum_bytes, maximum_bytes)?; let boxed: Box = try_new::>(mmap_memory)?; diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 01da0d1d4e20..712346165775 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -110,6 +110,7 @@ async fn test_limits_async() -> Result<()> { _current: usize, desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { Ok(desired <= self.memory_size) } @@ -401,6 +402,7 @@ impl ResourceLimiter for MemoryContext { current: usize, desired: usize, maximum: Option, + _kind: MemoryKind, ) -> Result { // Check if the desired exceeds a maximum (either from Wasm or from the host) assert!(desired < maximum.unwrap_or(usize::MAX)); @@ -513,6 +515,7 @@ impl ResourceLimiterAsync for MemoryContext { current: usize, desired: usize, maximum: Option, + _kind: MemoryKind, ) -> Result { // Show we can await in this async context: tokio::time::sleep(std::time::Duration::from_millis(1)).await; @@ -633,6 +636,7 @@ impl ResourceLimiter for TableContext { _current: usize, _desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { Ok(true) } @@ -703,7 +707,7 @@ struct FailureDetector { memory_current: usize, memory_desired: usize, /// Arguments of most recent call to memory_grown - memory_grown: Option<(usize, usize)>, + memory_grown: Option<(usize, usize, MemoryKind)>, /// Display impl of most recent call to memory_grow_failed memory_error: Option, /// Arguments of most recent call to table_growing @@ -719,17 +723,18 @@ impl ResourceLimiter for FailureDetector { current: usize, desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { self.memory_current = current; self.memory_desired = desired; Ok(true) } - fn memory_grow_failed(&mut self, err: wasmtime::Error) -> Result<()> { + fn memory_grow_failed(&mut self, err: wasmtime::Error, _kind: MemoryKind) -> Result<()> { self.memory_error = Some(err.to_string()); Ok(()) } - fn memory_grown(&mut self, current: usize, desired: usize) { - self.memory_grown = Some((current, desired)); + fn memory_grown(&mut self, current: usize, desired: usize, kind: MemoryKind) { + self.memory_grown = Some((current, desired, kind)); } fn table_growing( &mut self, @@ -777,7 +782,10 @@ fn custom_limiter_detect_grow_failure() -> Result<()> { assert!(store.data().memory_error.is_none()); assert_eq!(store.data().memory_current, 0); assert_eq!(store.data().memory_desired, 10 * 64 * 1024); - assert_eq!(store.data().memory_grown, Some((0, 10 * 64 * 1024))); + assert_eq!( + store.data().memory_grown, + Some((0, 10 * 64 * 1024, MemoryKind::LinearMemory)) + ); // Grow past the static limit set by ModuleLimits. // The ResourceLimiter will permit this, but the grow will fail. @@ -792,7 +800,10 @@ fn custom_limiter_detect_grow_failure() -> Result<()> { store.data().memory_error.as_ref().unwrap(), "Memory maximum size exceeded" ); - assert_eq!(store.data().memory_grown, Some((0, 10 * 64 * 1024))); + assert_eq!( + store.data().memory_grown, + Some((0, 10 * 64 * 1024, MemoryKind::LinearMemory)) + ); let table = instance.get_table(&mut store, "t").unwrap(); // Grow the table 10 elements @@ -831,6 +842,7 @@ impl ResourceLimiterAsync for FailureDetector { current: usize, desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { // Show we can await in this async context: tokio::time::sleep(std::time::Duration::from_millis(1)).await; @@ -838,12 +850,12 @@ impl ResourceLimiterAsync for FailureDetector { self.memory_desired = desired; Ok(true) } - fn memory_grow_failed(&mut self, err: wasmtime::Error) -> Result<()> { + fn memory_grow_failed(&mut self, err: wasmtime::Error, _kind: MemoryKind) -> Result<()> { self.memory_error = Some(err.to_string()); Ok(()) } - fn memory_grown(&mut self, current: usize, desired: usize) { - self.memory_grown = Some((current, desired)); + fn memory_grown(&mut self, current: usize, desired: usize, kind: MemoryKind) { + self.memory_grown = Some((current, desired, kind)); } async fn table_growing( @@ -951,6 +963,7 @@ impl ResourceLimiter for Panic { _current: usize, _desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { panic!("resource limiter memory growing"); } @@ -970,6 +983,7 @@ impl ResourceLimiterAsync for Panic { _current: usize, _desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { panic!("async resource limiter memory growing"); } diff --git a/tests/all/memory.rs b/tests/all/memory.rs index f321f5d98c36..e5995beeeb3f 100644 --- a/tests/all/memory.rs +++ b/tests/all/memory.rs @@ -385,6 +385,7 @@ fn massive_64_bit_still_limited() -> Result<()> { _current: usize, _request: usize, _max: Option, + _kind: MemoryKind, ) -> Result { self.hit = true; Ok(true) diff --git a/tests/all/missing_async.rs b/tests/all/missing_async.rs index 878458f5fc88..01fc63313af5 100644 --- a/tests/all/missing_async.rs +++ b/tests/all/missing_async.rs @@ -18,6 +18,7 @@ impl ResourceLimiterAsync for MyAsyncLimiter { _current: usize, _desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { Ok(true) } diff --git a/tests/all/pooling_allocator.rs b/tests/all/pooling_allocator.rs index fc19983934ca..f85f594dc1a8 100644 --- a/tests/all/pooling_allocator.rs +++ b/tests/all/pooling_allocator.rs @@ -523,7 +523,13 @@ fn drop_externref_global_during_module_init() -> Result<()> { struct Limiter; impl ResourceLimiter for Limiter { - fn memory_growing(&mut self, _: usize, _: usize, _: Option) -> Result { + fn memory_growing( + &mut self, + _: usize, + _: usize, + _: Option, + _: MemoryKind, + ) -> Result { Ok(false) } @@ -1473,7 +1479,13 @@ fn memory_reset_if_instantiation_fails() -> Result<()> { struct Limiter; impl ResourceLimiter for Limiter { - fn memory_growing(&mut self, _: usize, _: usize, _: Option) -> Result { + fn memory_growing( + &mut self, + _: usize, + _: usize, + _: Option, + _: MemoryKind, + ) -> Result { Ok(false) } diff --git a/tests/rlimited-memory.rs b/tests/rlimited-memory.rs index 54d2a44d9d07..a9fa2489903a 100644 --- a/tests/rlimited-memory.rs +++ b/tests/rlimited-memory.rs @@ -17,12 +17,13 @@ impl ResourceLimiter for MemoryGrowFailureDetector { current: usize, desired: usize, _maximum: Option, + _kind: MemoryKind, ) -> Result { self.current = current; self.desired = desired; Ok(true) } - fn memory_grow_failed(&mut self, err: wasmtime::Error) -> Result<()> { + fn memory_grow_failed(&mut self, err: wasmtime::Error, _kind: MemoryKind) -> Result<()> { self.error = Some(err.to_string()); Ok(()) }