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

Fuzzer: Allow using initial content with V8 #6327

Merged
merged 14 commits into from
Feb 22, 2024
7 changes: 3 additions & 4 deletions scripts/fuzz_opt.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Trap or validation failure? Should we make them validation failures in Binaryen as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, good catch... but then why does the proposal also support other encodings elsewhere?

Copy link
Member

Choose a reason for hiding this comment

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

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.

'string-lowering.wast',
]


Expand Down Expand Up @@ -752,10 +754,7 @@ def run(self, wasm, extra_d8_flags=[]):
return run_vm([shared.V8, FUZZ_SHELL_JS] + shared.V8_OPTS + extra_d8_flags + ['--', wasm])

def can_run(self, wasm):
# INITIAL_CONTENT is disallowed because some initial spec testcases
# have names that require mangling, see
# https://github.com/WebAssembly/binaryen/pull/3216
return not INITIAL_CONTENTS
return True

def can_compare_to_self(self):
# With nans, VM differences can confuse us, so only very simple VMs
Expand Down
5 changes: 4 additions & 1 deletion scripts/fuzz_shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ function printed(x, y) {
// JS has just one null. Print that out rather than typeof null which is
// 'object', below.
return 'null';
} else if (typeof x !== 'number' && typeof x !== 'string') {
} else if (typeof x === 'string') {
// Emit a string in the same format as the binaryen interpreter.
return 'string("' + x + '")';
} else if (typeof x !== 'number') {
// Something that is not a number or string, like a reference. We can't
// print a reference because it could look different after opts - imagine
// that a function gets renamed internally (that is, the problem is that
Expand Down
1 change: 1 addition & 0 deletions scripts/test/shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ def has_shell_timeout():
'--experimental-wasm-typed-funcref',
'--experimental-wasm-memory64',
'--experimental-wasm-extended-const',
'--experimental-wasm-stringref',
'--wasm-final-types',
]

Expand Down
2 changes: 1 addition & 1 deletion src/tools/execution-results.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ struct ExecutionResults {
// ignore the result if we hit an unreachable and returned no value
if (values->size() > 0) {
std::cout << "[fuzz-exec] note result: " << exp->name << " => ";
auto resultType = func->getResults();
auto resultType = values->getType();
if (resultType.isRef() && !resultType.isString()) {
// Don't print reference values, as funcref(N) contains an index
// for example, which is not guaranteed to remain identical after
Expand Down
9 changes: 9 additions & 0 deletions src/tools/fuzzing/fuzzing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,15 @@ void TranslateToFuzzReader::setupGlobals() {
}

void TranslateToFuzzReader::setupTags() {
// As in modifyInitialFunctions(), we can't allow tag imports as it would trap
// when the fuzzing infrastructure doesn't know what to provide.
for (auto& tag : wasm.tags) {
if (tag->imported()) {
tag->module = tag->base = Name();
}
}

// Add some random tags.
Index num = upTo(3);
for (size_t i = 0; i < num; i++) {
addTag();
Expand Down
Loading