Closed Bug 1219905 Opened 6 years ago Closed 6 years ago

Assertion failure: cx->isExceptionPending(), at builtin/TestingFunctions.cpp

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox44 --- affected
firefox45 --- fixed

People

(Reporter: gkw, Assigned: jonco)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:])

Attachments

(2 files, 1 obsolete file)

// Adapted from randomly chosen test: js/src/jit-test/tests/debug/onExceptionUnwind-12.js
var g = newGlobal();
g.parent = this;
g.eval("new Debugger(parent).onExceptionUnwind = function() {}");
// Adapted from randomly chosen test: js/src/jit-test/tests/gc/oomInParseFunction.js
oomTest(() => l);

asserts js debug shell on m-c changeset fc706d376f06 with --fuzzing-safe --no-threads --ion-eager at Assertion failure: cx->isExceptionPending(), at builtin/TestingFunctions.cpp

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-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --disable-threadsafe --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic" -r fc706d376f06

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9c365490d4ce
user:        Jon Coppeard
date:        Tue Oct 13 13:37:07 2015 +0100
summary:     Bug 1212469 - Make oomTest() into a shell function r=nbp

Jon, is bug 1212469 a likely regressor? (I tried removing oomTest but it didn't seem to reproduce)
Flags: needinfo?(jcoppeard)
Attached file stack
(lldb) bt 5
* thread #1: tid = 0x3215ba, 0x000000010042109d js-dbg-64-dm-darwin-fc706d376f06`OOMTest(cx=0x0000000102c45400, argc=<unavailable>, vp=0x00007fff5fbfe498) + 1485 at TestingFunctions.cpp:1161, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010042109d js-dbg-64-dm-darwin-fc706d376f06`OOMTest(cx=0x0000000102c45400, argc=<unavailable>, vp=0x00007fff5fbfe498) + 1485 at TestingFunctions.cpp:1161
    frame #1: 0x000000010066c232 js-dbg-64-dm-darwin-fc706d376f06`js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [inlined] js::CallJSNative(cx=0x0000000102c45400, native=(js-dbg-64-dm-darwin-fc706d376f06`OOMTest(JSContext*, unsigned int, JS::Value*) at TestingFunctions.cpp:1097))(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 174 at jscntxtinlines.h:235
    frame #2: 0x000000010066c184 js-dbg-64-dm-darwin-fc706d376f06`js::Invoke(cx=0x0000000102c45400, args=0x00007fff5fbfe440, construct=<unavailable>) + 596 at Interpreter.cpp:489
    frame #3: 0x0000000100691f3d js-dbg-64-dm-darwin-fc706d376f06`js::Invoke(cx=0x0000000102c45400, thisv=0x00007fff5fbfe6a0, fval=<unavailable>, argc=<unavailable>, argv=<unavailable>, rval=<unavailable>) + 669 at Interpreter.cpp:542
    frame #4: 0x000000010098138b js-dbg-64-dm-darwin-fc706d376f06`js::jit::DoCallFallback(cx=0x0000000102c45400, frame=0x00007fff5fbfe8b8, stub_=0x0000000103fafa20, argc=1, vp=0x00007fff5fbfe868, res=<unavailable>) + 2795 at BaselineIC.cpp:9023
(lldb)
Attached patch bug1219905-debugger-oom (obsolete) — Splinter Review
Not related to bug 1212469.

The problem is that if we hit OOM while firing the onExceptionUnwind hook we end up returning false but without an exception pending (this is something that oomTest() checks).

Debugger::handleUncaughtException() does report the OOM to the debugger context.  I think that in this case we want fireExceptionUnwind() to propagate the original exception as well.  Here's a patch to do this, and it passes the tests. 

Jan, do you think this makes sense?
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Attachment #8681227 - Flags: feedback?(jdemooij)
We agreed on IRC that it would require more significant changes to the debugger to make the check that an exception is pending on execution failure true everywhere, and that the best course of action here is just to relax the check.
Attachment #8681227 - Flags: feedback?(jdemooij)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
As discussed, remove the troublesome assert.
Attachment #8681227 - Attachment is obsolete: true
Attachment #8686131 - Flags: review?(jdemooij)
Attachment #8686131 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/4fe9c44ee56a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Duplicate of this bug: 1225078
You need to log in before you can comment on or make changes to this bug.