From f0b0b20913d8a2d5d552642e9447eea8eb438d5a Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 6 Oct 2026 15:16:47 -0700 Subject: [PATCH] Do not leak GC objects in `Array::new` on type-mismatch errors Fixes #14565 --- .../src/runtime/gc/enabled/arrayref.rs | 27 +++++++------- tests/all/arrays.rs | 35 +++++++++++++++++++ 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/crates/wasmtime/src/runtime/gc/enabled/arrayref.rs b/crates/wasmtime/src/runtime/gc/enabled/arrayref.rs index 4fca492dd926..0fbdd061497d 100644 --- a/crates/wasmtime/src/runtime/gc/enabled/arrayref.rs +++ b/crates/wasmtime/src/runtime/gc/enabled/arrayref.rs @@ -407,18 +407,19 @@ impl ArrayRef { .context("unrecoverable error when allocating new `arrayref`")? .map_err(|n| GcHeapOutOfMemory::new((), n))?; - // Type check the elements against the element type. - for elem in elems.clone() { - elem.ensure_matches_ty(store, allocator.ty.element_type().unpack()) - .context("element type mismatch")?; - } - // From this point on, if we get any errors, then the array is not // fully initialized, so we need to eagerly deallocate it before the // next GC where the collector might try to interpret one of the // uninitialized fields as a GC reference. let mut store = AutoAssertNoGc::new(store); match (|| { + // Type check the elements here, rather than before allocating, so + // that oversized arrays fail without visiting every element. + for elem in elems.clone() { + elem.ensure_matches_ty(&store, allocator.ty.element_type().unpack()) + .context("element type mismatch")?; + } + let elem_ty = allocator.ty.element_type(); for (i, elem) in elems.enumerate() { let i = u32::try_from(i)?; @@ -679,15 +680,15 @@ impl ArrayRef { .context("unrecoverable error when allocating new `arrayref`")? .map_err(|n| GcHeapOutOfMemory::new((), n))?; - let mut store = AutoAssertNoGc::new(store); - let data = store - .require_gc_store_mut()? - .gc_object_data(arrayref.as_gc_ref())?; - let copied = data.copy_from_slice(layout.base_size, elems); - // If the copy failed then the array is not fully initialized, so we // must eagerly deallocate it before the next GC. - match copied { + let mut store = AutoAssertNoGc::new(store); + match (|| { + store + .require_gc_store_mut()? + .gc_object_data(arrayref.as_gc_ref())? + .copy_from_slice(layout.base_size, elems) + })() { Ok(()) => Ok(Rooted::new(&mut store, arrayref.into())), Err(e) => { store diff --git a/tests/all/arrays.rs b/tests/all/arrays.rs index 4d37a0d0e005..b5fd61f340fe 100644 --- a/tests/all/arrays.rs +++ b/tests/all/arrays.rs @@ -1045,3 +1045,38 @@ fn host_arrayref_has_trace_info_for_gc() -> Result<()> { Ok(()) } + +#[test] +#[cfg_attr(miri, ignore)] +fn array_new_type_mismatch_does_not_leak() -> Result<()> { + for collector in [Collector::Copying, Collector::DeferredReferenceCounting] { + println!("Using GC collector: {collector:?}"); + + let mut config = Config::new(); + config.wasm_gc(true); + config.collector(collector); + config.gc_heap_may_move(false); + config.gc_heap_reservation(64 << 10); + config.gc_heap_reservation_for_growth(0); + let engine = Engine::new(&config)?; + let mut store = Store::new(&engine, ()); + + let array_ty = ArrayType::new( + &engine, + FieldType::new(Mutability::Var, ValType::I32.into()), + ); + let pre = ArrayRefPre::new(&mut store, array_ty); + let bad_elems = vec![Val::I64(0); 1000]; + + for _ in 0..100 { + let err = ArrayRef::new_fixed(&mut store, &pre, &bad_elems).unwrap_err(); + err.assert_contains("element type mismatch"); + let err = ArrayRef::new(&mut store, &pre, &Val::I64(0), 1000).unwrap_err(); + err.assert_contains("element type mismatch"); + + let mut scope = RootScope::new(&mut store); + ArrayRef::new(&mut scope, &pre, &Val::I32(0), 1000)?; + } + } + Ok(()) +}