Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Description • 8 years ago

The following testcase crashes on mozilla-central revision bad312aefb42 (build with --enable-debug, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

// Adapted from randomly chosen test: js/src/jit-test/tests/debug/wasm-01.js
oomTest(function() {
    var g = newGlobal();
    g.eval("new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary('(module (func) (export \"\" 0))')));");
    var dbg = new Debugger(g);
    dbg.findScripts();
})


Backtrace:

0   js-dbg-64-clang-darwin-bad312aefb42	0x00000001031d38d7 js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 183 (jscntxtinlines.h:242)
1   js-dbg-64-clang-darwin-bad312aefb42	0x00000001031d3494 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) + 548 (Interpreter.cpp:447)
2   js-dbg-64-clang-darwin-bad312aefb42	0x00000001031cb254 Interpret(JSContext*, js::RunState&) + 36404 (Interpreter.cpp:2922)
3   js-dbg-64-clang-darwin-bad312aefb42	0x00000001031c21eb js::RunScript(JSContext*, js::RunState&) + 443 (Interpreter.cpp:405)
4   js-dbg-64-clang-darwin-bad312aefb42	0x00000001031d3448 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) + 472 (Interpreter.cpp:477)
/snip

For detailed crash information, see attachment.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 2 • 8 years ago

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/958074f3b830
user:        Dan Gohman
date:        Fri Sep 23 09:13:15 2016 -0500
summary:     Bug 1287220 - Baldr: update to binary version 0xc (r=luke)

Dan, is bug 1287220 a likely regressor?

	Benjamin Bouvier [:bbouvier] (inactive) Assignee
	Comment 3 • 8 years ago

Probably even older than that. Two issues here:
- Debugger::findScripts calls consider(WasmInstanceObject*), which can set oom, but never checks against this flag and report it.
- GetBufferSource can OOM without reporting too (because the Bytes in SharedBytes use the SystemAllocPolicy)

	Julien Cristau [:jcristau]
	Comment 4 • 8 years ago

tracking this wasm issue for 52

	Luke Wagner [:luke]
	Comment 5 • 8 years ago

Comment on attachment 8814361 [details] [diff] [review]
oom.patch

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

Thanks!

::: js/src/wasm/WasmJS.cpp
@@ +711,5 @@
>  
>      JSObject* unwrapped = CheckedUnwrap(obj);
>  
> +    size_t byteLength;
> +    uint8_t* ptr = nullptr;

Probably good to initialize byteLength too for symmetry.

	Benjamin Bouvier [:bbouvier] (inactive) Assignee
	Comment 6 • 8 years ago

Thanks for the review!

carrying forward r+, updated patch for aurora uplift.

	Pulsebot
	Comment 7 • 8 years ago

Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd4aacce0b00
Report OOM when going through all the wasm instances and when copying the wasm buffer source fails; r=luke

	Carsten Book [:Tomcat]
	Comment 8 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/fd4aacce0b00

	Benjamin Bouvier [:bbouvier] (inactive) Assignee
	Comment 9 • 8 years ago

Comment on attachment 8815179 [details] [diff] [review]
oom.patch

Approval Request Comment
[Feature/Bug causing the regression]: wasm
[User impact if declined]: errors not getting reported + undefined behavior when looking at wasm code through the debugger
[Is this code covered by automated tests?]: no; OOM bugs are very sensitive to memory allocations and thus hard to reproduce in a consistent fashion
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: very low
[Why is the change risky/not risky?]: tested/fuzzed for a week and no linked reports about it; plus it's a more conservative change
[String changes made/needed]: n/a

	Julien Cristau [:jcristau]
	Comment 10 • 8 years ago

Comment on attachment 8815179 [details] [diff] [review]
oom.patch

wasm fix for aurora52

	Carsten Book [:Tomcat]
	Comment 11 • 8 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/91b2d33c983f