Fuzzer: Allow using initial content with V8 #6327
Conversation
|
Ah, there are other issues preventing this, the fuzzer found one. Converted to draft for now. |
|
This looks ready now. I added a few trivial fuzzer fixes (see updated top comment) and with those I can fuzz over 6,000 iterations without issue. |
|
(Also the nontrivial fix in #6331 needed to land first.) |
| @@ -311,6 +311,8 @@ def is_git_repo(): | |||
| 'exception-handling.wast', | |||
| 'translate-eh-old-to-new.wast', | |||
| 'rse-eh.wast', | |||
| # Non-UTF8 strings trap in V8 | |||
There was a problem hiding this comment.
Trap or validation failure? Should we make them validation failures in Binaryen as well?
There was a problem hiding this comment.
Hmm, I think it was a trap actually but I'd need to find the testcase again to check.
But I don't think these should fail to validate - non-utf8 strings are ok in the stringref proposal, as you can use other encodings there. It should only trap if you use the utf8 encoding I think.
There was a problem hiding this comment.
According to the overivew, the string literals must be valid WTF-8, although it doesn't say what happens (presumably a decoding error) if they're not: https://github.com/WebAssembly/stringref/blob/main/proposals/stringref/Overview.md#string-literal-section
There was a problem hiding this comment.
Oh, good catch... but then why does the proposal also support other encodings elsewhere?
There was a problem hiding this comment.
Presumably they want to support encoding and decoding to all kinds of formats for interoperability with other systems, but there's no need to support more than one format for literals because they can be encoded correctly, whatever correctly means, during compilation.
|
Landing, though this may keep finding new bugs for a while, like #6336 for example. But the frequency of them seems low (multiples of 10K iterations to find one) which seems acceptable on |
One problem was that spec testcases had exports with names that are not valid to write as JS exports.name. For example an export with a - in the name would end up as exports.foo-bar etc. Since WebAssembly#6310 that is fixed as we do not emit such JS (we use the generic fuzz_shell.js script which iterates over the keys in exports with exports[name]). Also fix a few trivial fuzzer issues that initial content uncovered: - Ignore a wat file with invalid utf-8. - Print string literals in the same way from JS as from C++. - Enable the stringref flag in V8. - Remove tag imports (the same as we do for global and function and other imports).
One problem was that spec testcases had exports with names that are not
valid to write as JS
exports.name. For example an export with a-in thename would end up as
exports.foo-baretc. Since #6310 that is fixed aswe do not emit such JS (we use the generic
fuzz_shell.jsscript which iteratesover the keys in
exportswithexports[name]).Also fix a few trivial fuzzer issues that initial content uncovered: