1320226 - Assertion failure: !cx->isExceptionPending(), at js/src/jscntxtinlines.h:242

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Detailed Crash Information

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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?

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: oom.patch

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)

Comment 4

[Julien Cristau [:jcristau]]

tracking this wasm issue for 52

Comment 5

[Luke Wagner [:luke]]

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.

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: oom.patch

Thanks for the review!

carrying forward r+, updated patch for aurora uplift.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
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

Comment 8

[Carsten Book [:Tomcat]]

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

Comment 9

[Benjamin Bouvier [:bbouvier] (inactive)]

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

Comment 10

[Julien Cristau [:jcristau]]

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

wasm fix for aurora52

Comment 11

[Carsten Book [:Tomcat]]

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