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

[legacy] Add setjmp bait #332

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

SoniEx2
Copy link
Contributor

@SoniEx2 SoniEx2 commented Sep 22, 2024

sadly UBSAN doesn't appear to detect this (yet).

@rossberg
Copy link
Member

Shouldn't we have a corresponding test for the non-legacy instruction?

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Sep 22, 2024

yes, eventually

wasm2c only supports EHL at this point so having the test for EHv4 isn't that critical yet.

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Sep 23, 2024

surprisingly, this test also appears to break wabt interpreter.

for context, the interpreter isn't even using setjmp.

@aheejin
Copy link
Member

aheejin commented Sep 24, 2024

Sorry I may lack some context, but what is this test about? They say setjmp and longjmp but here they are just normal functions. Does this test have any specific characteristics or does this just happen to be the case the wabt interpreter fails on? Apparenly this PR doesn't have any changes to the spec interpreter so that is fine, right?

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Sep 24, 2024

the test targets wasm2c specifically, where exceptions are implemented using setjmp/longjmp, but also happens to trigger a bug in wabt interpreter (which is mostly unrelated to wasm2c, despite also being part of wabt).

the wabt interpreter is unrelated to the spec interpreter, the spec interpreter is probably fine.

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.

3 participants