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

[EH] Fuzzer: Add WebAssembly.JSTag fuzzing #7283

Merged
merged 30 commits into from
Feb 10, 2025

Conversation

kripken
Copy link
Member

@kripken kripken commented Feb 7, 2025

This JS API represents the tag of JS exceptions. Import it into the wasm
so that wasm can catch and throw JS exceptions.

Rename the previous "jsTag", which means "wasm tag created in JS" to
"wasmTag" to avoid confusion.

@kripken kripken requested a review from aheejin February 7, 2025 21:37
Copy link
Member

@aheejin aheejin left a comment

Choose a reason for hiding this comment

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

Do we have tests that actually use the JSTag?

@kripken
Copy link
Member Author

kripken commented Feb 7, 2025

Yes, once we import it into wasm, it is just another in the list of tags, which means it will get catches, throws, etc.

@kripken
Copy link
Member Author

kripken commented Feb 8, 2025

The fuzzer just found a bug here in fact, I pushed a fix. I forgot to use the imported tag in execution-results, so the binaryen interpreter was not catching JS exceptions when it used that tag (which differs from v8, so the fuzzer errored).

@@ -173,8 +181,11 @@ struct LoggingExternalInterface : public ShellExternalInterface {
}

void throwEmptyException() {
Copy link
Member

Choose a reason for hiding this comment

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

If the intention of this method is to throw a JS exception, how about naming it throwJSException or something to be clear?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, done.

@kripken
Copy link
Member Author

kripken commented Feb 10, 2025

The fuzzer noticed that duplicate tags can cause issues due to the interpreter using tag names, see

#6823 (comment)

I pushed a workaround.

@kripken kripken merged commit 57c9c25 into WebAssembly:main Feb 10, 2025
14 checks passed
@kripken kripken deleted the fuzz.imported.tag2 branch February 10, 2025 21:26
kripken added a commit that referenced this pull request Feb 10, 2025
…#7286)

We do not compare exceptions in binaryen (not in the optimizer, where we
assume we can reorder traps, and not in the fuzzer, where we assume VMs
may have different text for them). But, since we have try-catch in wasm,
we can actually end up comparing them, by catching the exception and
logging the output. For that reason, we need to throw exactly the same
JS exception in #7283, which this fixes.
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