1108824 - Assertion failure: !cx->isExceptionPending(), at jscntxtinlines.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

// Randomly chosen test: js/src/tests/test262/ch10/10.4/10.4.3/10.4.3-1-15gs.js
// -- reduced away --
// Randomly chosen test: js/src/jit-test/tests/bug793385.js
try {
    gcparam("maxBytes", gcparam("gcBytes") + 1);
    function f() {
        f();
    }
    assertEq((f()), null);
} catch (e) {}
// Randomly chosen test: js/src/jit-test/tests/basic/bug532568.js
load("bug532568.js");
// jsfunfuzz
" " + ";";

asserts js debug shell on m-i changeset 917fafd942ae with --fuzzing-safe --no-threads --no-ion at Assertion failure: !cx->isExceptionPending(), at jscntxtinlines.h.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-inbound/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

This was found by combining random js tests together with jsfunfuzz, the specific file(s) is/are:

http://hg.mozilla.org/mozilla-central/file/917fafd942ae/js/src/tests/test262/ch10/10.4/10.4.3/10.4.3-1-15gs.js
http://hg.mozilla.org/mozilla-central/file/917fafd942ae/js/src/jit-test/tests/bug793385.js
http://hg.mozilla.org/mozilla-central/file/917fafd942ae/js/src/jit-test/tests/basic/bug532568.js

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/eae28492fdc6
user:        Terrence Cole
date:        Thu Dec 04 09:41:12 2014 -0800
summary:     Bug 1100493 - Call js_ReportOutOfMemory on all failure paths in refillFreeList; r=jorendorff

Terrence, is bug 1100493 a likely regressor?

Comment 1

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

bug532568.js is:

x = 0;
x += 0 + '';

Comment 2

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x5edbf9, 0x000000010064964e js-dbg-opt-64-dm-nsprBuild-darwin-917fafd942ae`js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) [inlined] js::NativeObject& JSObject::as<js::NativeObject>(this=<unavailable>) + 5 at jsobj.h:742, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010064964e js-dbg-opt-64-dm-nsprBuild-darwin-917fafd942ae`js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) [inlined] js::NativeObject& JSObject::as<js::NativeObject>(this=<unavailable>) + 5 at jsobj.h:742
    frame #1: 0x0000000100649649 js-dbg-opt-64-dm-nsprBuild-darwin-917fafd942ae`js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) [inlined] NoSuchMethod(argc=<unavailable>, vp=<unavailable>) at Interpreter.cpp:193
    frame #2: 0x0000000100649649 js-dbg-opt-64-dm-nsprBuild-darwin-917fafd942ae`js::Invoke(cx=<unavailable>, args=<unavailable>, construct=<unavailable>) + 1257 at Interpreter.cpp:471
    frame #3: 0x00000001006672b0 js-dbg-opt-64-dm-nsprBuild-darwin-917fafd942ae`Interpret(cx=<unavailable>, state=<unavailable>) + 46000 at Interpreter.cpp:2541
    frame #4: 0x000000010065bed9 js-dbg-opt-64-dm-nsprBuild-darwin-917fafd942ae`js::RunScript(cx=0x0000000101d01cf0, state=0x00007fff5fbff178) + 345 at Interpreter.cpp:434
(lldb)

Comment 3

[Terrence Cole [:terrence]]

Attachment: bug-1108824-v0.diff

The second js_ReportOutOfMemory I added to refillFreeLists in bug 1100493 is not actually correct -- in the OOM-but-NoGC case we need to let the caller retry.

Comment 5

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

There are many (seemingly-recent) variants of such asserts involving isExceptionPending, setting [fuzzblocker].

Comment 6

[Terrence Cole [:terrence]]

Hurm, we might want to back out and re-land 1100493 without the broken hunk then.

Comment 7

[Jason Orendorff [:jorendorff]]

Comment on attachment 8533426 [details] [diff] [review]
bug-1108824-v0.diff

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

I don't understand how you can tolerate this. How do you ever know what's correct?

Working on a static analysis that might reach this kind of mistake. Not sure.

Comment 8

[Terrence Cole [:terrence]]

(In reply to Jason Orendorff [:jorendorff] from comment #7)
> Comment on attachment 8533426 [details] [diff] [review]
> bug-1108824-v0.diff
> 
> Review of attachment 8533426 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't understand how you can tolerate this. How do you ever know what's
> correct?

I stand by my earlier statement: speckled chickens and fuzzers. And we're all out of chickens.
 
> Working on a static analysis that might reach this kind of mistake. Not sure.

The problem here is the decoupling of cx->exception and stack return codes. The correct solution is haskell's Maybe type. I guess that is arguably a static analysis. We're currently working hard to make Gecko effectively treat cx->exception as the return from Execute, so this may not actually be infeasible, if we want to put the work in.

Comment 9

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c291b2eac809

Comment 10

[Carsten Book [:Tomcat]]

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