-
Notifications
You must be signed in to change notification settings - Fork 747
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
Fuzz JSPI #7148
Fuzz JSPI #7148
Conversation
scripts/fuzz_shell.js
Outdated
setTimeout(() => { | ||
resolve(id); | ||
}, 0); // TODO: Use the ms in some reasonable, deterministic manner, but | ||
// the d8 shell ignores this anyhow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it best to avoid sleeping for any amount of time to avoid slowing the fuzzer down? Maybe we should just return a resolved promise here to make things run as quickly as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sleep would lead to interleaved execution, though, which seems useful to test. That is, actual swapping back and forth of stacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning a resolved promise would still interleave execution, though. It's basically equivalent to what d8 is doing since it's ignoring the sleep time anyway. For extra interleaving, rather than sleeping we could have an integer counter that recursively decrements on every turn of the microtask queue and finally resolves the promise when the counter hits zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah, we could manage interleaving manually that way, interesting idea. I can add that to the TODO
scripts/fuzz_shell.js
Outdated
@@ -312,13 +360,16 @@ function build(binary) { | |||
// keep the ability to call anything that was ever exported.) | |||
for (var key in instance.exports) { | |||
var value = instance.exports[key]; | |||
if (JSPI) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can move the if (JSPI)
check into wrapExportForJSPI
so we don't have to duplicate it everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, done.
fuzz_shell.js
. This is in the form of commented-out async/awaitkeywords - commented out so that normal fuzzing is not impacted. When we want
to fuzz JSPI, we uncomment them. We also apply the JSPI operations of marking
imports and exports as suspending/promising.
fuzz_opt.py
and ClusterFuzz'srun.py
.There is more that can be done here, but this did notice a real bug in V8
that I saw was fixed when I updated d8, so it seems useful already. Ideas
for more ways to fuzz this are welcome.