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

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

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.

Comment 1

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

Attachment: 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)

Comment 2

[Jan de Mooij [:jandem]]

Based on the stack and testcase, I think it's a debugger issue.

Comment 3

[Shu-yu Guo [:shu]]

Yeah I'll take a look.

Comment 4

[Shu-yu Guo [:shu]]

Attachment: 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?

Comment 5

[Terrence Cole [: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.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Fixed for Fx36 by the roll-up in bug 1114757.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3f08b1d1f293

Comment 8

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

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.