Assertion failure: !hasIonScript(), at jsscriptinlines.h

RESOLVED FIXED in Firefox 36

Status

()

Core
JavaScript Engine: JIT
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gkw, Assigned: shu)

Tracking

(Blocks: 2 bugs, {assertion, regression, testcase})

Trunk
mozilla37
x86_64
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox36 fixed, firefox37 affected)

Details

(Whiteboard: [jsbugmon:update,testComment=8,origRev=1427b365cd39])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
b1 = "";
b2 = "";
eval(`
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    function compareSource() {
        replace(/[]/g, '').
        replace(/[]/g, '');
        try {} catch (e) {}
    }
    function a1() {}
    function a2() {
        options().split(',');
    }
    a2();
`)
// Randomly chosen test: js/src/jit-test/tests/closures/set-outer-trace-4.js
try {
    load("z.js");
} catch (e) {}
// Randomly chosen test: js/src/jit-test/tests/baseline/bug836742.js
eval(`
    var g = newGlobal();
    g.debuggeeGlobal = this;
    g.eval("(" + function() {
        var dbg = Debugger(debuggeeGlobal);
        dbg.onEnterFrame = function(frame) {};
        dbg.onExceptionUnwind = function(frame) {}
    } + ")()");
    function f(n) {}
    try {} catch (e) {}
`)
// jsfunfuzz
function r() {}
var s = [
    function() {}
]
var e = ([{}, , ])
function r() {
    (function() {})
}
r([])
var a = "" ? function() {} : function() {}

and z.js is:

function f() {}

asserts js debug shell on m-c changeset 1427b365cd39 with --fuzzing-safe --gc-zeal=10 --no-threads --ion-eager at Assertion failure: !hasIonScript(), at jsscriptinlines.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/1427b365cd39/js/src/jit-test/tests/closures/set-outer-trace-4.js
http://hg.mozilla.org/mozilla-central/file/1427b365cd39/js/src/jit-test/tests/baseline/bug836742.js

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/5e513880db47
parent:      219877:b2649ad7d86e
user:        Boris Zbarsky
date:        Sat Dec 13 01:25:25 2014 -0500
summary:     Bug 1111170.  Make ArrayIterator and StringIterator next() methods work even with cross-compartment wrappers for those objects as this values.  r=waldo

Not sure if bug 1111170 is really at fault here though - this seems OOM-related and so may be a red herring. Setting needinfo? from our JIT folks, hopefully someone may be able to reproduce.
Flags: needinfo?(jdemooij)
Flags: needinfo?(hv1989)
(Reporter)

Comment 1

3 years ago
Created attachment 8539339 [details]
stack

(lldb) bt 5
* thread #1: tid = 0x27c68, 0x000000010028f6fe js-dbg-opt-64-dm-nsprBuild-darwin-1427b365cd39`JSScript::setBaselineScript(JSContext*, js::jit::BaselineScript*) [inlined] JSScript::hasBaselineScript(this=<unavailable>) const + 8 at jsscript.h:1351, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010028f6fe js-dbg-opt-64-dm-nsprBuild-darwin-1427b365cd39`JSScript::setBaselineScript(JSContext*, js::jit::BaselineScript*) [inlined] JSScript::hasBaselineScript(this=<unavailable>) const + 8 at jsscript.h:1351
    frame #1: 0x000000010028f6f6 js-dbg-opt-64-dm-nsprBuild-darwin-1427b365cd39`JSScript::setBaselineScript(this=<unavailable>, maybecx=<unavailable>, baselineScript=<unavailable>) + 294 at jsscriptinlines.h:170
    frame #2: 0x000000010024dcc9 js-dbg-opt-64-dm-nsprBuild-darwin-1427b365cd39`js::jit::FinishDiscardBaselineScript(fop=0x0000000102027d88, script=0x00000001048870d0) + 185 at BaselineJIT.cpp:1000
    frame #3: 0x000000010063c921 js-dbg-opt-64-dm-nsprBuild-darwin-1427b365cd39`UpdateExecutionObservabilityOfScriptsInZone(cx=<unavailable>, zone=<unavailable>, obs=<unavailable>, observing=<unavailable>) + 1185 at Debugger.cpp:1942
    frame #4: 0x000000010063c3f9 js-dbg-opt-64-dm-nsprBuild-darwin-1427b365cd39`js::Debugger::updateExecutionObservabilityOfScripts(cx=0x0000000101c01e30, obs=0x00007fff5fbfa5e8, observing=Observing) + 233 at Debugger.cpp:1957
(lldb)
Based on the stack and testcase, I think it's a debugger issue.
Flags: needinfo?(shu)
Flags: needinfo?(jdemooij)
Flags: needinfo?(hv1989)
(Assignee)

Comment 3

3 years ago
Yeah I'll take a look.
Assignee: nobody → shu
Flags: needinfo?(shu)
(Assignee)

Comment 4

3 years ago
Created attachment 8539646 [details] [diff] [review]
Don't try to ensure Debugger visibility of about-to-be-finalized scripts.

The bug is that when trying to ensure a compartment's scripts are visible to
the Debugger (by recompiling their JIT code), we use a ZoneCellIter to find all
the scripts. Apparently sweeping can be incremental and we can pick out a
script that is about to be finalized.

This particular crash is because at the beginning of the sweep phase we sweep
the TypeZone and pruning dead CompilerOutputs. This confuses the
recompile-for-Debugger logic, which depends on all scripts it finds with JIT
code having a valid CompilerOutput.

I'm not sure if this patch is the right fix, but it does fix the crash. I've
never seen IsThingAboutToBeFinalized called outside of a sweep function, but
nothing in its impl suggests that it can't.

Terrence, what do you think?
Attachment #8539646 - Flags: review?(terrence)
Comment on attachment 8539646 [details] [diff] [review]
Don't try to ensure Debugger visibility of about-to-be-finalized scripts.

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

Yes, I think that will work and is the right short-term fix. It isn't any worse than what's already there: we at least make API-level promises about the sanctity of the mark bits between GC, so those should be at least as reliable as properties of the CellIter. Use of a CellIter anywhere that isn't sweeping is terrifying on multiple leves, so it would be super awesome if (1) we had not ever exposed the functionality, even in an inlines.h file, and (2) if we had not architected everything to depend on full-heap traversals all the time. I don't see that changing in the short term, however.
Attachment #8539646 - Flags: review?(terrence) → review+
(Assignee)

Updated

3 years ago
Blocks: 1114757
Fixed for Fx36 by the roll-up in bug 1114757.
status-firefox36: --- → fixed
Flags: in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/3f08b1d1f293
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
(Reporter)

Comment 8

3 years ago
gczeal(10);
b1 = "";
b2 = "";
eval(`
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    try {} catch (e) {}
    function compareSource() {
        replace(/[]/g, '').
        replace(/[]/g, '');
        try {} catch (e) {}
    }
    function a1() {}
    function a2() {
        options().split(',');
    }
    a2();
`)
// Randomly chosen test: js/src/jit-test/tests/closures/set-outer-trace-4.js
try {
    evaluate(`
        function f() {}
    `, {
        compileAndGo: true
    })
} catch (e) {}
// Randomly chosen test: js/src/jit-test/tests/baseline/bug836742.js
eval(`
    var g = newGlobal();
    g.debuggeeGlobal = this;
    g.eval("(" + function() {
        var dbg = Debugger(debuggeeGlobal);
        dbg.onEnterFrame = function(frame) {};
        dbg.onExceptionUnwind = function(frame) {}
    } + ")()");
    function f(n) {}
    try {} catch (e) {}
`)
// jsfunfuzz
function r() {}
var s = [
    function() {}
]
var e = ([{}, , ])
function r() {
    (function() {})
}
r([])
var a = "" ? function() {} : function() {}

asserts js debug shell on m-c changeset 1427b365cd39 with --fuzzing-safe --no-threads --ion-eager at Assertion failure: !hasIonScript(), at jsscriptinlines.h.
Whiteboard: [jsbugmon:update,testComment=8,origRev=1427b365cd39]
You need to log in before you can comment on or make changes to this bug.