Closed Bug 1117235 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox37 --- affected

People

(Reporter: gkw, Assigned: luke)

Details

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

Attachments

(2 files)

// Randomly chosen test: js/src/tests/js1_7/iterable/regress-355075-01.js
options('werror');
// Randomly chosen test: js/src/jit-test/tests/debug/onNewScript-off-main-thread-02.js
newGlobal().eval(`
    offThreadCompileScript("\
        function f() {\
            \\\"use asm\\\";\
        }");
    runOffThreadScript();
`);

asserts js debug shell on m-c changeset 13fe5ad0364d with --fuzzing-safe --ion-offthread-compile=off --ion-eager 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-central/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/13fe5ad0364d/js/src/tests/js1_7/iterable/regress-355075-01.js
http://hg.mozilla.org/mozilla-central/file/13fe5ad0364d/js/src/jit-test/tests/debug/onNewScript-off-main-thread-02.js

Due to skipped revisions, the first bad revision could be any of:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a0dd5a83ba36
user:        Jan de Mooij
date:        Thu Jul 24 11:56:43 2014 +0200
summary:     Bug 1031529 part 2 - Remove JS_THREADSAFE #ifdefs everywhere. r=bhackett

changeset:   https://hg.mozilla.org/mozilla-central/rev/6426fef52f51
user:        Jan de Mooij
date:        Thu Jul 24 11:56:45 2014 +0200
summary:     Bug 1031529 part 3 - Step defining JS_THREADSAFE, remove --disable-threadsafe. r=glandium

I don't think this is accurate, but it takes more time to go back and bisect further, so setting needinfo? from Jan to see if this is anything obvious.
Flags: needinfo?
Attached file stack
(lldb) bt
* thread #1: tid = 0xc05dd, 0x000000010069d4d3 js-dbg-opt-64-dm-nsprBuild-darwin-13fe5ad0364d`js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [inlined] js::ThreadSafeContext::asExclusiveContext(this=0x0000000105818000, cx=<unavailable>) const + 67 at jscntxt.h:226, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010069d4d3 js-dbg-opt-64-dm-nsprBuild-darwin-13fe5ad0364d`js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [inlined] js::ThreadSafeContext::asExclusiveContext(this=0x0000000105818000, cx=<unavailable>) const + 67 at jscntxt.h:226
    frame #1: 0x000000010069d490 js-dbg-opt-64-dm-nsprBuild-darwin-13fe5ad0364d`js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [inlined] void js::assertSameCompartment<JS::CallArgs>(js::ThreadSafeContext*, JS::CallArgs const&) at jscntxtinlines.h:157
    frame #2: 0x000000010069d490 js-dbg-opt-64-dm-nsprBuild-darwin-13fe5ad0364d`js::CallJSNative(cx=<unavailable>, native=<unavailable>, args=<unavailable>)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 368 at jscntxtinlines.h:230
    frame #3: 0x0000000100648e64 js-dbg-opt-64-dm-nsprBuild-darwin-13fe5ad0364d`js::Invoke(cx=<unavailable>, args=<unavailable>, construct=<unavailable>) + 420 at Interpreter.cpp:495
    frame #4: 0x00000001006329ef js-dbg-opt-64-dm-nsprBuild-darwin-13fe5ad0364d`js::Invoke(cx=0x0000000101b055b0, thisv=<unavailable>, fval=<unavailable>, argc=<unavailable>, argv=<unavailable>, rval=<unavailable>) + 815 at Interpreter.cpp:558
    frame #5: 0x000000010023b15c js-dbg-opt-64-dm-nsprBuild-darwin-13fe5ad0364d`js::jit::DoCallFallback(cx=0x0000000101b055b0, frame=<unavailable>, stub_=<unavailable>, argc=0, vp=0x00007fff5fbfd150, res=<unavailable>) + 2204 at BaselineIC.cpp:9493
    frame #6: 0x0000000101af54d3
(lldb)
Flags: needinfo? → needinfo?(jdemooij)
In TokenStream::reportAsmJSError we report a warning but werror is set so we throw an exception. But then we fail to propagate this in ~ModuleCompiler and then in Parser<ParseHandler>::functionDef.

Then off-thread compilation succeeds and we report the error on the main thread but we don't propagate this in finishParseTask.
Flags: needinfo?(jdemooij) → needinfo?(luke)
What is described in comment 2 is expected: higher up on the callstack, ValidateAsmJS returns NoExceptionPending() for this and related reasons.  The problem in this case is that, since we're parsing off thread, the exception isn't set immediately, but instead it is stored in parseTask->errors where it is later thrown in GlobalHelperThreadState::finishParseTask.  I suppose this is the only case where there is a error that throws but script compilation succeeds.

We could change ValidateAsmJS so that warnings-as-errors would cause failure, but this is a pain.  Alternatively, it seems nice to have an "if (cx->isExceptionPending()) return nullptr" backstop in finishParseTask so that we don't depend on the implicit invariant "error-thrown iff !script".
Assignee: nobody → luke
Status: NEW → ASSIGNED
Flags: needinfo?(luke)
Attachment #8544230 - Flags: review?(bhackett1024)
Attachment #8544230 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/9d2bba85e806
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: