Skip to content

Fix table.init text format parsing - #7654

Merged
tlively merged 10 commits into
WebAssembly:mainfrom
osa1:table_init_wast_parsing
Jun 16, 2025
Merged

tlively merged 10 commits into
WebAssembly:mainfrom
osa1:table_init_wast_parsing

Conversation

@osa1

@osa1 osa1 commented Jun 13, 2025 •

Copy link
Copy Markdown
Contributor

table.init text format has two forms:

  • table.init tableidx elemidx
  • table.init elemidx (tableidx is implicitly 0)

Because the optional tableidx is not the last (second) argument, we have to parse one or two immediate arguments, and depending on how many we parsed decide which one is elemidx.

Currently the code assumes first immediate argument is always a table, which does not handle the second form.

Before this PR, wasm-shell fails to parse this functions in spec tests: https://github.com/WebAssembly/testsuite/blob/e05365077e13a1d86ffe77acfb1a835b7aa78422/bulk.wast#L207-L211

After the PR we're able to parse it, but we still can't parse the whole file, because of other issues.

Error when parsing bulk.wast, before this PR:

209:6: error: expected elem index or identifier

After this PR:

250:33: error: unrecognized instruction

(The new error is about elem.drop, which we don't support yet, see #7209)

@osa1
osa1 marked this pull request as draft June 13, 2025 10:54
@osa1
osa1 marked this pull request as ready for review June 13, 2025 11:20
Comment thread src/parser/lexer.h Outdated
@kripken
kripken requested a review from tlively June 13, 2025 15:23
Comment thread src/parser/lexer.h Outdated
Comment thread src/parser/parsers.h Outdated
@tlively

tlively commented Jun 15, 2025

Copy link
Copy Markdown
Member

It would also be good to update the comment here as part of this.

@osa1

osa1 commented Jun 16, 2025

Copy link
Copy Markdown
Contributor Author

It would also be good to update the comment here as part of this.

Done. With this change we're able to parse elem.wast but I think it has a wrong test, I reported it to Wasm spec repo and added link to it in this file.

Comment thread src/parser/parsers.h Outdated
Comment thread scripts/test/shared.py Outdated

@tlively tlively left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@tlively
tlively enabled auto-merge (squash) June 16, 2025 12:55
@tlively
tlively merged commit c91c052 into WebAssembly:main Jun 16, 2025
@osa1
osa1 deleted the table_init_wast_parsing branch June 16, 2025 13:09
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