Do not leak GC objects in Array::new on type-mismatch errors

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

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(())
+}