Skip to content
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
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
55 changes: 49 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,55 @@ 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;
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.

Copy link
Member

Choose a reason for hiding this comment

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

  1. There aren't any other errors that could happen here, right? Maybe we can just detect the presence of any error at all.

  2. What if the exported global holds a null anyref? Wouldn't that print null 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.

  1. At minimum there could be a VM bug that causes some other kind of error, and we would want to differentiate that. But also maybe the spec has more..? It just seems better to be precise.
  2. Yes, a null anyref will print null, but the type + the value is unambiguous. If the type is illegal, it will print null. We could also put a string here "illegal value" or such?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, "illegal value" or "failed conversion" or something like that sgtm.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

// Just log a string instead of a value we cannot access from JS,
// like an exnref. Note we don't need matching code on the C++
// side in execution-results.h because illegal exports are pruned
// anyhow if we are going to compare execution in JS to C++.
actualValue = '<illegal value>';
} 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...

Copy link
Member

Choose a reason for hiding this comment

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

maybe "export" ?

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, yeah, maybe just "export" is good. My brain was stuck looking for a verb 😄 Will do in a followup.

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
24 changes: 24 additions & 0 deletions test/lit/exec/fuzzing-api-globals.wast
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
;; 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))
(global $global-v128 v128 (v128.const i64x2 12 34))

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

;; 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]
;; CHECK-NEXT: [fuzz-exec] calling global-v128
;; CHECK-NEXT: [LoggingExternalInterface logging i32x4 0x0000000c 0x00000000 0x00000022 0x00000000]

Loading