Implement dummy wasm functions in deterministic mode

RESOLVED FIXED in Firefox 47

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bbouvier, Assigned: bbouvier)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Created attachment 8712235 [details] [diff] [review]
deterministic.patch

Gary showed me a piece of code on IRC that unevals the global object and prints out the result. On x86 without fpu, the two wasm functions aren't there, which causes a differential testing issue.

This patch implements dummy wasm functions to prevent this, but that's not enough: if wasm functions are dummy implementations, differential testing fuzzing will get crazy when comparing dummy vs real. So I made the dummy implementations write "WebAssembly is not supported" on stderr, thus fuzzers can pattern match this string and abort fuzzing, as they do for some error messages in asm.js https://github.com/MozillaSecurity/funfuzz/blob/master/js/compareJIT.py#L120 .
Attachment #8712235 - Flags: review?(luke)

Comment 2

2 years ago
Could we instead have wasmEval/wasmTextToString *always* be defined but if !SupportsWasm, they throw an error and, #ifdef MORE_DETERMINISTIC, also fprintf a message that can tell the fuzzers to ignore this test case?
(Assignee)

Comment 3

2 years ago
Created attachment 8712609 [details] [diff] [review]
deterministic.patch
Attachment #8712235 - Attachment is obsolete: true
Attachment #8712235 - Flags: review?(luke)
Attachment #8712609 - Flags: review?(luke)

Comment 4

2 years ago
Comment on attachment 8712609 [details] [diff] [review]
deterministic.patch

Review of attachment 8712609 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: js/src/asmjs/Wasm.cpp
@@ +575,5 @@
> +    if (!SupportsWasm(cx)) {
> +#ifdef JS_MORE_DETERMINISTIC
> +        fprintf(stderr, "WebAssembly is not supported.\n");
> +#endif // JS_MORE_DETERMINISTIC
> +        JS_ReportError(cx, "WebAssembly is not supported.");

Can you say "on the current device"?

@@ +653,5 @@
>  static bool
>  WasmTextToBinary(JSContext* cx, unsigned argc, Value* vp)
>  {
> +    if (!CheckWasmSupport(cx))
> +        return false;

I think this check isn't necessary.
Attachment #8712609 - Flags: review?(luke) → review+
(Assignee)

Updated

2 years ago
Flags: needinfo?(bbouvier)

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0ae79cfe1fdf
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
I've turned on --no-fpu testing again, this time ignoring the phrase "WebAssembly is not supported on the current device".

https://github.com/MozillaSecurity/funfuzz/commit/7cb9b3be4f26379f6fca96163456f85febf26f6e
You need to log in before you can comment on or make changes to this bug.