Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wast2json fails to parse if.wast from latest testsuite #2347

Closed
andreaTP opened this issue Dec 4, 2023 · 4 comments · Fixed by #2349
Closed

wast2json fails to parse if.wast from latest testsuite #2347

andreaTP opened this issue Dec 4, 2023 · 4 comments · Fixed by #2349

Comments

@andreaTP
Copy link

andreaTP commented Dec 4, 2023

Hi 👋 and thanks for the super-helpful project!

On my Mac I ca reproduce as follows (in a tmp folder):

git clone https://github.com/WebAssembly/testsuite.git
wget https://github.com/WebAssembly/wabt/releases/download/1.0.34/wabt-1.0.34-macos-12.tar.gz
./wabt-1.0.34/bin/wast2json ./testsuite/if.wast

Produces the unexpected result:

./testsuite/if.wast:533:33: error: unexpected token (, expected ).
    (if (i32.const 1) (i32.eqz) (then) (else))
                                ^
./testsuite/if.wast:537:17: error: unexpected token "invoke", expected an instr.
(assert_return (invoke "empty" (i32.const 0)))
                ^^^^^^

The regression seems to be caused by this diff:
WebAssembly/testsuite@26d62d0#diff-76c78c41ca3eaa954f864741d9462243de307c6e0ca747b7bcab93fba9767837

@keithw
Copy link
Member

keithw commented Dec 4, 2023

Yes, here is the upstream addition to the testsuite: WebAssembly/spec#1682

Will have to take a look at the WAT parser (wasm-tools already fixed this here, which maybe we can use as inspiration: bytecodealliance/wasm-tools#1215).

Unfortunately this isn't the only change in the testsuite repo that will cause issues; the new testsuite also upgrades the exception-handling proposal to "v4" of that proposal (adding try_table, eliminating try_catch & try_delegate, and restoring exnref), which is going to require a bunch of work in the text/binary readers and writers, interpreter, and wasm2c to support

@andreaTP
Copy link
Author

andreaTP commented Dec 5, 2023

Thanks for the quick turnaround!
Do you have already a release date in mind?

@keithw
Copy link
Member

keithw commented Dec 5, 2023

Not really -- if this is something users are clamoring for, we could cut a release right now.

@andreaTP
Copy link
Author

andreaTP commented Dec 5, 2023

Just nice to have from my side, I'm not blocked 👍

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 a pull request may close this issue.

2 participants