1203791 - Assertion failure: !builder || !hasPendingIonBuilder(), at jit/BaselineJIT.h or Assertion failure: script->ionScript()->isRecompiling(), at jit/CodeGenerator.cpp or Assertion failure: hasBaselineScript(), at jsscript.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

function n(x) {
    parse(x);
}
function m() {
    n();
}
var g = newGlobal();
g.parent = this;
g.eval(`
    var dbg = new Debugger();
    var parentw = dbg.addDebuggee(parent);
    var pw = parentw.makeDebuggeeValue(parent.p);
    var scriptw = pw.script;
`);
g.dbg.onIonCompilation = function(graph) {
    if (graph.scripts[0] != g.scriptw)
        return;
    m();
};
function p() {
    for (var res = false; !res; res = inIon()) {}
}
p();
(function() {})();

asserts js debug shell on m-c changeset 7671701d15ca with --fuzzing-safe --ion-eager at Assertion failure: !builder || !hasPendingIonBuilder(), at jit/BaselineJIT.h . Unreduced variants assert at Assertion failure: script->ionScript()->isRecompiling(), at jit/CodeGenerator.cpp and Assertion failure: hasBaselineScript(), at jsscript.h

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-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build" -r 7671701d15ca

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/938701d3cf0c
user:        Shu-yu Guo
date:        Wed Aug 19 12:02:32 2015 -0700
summary:     Bug 1194430 - Always mark the global jitcode table during major GCs. (r=djvj)

Shu-yu, is bug 1194430 a likely regressor?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x289686, 0x00000001002d4455 js-dbg-64-dm-nsprBuild-darwin-7671701d15ca`js::jit::BaselineScript::setPendingIonBuilder(this=<unavailable>, maybecx=<unavailable>, script=<unavailable>, builder=<unavailable>) + 229 at BaselineJIT.h:475, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001002d4455 js-dbg-64-dm-nsprBuild-darwin-7671701d15ca`js::jit::BaselineScript::setPendingIonBuilder(this=<unavailable>, maybecx=<unavailable>, script=<unavailable>, builder=<unavailable>) + 229 at BaselineJIT.h:475
    frame #1: 0x00000001005a55b7 js-dbg-64-dm-nsprBuild-darwin-7671701d15ca`js::jit::AttachFinishedCompilations(cx=0x000000010284c400) + 599 at Ion.cpp:1898
    frame #2: 0x00000001003025f8 js-dbg-64-dm-nsprBuild-darwin-7671701d15ca`JSRuntime::handleInterrupt(JSContext*) [inlined] InvokeInterruptCallback(cx=0x000000010284c400) + 38 at Runtime.cpp:553
    frame #3: 0x00000001003025d2 js-dbg-64-dm-nsprBuild-darwin-7671701d15ca`JSRuntime::handleInterrupt(this=<unavailable>, cx=0x000000010284c400) + 98 at Runtime.cpp:652
    frame #4: 0x00000001007577bc js-dbg-64-dm-nsprBuild-darwin-7671701d15ca`js::jit::CheckOverRecursedWithExtra(cx=0x000000010284c400, frame=<unavailable>, extra=<unavailable>, earlyCheck=<unavailable>) + 172 at VMFunctions.cpp:166
(lldb)

Comment 2

[Shu-yu Guo [:shu]]

I cannot reproduce. Seems like a timing issue...

Comment 3

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

Tricky bug, here's what happened in LazyLink:

(1) We remove the pending builder from the script.

(2) The Debugger::onIonCompilation call can trigger an off-thread compilation of the same script.

(3) We then called FinishOffThreadBuilder, which messed things up because it got confused by the new in-progress compilation.

This patch changes LazyLink so we call FinishOffThreadBuilder before we call Debugger::onIonCompilation. I also had to make sure OnIonCompilationInfo uses its own LifoAlloc instead of the builder's LifoAlloc, because FinishOffThreadBuilder destroys the builder.

Comment 5

[Hannes Verschore [:h4writer]]

Comment on attachment 8660192 [details] [diff] [review]
Patch

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

::: js/src/jit/Ion.cpp
@@ +415,3 @@
>        : numBlocks(0),
>          scriptIndex(0),
> +        alloc(4096),

Doesn't this mean we will always allocate 4096, even though in the default case we won't use it?

Comment 6

[Jan de Mooij [:jandem]]

(In reply to Hannes Verschore [:h4writer] from comment #5)
> > +        alloc(4096),
> 
> Doesn't this mean we will always allocate 4096, even though in the default
> case we won't use it?

No we'll only allocate when we need it, the LifoAlloc's constructor does not allocate anything.

Comment 7

[Hannes Verschore [:h4writer]]

Comment on attachment 8660192 [details] [diff] [review]
Patch

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

Good find!

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9bdb6d48a34e

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9bdb6d48a34e