Skip to content

Fuzzer: Use a WebAssembly.Tag from JS#7277

Merged
kripken merged 17 commits into
WebAssembly:mainfrom
kripken:fuzz.imported.tag
Feb 7, 2025
Merged

Fuzzer: Use a WebAssembly.Tag from JS#7277
kripken merged 17 commits into
WebAssembly:mainfrom
kripken:fuzz.imported.tag

Conversation

@kripken

@kripken kripken commented Feb 6, 2025

Copy link
Copy Markdown
Member

We now create a Tag in JS and import it in the wasm.

We can then use that tag from JS when we throw (only some of the
time, controlled by a new parameter to the throwing import).

@kripken kripken requested a review from tlively February 6, 2025 19:09
@kripken kripken requested a review from aheejin February 6, 2025 21:47
Comment on lines 94 to +102
} else if (import->base == "throw") {
throwEmptyException();
if (arguments[0].geti32() == 0) {
// Throw in a way similar to JS.
throwEmptyException();
} else {
// Throw the argument in the imported JS tag.
auto payload = std::make_shared<ExnData>(fuzzTag, arguments);
throwException(WasmException{Literal(payload)});
}

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.

  1. if part: Doesn't arguments[0].geti32() == 0 mean we should throw 0 as an i32 (rather than an empty exception)?
  2. else part: fuzzTag is an i32, then what happens when the length of arguments is > 1?

Ah I understand what this means now I've seen TranslateToFuzzReader::makeImportThrowing, but I think this can be confusing... how about just passing an empty array when we want to throw an empty exception and add assert(arguments.size() == 1) in the else part?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, we can't pass an empty array, since the signature is fixed by the wasm import. (Or do you mean a wasm Array? We wouldn't want to depend on GC here though, as this can be used when GC is disabled.)

We could have two i32s, one "throw JS?" and the other meaning "if not JS, this is the value to throw". I basically just combined those two into one, as it didn't seem to matter much, but if that's clearer I'm happy to do it.

@aheejin aheejin Feb 6, 2025

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.

Not sure what "the signature is fixed by the wasm import" means..
What I meant was #7277 (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.

I see (after reading #7277 (comment)). How about adding a comment here too about what the rule is?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

Comment thread src/tools/fuzzing/fuzzing.cpp

@aheejin aheejin 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.

@kripken kripken merged commit 3fcfec4 into WebAssembly:main Feb 7, 2025
@kripken kripken deleted the fuzz.imported.tag branch February 7, 2025 21:30
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