Stream: git-wasmtime

Topic: wasmtime / PR #10435 Fix a bug with rec groups in the typ...

Wasmtime GitHub notifications bot (Mar 20 2025 at 19:07):

fitzgen requested alexcrichton for a review on PR #10435.

Wasmtime GitHub notifications bot (Mar 20 2025 at 19:07):

fitzgen requested wasmtime-core-reviewers for a review on PR #10435.

Wasmtime GitHub notifications bot (Mar 20 2025 at 19:07):

fitzgen opened PR #10435 from fitzgen:rec-group-registration-fuzz-bug to bytecodealliance:main:

And add a bunch more assertions and clean up the trace logging output for the
types registry.

The bug is that TypeRegistryInner::register_rec_group mistakenly assumed that
a rec group's types would be assigned a contiguous range of
VMSharedTypeIndexes, computing intra-group type references' indices via adding
the rec-group-index to the group's first type's VMSharedTypeIndex. However,
this is invalid because the underlying slab maintains a free list and will reuse
slots after they've been deallocated, leading to non-contiguous
VMSharedTypeIndexes for the types within a rec group. This bug only affects
modules using the GC proposal.

The fix is to reserve the indices for the rec group's types up front, before
registering each type, and store them in a boxed slice. We can index into this
boxed slice to go from a rec-group-index to VMSharedTypeIndex. Note that this
allocation is fine because we need, and were already constructing, this exact
boxed slice of type indices just below anyways.

Reserving VMSharedTypeIndexes up front does mean that the slab now contains
Option<Arc<Type>> instead of plain Arc<Type> so that we have a sentinel
value (None) to insert into the slab (so we can reserve a VMSharedTypeIndex)
during rec group registration, but before we have actually registered that
particular type. This leads to a sprinkle of unwrap()s throughout the
module. It should not, however, lead to additional heap usage, because the sizes
of Arc<T> and Option<Arc<T>> are the same.

This bug is found pretty much immediately by the instantiate_many fuzz target,
now that the GC proposal is enabled for fuzzing.

---

Depends on https://github.com/bytecodealliance/wasmtime/pull/10434

Wasmtime GitHub notifications bot (Mar 20 2025 at 19:14):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Mar 20 2025 at 19:33):

fitzgen updated PR #10435.

Wasmtime GitHub notifications bot (Mar 20 2025 at 19:33):

fitzgen has enabled auto merge for PR #10435.

Wasmtime GitHub notifications bot (Mar 20 2025 at 20:10):

fitzgen merged PR #10435.

Last updated: Apr 17 2025 at 09:03 UTC