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: Remove --emit-js-shell logic and reuse fuzz_shell.js instead #6310

Merged
merged 26 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions scripts/fuzz_opt.py
Original file line number Diff line number Diff line change
Expand Up @@ -665,8 +665,11 @@ def run_d8_js(js, args=[], liftoff=True):
return run_vm(cmd)


FUZZ_SHELL_JS = in_binaryen('scripts', 'fuzz_shell.js')


def run_d8_wasm(wasm, liftoff=True):
return run_d8_js(in_binaryen('scripts', 'fuzz_shell.js'), [wasm], liftoff=liftoff)
return run_d8_js(FUZZ_SHELL_JS, [wasm], liftoff=liftoff)


def all_disallowed(features):
Expand Down Expand Up @@ -746,8 +749,7 @@ class D8:
name = 'd8'

def run(self, wasm, extra_d8_flags=[]):
run([in_bin('wasm-opt'), wasm, '--emit-js-wrapper=' + wasm + '.js'] + FEATURE_OPTS)
return run_vm([shared.V8, wasm + '.js'] + shared.V8_OPTS + extra_d8_flags + ['--', wasm])
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
Expand Down Expand Up @@ -1036,7 +1038,8 @@ def fix_number(x):
compare_between_vms(before, interpreter, 'Wasm2JS (vs interpreter)')

def run(self, wasm):
wrapper = run([in_bin('wasm-opt'), wasm, '--emit-js-wrapper=/dev/stdout'] + FEATURE_OPTS)
with open(FUZZ_SHELL_JS) as f:
wrapper = f.read()
cmd = [in_bin('wasm2js'), wasm, '--emscripten']
# avoid optimizations if we have nans, as we don't handle them with
# full precision and optimizations can change things
Expand Down
49 changes: 36 additions & 13 deletions scripts/fuzz_shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,34 @@ var detrand = (function() {
};
})();

// Fuzz integration.
function logValue(x, y) {
// Print out a value in a way that works well for fuzzing.
function printed(x, y) {
if (typeof y !== 'undefined') {
console.log('[LoggingExternalInterface logging ' + x + ' ' + y + ']');
// A pair of i32s which are a legalized i64.
return x + ' ' + y;
} else if (x === null) {
// 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') {
// 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
// JS printing will emit some info about the reference and not a stable
// external representation of it). In those cases just print the type,
// which will be 'object' or 'function'.
return typeof x;
} else {
console.log('[LoggingExternalInterface logging ' + x + ']');
// A number. Print the whole thing.
return '' + x;
}
}

// Fuzzer integration.
function logValue(x, y) {
console.log('[LoggingExternalInterface logging ' + printed(x, y) + ']');
}

// Set up the imports.
var imports = {
'fuzzing-support': {
Expand Down Expand Up @@ -94,21 +113,25 @@ function refreshView() {
}

// Run the wasm.
var sortedExports = [];
for (var e in exports) {
sortedExports.push(e);
}
sortedExports.sort();
sortedExports.forEach(function(e) {
if (typeof exports[e] !== 'function') return;
if (typeof exports[e] !== 'function') {
continue;
}
// Send the function a null for each parameter. Null can be converted without
// error to both a number and a reference.
Comment on lines +120 to +121
Copy link
Member

Choose a reason for hiding this comment

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

What does sending a null for a parameter mean? Does this mean the null arg in

func.apply(null, args);

?

I'm not very familiar with JS still, so I looked it up (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply), and it looks the first param is thisArg. Do we need this arg? What does converting this to a number/reference mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yeah, this could be confusing because null is used in two ways. Yes, the first null is thisArg, which we have to pass only because we are using .apply. Basically we want to do

func(null, null);
func(null, null, null);

for a function with two or three parameters, etc. To do that we want to send a vector of arguments, which means we need to use apply, which forces us to provide something for thisArg. The value there doesn't matter.

The nulls that we pass in to the params get converted into wasm types. Sending null to an i32 param gives a 0, and to a (ref null any) gives (ref.null any) and so forth. The specific values don't matter (though it would be nice eventually to send in more values than 0, but the fuzzer added calls inside the wasm with such things). But we do not want to trap because of a conversion, which would happen if we passed 0 because that can't be converted to a (ref null any) for example.

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Do we care about supporting non-nullable reference parameters?

Copy link
Member Author

Choose a reason for hiding this comment

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

In calls from JS I'd say no - we don't have a good way to construct those atm. As mentioned above the fuzzer generates internal calls to exports which gives internal coverage for such functions at least (but not for the JS-wasm boundary, which maybe some day will be important here).

var func = exports[e];
var args = [];
for (var i = 0; i < func.length; i++) {
args.push(null);
}
try {
console.log('[fuzz-exec] calling ' + e);
var result = exports[e]();
var result = func.apply(null, args);
if (typeof result !== 'undefined') {
console.log('[fuzz-exec] note result: $' + e + ' => ' + result);
console.log('[fuzz-exec] note result: ' + e + ' => ' + printed(result));
}
} catch (e) {
console.log('exception!');// + [e, e.stack]);
}
});
}

14 changes: 12 additions & 2 deletions src/tools/execution-results.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,18 @@ struct ExecutionResults {
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
// optimizations.
std::cout << resultType << '\n';
// optimizations. Do not print the type in detail (as even that
// may change due to closed-world optimizations); just print a
// simple type like JS does, 'object' or 'function', but also
// print null for a null (so a null function does not get
// printed as object, as in JS we have typeof null == 'object').
if (values->size() == 1 && (*values)[0].isNull()) {
std::cout << "null\n";
} else if (resultType.isFunction()) {
std::cout << "function\n";
} else {
std::cout << "object\n";
}
} else {
// Non-references can be printed in full. So can strings, since we
// always know how to print them and there is just one string
Expand Down
134 changes: 0 additions & 134 deletions src/tools/js-wrapper.h

This file was deleted.

24 changes: 0 additions & 24 deletions src/tools/wasm-opt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

#include "execution-results.h"
#include "fuzzing.h"
#include "js-wrapper.h"
#include "optimization-options.h"
#include "pass.h"
#include "shell-interface.h"
Expand Down Expand Up @@ -86,7 +85,6 @@ int main(int argc, const char* argv[]) {
bool fuzzPasses = false;
bool fuzzMemory = true;
bool fuzzOOB = true;
std::string emitJSWrapper;
std::string emitSpecWrapper;
std::string emitWasm2CWrapper;
std::string inputSourceMapFilename;
Expand Down Expand Up @@ -180,15 +178,6 @@ int main(int argc, const char* argv[]) {
WasmOptOption,
Options::Arguments::Zero,
[&](Options* o, const std::string& arguments) { fuzzOOB = false; })
.add("--emit-js-wrapper",
"-ejw",
"Emit a JavaScript wrapper file that can run the wasm with some test "
"values, useful for fuzzing",
WasmOptOption,
Options::Arguments::One,
[&](Options* o, const std::string& arguments) {
emitJSWrapper = arguments;
})
.add("--emit-spec-wrapper",
"-esw",
"Emit a wasm spec interpreter wrapper file that can run the wasm with "
Expand Down Expand Up @@ -329,24 +318,11 @@ int main(int argc, const char* argv[]) {
}
}

if (emitJSWrapper.size() > 0) {
// As the code will run in JS, we must legalize it.
PassRunner runner(&wasm);
runner.add("legalize-js-interface");
runner.run();
}

ExecutionResults results;
if (fuzzExecBefore) {
results.get(wasm);
}

if (emitJSWrapper.size() > 0) {
std::ofstream outfile;
outfile.open(wasm::Path::to_path(emitJSWrapper), std::ofstream::out);
outfile << generateJSWrapper(wasm);
outfile.close();
}
if (emitSpecWrapper.size() > 0) {
std::ofstream outfile;
outfile.open(wasm::Path::to_path(emitSpecWrapper), std::ofstream::out);
Expand Down
4 changes: 2 additions & 2 deletions test/lit/exec/no-compare-refs.wast
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
;; The type of the reference this function returns will change as a result of
;; signature pruning. The fuzzer should not complain about this.
;; CHECK: [fuzz-exec] calling return-ref
;; CHECK-NEXT: [fuzz-exec] note result: return-ref => funcref
;; CHECK-NEXT: [fuzz-exec] note result: return-ref => function
(func $return-ref (export "return-ref") (result funcref)
(ref.func $no-use-param)
)
)
;; CHECK: [fuzz-exec] calling return-ref
;; CHECK-NEXT: [fuzz-exec] note result: return-ref => funcref
;; CHECK-NEXT: [fuzz-exec] note result: return-ref => function
;; CHECK-NEXT: [fuzz-exec] comparing return-ref
4 changes: 0 additions & 4 deletions test/lit/help/wasm-opt.test
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@
;; CHECK-NEXT: loads/stores/indirect calls when
;; CHECK-NEXT: fuzzing
;; CHECK-NEXT:
;; CHECK-NEXT: --emit-js-wrapper,-ejw Emit a JavaScript wrapper file
;; CHECK-NEXT: that can run the wasm with some
;; CHECK-NEXT: test values, useful for fuzzing
;; CHECK-NEXT:
;; CHECK-NEXT: --emit-spec-wrapper,-esw Emit a wasm spec interpreter
;; CHECK-NEXT: wrapper file that can run the
;; CHECK-NEXT: wasm with some test values,
Expand Down
2 changes: 1 addition & 1 deletion test/lit/unicode-filenames.wast
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
;; RUN: wasm-as %s -o %t-❤.wasm --source-map %t-🗺️.map
;; RUN: cat %t-🗺️.map | filecheck %s --check-prefix SOURCEMAP
;; RUN: wasm-opt %t-❤.wasm -o %t-🤬.wasm --emit-js-wrapper %t-❤.js --input-source-map %t-🗺️.map --output-source-map %t-🗺️.out.map
;; RUN: wasm-opt %t-❤.wasm -o %t-🤬.wasm --emit-spec-wrapper %t-❤.js --input-source-map %t-🗺️.map --output-source-map %t-🗺️.out.map
;; RUN: cat %t-🗺️.out.map | filecheck %s --check-prefix SOURCEMAP
;; RUN: wasm-dis %t-🤬.wasm | filecheck %s --check-prefix MODULE

Expand Down
Loading
Loading