Skip to content

Fix reference-type struct and array fields in the wast parser - #2867

Open
aizu-m wants to merge 1 commit into
WebAssembly:mainfrom
aizu-m:struct-field-ref-type
Open

aizu-m wants to merge 1 commit into
WebAssembly:mainfrom
aizu-m:struct-field-ref-type

Conversation

@aizu-m

@aizu-m aizu-m commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Debug wat2wasm, --enable-gc:

type.h:75: Assertion failed: (!IsReferenceWithIndex()), function Type

Found while fuzzing the text parser with all features enabled. A struct or array field whose type is a reference, such as (ref 0), (ref $t) or (ref null $t), reaches ParseField. It built the field type with Type(type.opt_type()). opt_type() only carries the Type::Enum, so the reference index is dropped and the single-argument Type(Enum) constructor asserts. A release build skips the assert and keeps a wrong type (index 0).

Every other field and value-type site already routes through VarToType (ParseGlobalType, the table and elem paths), which keeps the index and defers named references. ParseField now does the same. VarToType stores a pointer into the field for end-of-module resolution, and ParseFieldList copies each field into a vector, so those pointers are repointed at the stored elements once the vector has stopped growing.

Test covers indexed, named backward, named forward, mut and abstract-ref fields in both struct and array.

@zherczeg

Copy link
Copy Markdown
Collaborator

GC patches reworks these, probably not worth to update the code before those patches land.

@aizu-m

aizu-m commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Fair enough. I had a look at #2607: it makes the same change in ParseField (VarToType(type, &field->type)) and moves ParseFieldList over to a StructType*, so the per-field vector copy my patch had to repoint around disappears as well. No point duplicating that.

Happy to leave it to the GC series. The test here covers indexed, named backward and named forward refs in both struct and array, so take it across if it's useful coverage; otherwise this can just be closed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants