1158632 - Assertion failure: exprStack == stackDepth, at jit/Recover.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, critical)

Description

[Gary Kwong [:gkw] [:nth10sd]]

for (var j = 0; j < 1; ++j) {
    (function() {
        function f(x) {
            x = 4294967295 >>> 4294967295 % x
            switch (-1) {
                case 1:
                case -1:
                    x = 0
            }
        }
        return f;
    }())()
}

asserts js debug shell on m-c changeset f214df6ac75f with --fuzzing-safe --no-threads --ion-eager at Assertion failure: exprStack == stackDepth, at jit/Recover.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 --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build" -r f214df6ac75f

Due to skipped revisions, the first bad revision could be any of:
changeset:   https://hg.mozilla.org/mozilla-central/rev/79097ed14588
user:        Nicolas B. Pierron
date:        Fri Oct 03 14:25:59 2014 +0200
summary:     Bug 1072188 - IonMonkey: Split truncate function. r=sunfish

changeset:   https://hg.mozilla.org/mozilla-central/rev/e25f541006dc
user:        Nicolas B. Pierron
date:        Fri Oct 03 14:25:59 2014 +0200
summary:     Bug 1072188 - Recover truncated instruction. r=sunfish

Nicolas, is bug 1072188 a likely regressor?

Comment 1

[Gary Kwong [:gkw] [:nth10sd]]

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x81802c, 0x00000001005ec3be js-dbg-64-dm-nsprBuild-darwin-f214df6ac75f`js::jit::MResumePoint::writeRecoverData(this=0x0000000101eca8d0, writer=<unavailable>) const + 958 at Recover.cpp:107, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001005ec3be js-dbg-64-dm-nsprBuild-darwin-f214df6ac75f`js::jit::MResumePoint::writeRecoverData(this=0x0000000101eca8d0, writer=<unavailable>) const + 958 at Recover.cpp:107
    frame #1: 0x000000010067cb0c js-dbg-64-dm-nsprBuild-darwin-f214df6ac75f`js::jit::CodeGeneratorShared::encode(js::jit::LRecoverInfo*) + 9 at Snapshots.cpp:715
    frame #2: 0x000000010067cb03 js-dbg-64-dm-nsprBuild-darwin-f214df6ac75f`js::jit::CodeGeneratorShared::encode(this=0x0000000101ed5000, recover=0x0000000101ed01c0) + 227 at CodeGenerator-shared.cpp:492
    frame #3: 0x000000010067cd21 js-dbg-64-dm-nsprBuild-darwin-f214df6ac75f`js::jit::CodeGeneratorShared::encode(this=0x0000000101ed5000, snapshot=0x0000000101ed0228) + 49 at CodeGenerator-shared.cpp:506
    frame #4: 0x000000010070b99f js-dbg-64-dm-nsprBuild-darwin-f214df6ac75f`void js::jit::CodeGeneratorX86Shared::bailout<js::jit::BailoutLabel>(this=0x0000000101ed5000, binder=0x00007fff5fbfe4a0, snapshot=0x0000000101ed0228) + 31 at CodeGenerator-x86-shared.cpp:497
(lldb)

Comment 2

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision be81b8d6fae9).

Comment 3

[Gary Kwong [:gkw] [:nth10sd]]

This still reproduces for me with m-c rev ffa83d153080.

Comment 4

[Nicolas B. Pierron [:nbp]]

Attachment: IonBuilder TableSwitch, fill-the-gap cases should bail at the PC of the default block.

When instructions which might fail are added in fill-the-gap blocks, we
expect the be able to encode snapshots.  In this case we were trying to
encode a snapshot which had one extra argument.

The reason is that the switch cases encoded by the bytecode emitter expect
us to pop the tested variable.  Which means that the current scheme used for
creating a new block at the same pc as the end of the previous case
statement is effectively expecting one less local on the stack frame.

This patch changes the creation of the fill-the-gap blocks such that we
create them with the basic block of the default case instead of the basic
block of the end of the previous case.

Note: we cannot use the default case as a successor as the MIRGraph only
expects blocks to appear only once in the list predecessors, and doing so
would effectively register the MTableSwitch block multiple times.

Comment 5

[Hannes Verschore [:h4writer]]

Comment on attachment 8676410 [details] [diff] [review]
IonBuilder TableSwitch, fill-the-gap cases should bail at the PC of the default block.

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

Wow, good catch!

Note: I was totally confused by your commit comment. The comment suggested to me that we would now start bailing when we encountered the 'fill-the-gap' case.
Can you update your commit comment to: 

Bug 1158632 - IonBuilder TableSwitch, fill-the-gap case should encode the PC of the default block. r=

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/105c07e0c6ad

Comment 7

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/105c07e0c6ad