Skip to content
Open
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
21 changes: 16 additions & 5 deletions scripts/fuzz_opt.py
Original file line number Diff line number Diff line change
Expand Up @@ -1370,14 +1370,24 @@ class CtorEval(TestCaseHandler):
frequency = 0.1

def handle(self, wasm):
# get the expected execution results.
wasm_exec = run_bynterp(wasm, ['--fuzz-exec-before'])

# get the list of func exports, so we can tell ctor-eval what to eval.
ctors = ','.join(get_exports(wasm, ['func']))
func_exports = get_exports(wasm, ['func'])
ctors = ','.join(func_exports)
if not ctors:
return

# The fuzzer evaluates exports in the order they are given, so if there
# are global exports it may read them before the ctors are run - but
# the ctors are meant to run before anything else, and can modify
# those global values. Keep only function exports to avoid this
# confusion.
filtered = wasm + '.filtered.wasm'
filter_exports(wasm, filtered, func_exports)
wasm = filtered

# get the expected execution results.
wasm_exec = run_bynterp(wasm, ['--fuzz-exec-before'])

# Fix escaping of the names, as we will be passing them as commandline
# parameters below (e.g. we want --ctors=foo\28bar and not
# --ctors=foo\\28bar; that extra escaping \ would cause an error).
Expand Down Expand Up @@ -1871,7 +1881,8 @@ def handle(self, wasm):

# Make sure that we actually executed all exports from both
# wasm files.
exports = get_exports(wasm, ['func']) + get_exports(second_wasm, ['func'])
exports = get_exports(wasm, ['func', 'global'])
exports += get_exports(second_wasm, ['func', 'global'])
calls_in_output = output.count(FUZZ_EXEC_CALL_PREFIX)
if calls_in_output == 0:
print(f'warning: no calls in output. output:\n{output}')
Expand Down
53 changes: 47 additions & 6 deletions scripts/fuzz_shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@ function callFunc(func) {
return func.apply(null, args);
}

// wasm2js does not define RuntimeError, so use that to check for it. wasm2js
// overrides the entire WebAssembly object with a polyfill, so we know exactly
// what it contains, and we need to handle some things differently below.
var wasm2js = !WebAssembly.RuntimeError;

// Calls a given function in a try-catch. Return 1 if an exception was thrown.
// If |rethrow| is set, and an exception is thrown, it is caught and rethrown.
// Wasm traps are not swallowed (see details below).
Expand All @@ -213,11 +218,7 @@ function callFunc(func) {

// We only want to catch exceptions, not wasm traps: traps should still
// halt execution. Handling this requires different code in wasm2js, so
// check for that first (wasm2js does not define RuntimeError, so use
// that for the check - when wasm2js is run, we override the entire
// WebAssembly object with a polyfill, so we know exactly what it
// contains).
var wasm2js = !WebAssembly.RuntimeError;
// check for that first.
if (!wasm2js) {
// When running native wasm, we can detect wasm traps.
if (e instanceof WebAssembly.RuntimeError) {
Expand Down Expand Up @@ -555,13 +556,53 @@ function build(binary, isSecond) {
name = e;
value = exports[e];
} else {
// We are given an object form exportList, which has both a name and a
// We are given an object from exportList, which has both a name and a
// value.
name = e.name;
value = e.value;
}

// Check for a global. Note we must be careful in wasm2js mode, where we
// can't do instanceof here (the wasm polyfill there doesn't have such
// things). In wasm2js we strip global exports to avoid needing to handle
// them here (using stub-unsupported-js).
if (!wasm2js && (value instanceof WebAssembly.Global)) {
// We can log a global value and do other operations to check for bugs.
// First, do some operations on the Global wrapper itself.
JSON.stringify(value);
value.foobar;

// Log it at the right time later using a lambda. Note that we can't just
// capture |value| for the lambda, as the loop modifies it.
(() => {
var global = value;
Comment on lines +575 to +578
Copy link
Member

Choose a reason for hiding this comment

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

Not surprised to be surprised once again by JS semantics. Even the var global = value is not enough to avoid the need for this outer lambda?

Copy link
Member Author

Choose a reason for hiding this comment

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

It isn't enough, correct. For loops etc. don't create scopes in JS like functions do.

value = () => {
// Time to log. Look at the exported value itself, not the global
// wrapper.
let actualValue;
try {
actualValue = global.value;
} catch (e) {
if (e.message.startsWith('get WebAssembly.Global.value')) {
Copy link
Member

Choose a reason for hiding this comment

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

  1. Can we check the type of the error rather than the message? Error messages are not standardized and may differ between implementations.

  2. Maybe we should print something else to distinguish this case from an exported global actually containing null.

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. I think it's just a generic RuntimeError, so the text is really all we have to go on...
  2. Hmm, but we'd need to print some value in other fuzzer places too. I don't think this is ambiguous - this happens only for the illegal types. So if the type is v128, we know 'null' means "i can't print this" etc.

// Just log a null instead of a value we cannot access from JS,
// like an exnref.
actualValue = null;
} else {
throw e;
}
}
if (typeof actualValue === 'object') {
// logRef can do a little more than logValue, so use it when possible.
logRef(actualValue);
} else {
logValue(actualValue);
}
};
})();
}

if (typeof value !== 'function') {
// Nothing we can call.
continue;
}

Expand Down
1 change: 0 additions & 1 deletion src/passes/RemoveNonJSOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,6 @@ struct StubUnsupportedJSOpsPass
void visitModule(Module* module) {
// We remove global exports, as wasm2js doesn't emit them in a fully
// compatible form yet (they aren't instances of WebAssembly.Global).
// Globals.
std::vector<Name> badExports;
for (auto& exp : module->exports) {
if (exp->kind == ExternalKind::Global) {
Expand Down
38 changes: 24 additions & 14 deletions src/tools/execution-results.h
Original file line number Diff line number Diff line change
Expand Up @@ -487,23 +487,33 @@ struct ExecutionResults {
// execute all exported methods (that are therefore preserved through
// opts)
for (auto& exp : wasm.exports) {
if (exp->kind != ExternalKind::Function) {
continue;
}
std::cout << "[fuzz-exec] calling " << exp->name << "\n";
auto* func = wasm.getFunction(*exp->getInternalName());
FunctionResult ret = run(func, wasm, instance);
results[exp->name] = ret;
if (auto* values = std::get_if<Literals>(&ret)) {
// ignore the result if we hit an unreachable and returned no value
if (values->size() > 0) {
std::cout << "[fuzz-exec] note result: " << exp->name << " => ";
for (auto value : *values) {
printValue(value);
std::cout << '\n';
if (exp->kind == ExternalKind::Function) {
std::cout << "[fuzz-exec] calling " << exp->name << "\n";
auto* func = wasm.getFunction(*exp->getInternalName());
FunctionResult ret = run(func, wasm, instance);
results[exp->name] = ret;
if (auto* values = std::get_if<Literals>(&ret)) {
// ignore the result if we hit an unreachable and returned no value
if (values->size() > 0) {
std::cout << "[fuzz-exec] note result: " << exp->name << " => ";
for (auto value : *values) {
printValue(value);
std::cout << '\n';
}
}
}
} else if (exp->kind == ExternalKind::Global) {
// Log the global's value. (We use "calling" here to match the output
// for calls, which simplifies the fuzzer.)
std::cout << "[fuzz-exec] calling " << exp->name << "\n";
Comment on lines +506 to +508
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove "calling" everywhere as a follow-up?

Copy link
Member Author

Choose a reason for hiding this comment

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

Any idea for what to replace it with? We need something to differentiate it from other things. I don't have a good idea myself...

Literals* value = instance.getExportedGlobalOrNull(exp->name);
assert(value);
assert(value->size() == 1);
std::cout << "[LoggingExternalInterface logging ";
printValue((*value)[0]);
std::cout << "]\n";
}
// Ignore other exports for now. TODO
}
}

Expand Down
21 changes: 21 additions & 0 deletions test/lit/exec/fuzzing-api-globals.wast
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
;; Test logging of global exports. We don't do this using update_lit_checks as
;; global exports confuse the auto-updater.

(module
(type $struct (struct))

(global $global (mut i32) (i32.const 42))

(global $global-immref anyref (struct.new $struct))

(export "global" (global $global))
(export "global-immref" (global $global-immref))
)

;; RUN: wasm-opt %s -all --fuzz-exec -o /dev/null 2>&1 | filecheck %s

;; CHECK: [fuzz-exec] calling global
;; CHECK-NEXT: [LoggingExternalInterface logging 42]
;; CHECK-NEXT: [fuzz-exec] calling global-immref
;; CHECK-NEXT: [LoggingExternalInterface logging object]

Loading