956166 - Crash [@ searchLinear] or [@ js::jit::IonBuilder::traverseBytecode] or Assertion failure: isNewObject(), at jit/MIR.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: lldb stack

({
    l: [0
        for each(s in 0) if ('')
    ]
})

asserts js debug shell on m-c changeset 25524dc5c99f with --ion-parallel-compile=off --ion-eager --ion-check-range-analysis at Assertion failure: isNewObject(), at jit/MIR.h

My configure flags are:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/eb804b2f1e96
user:        Andy Wingo
date:        Wed Dec 18 14:45:09 2013 +0100
summary:     Bug 942804 - Ion-compile scripts with unaliased let bindings. r=jandem

Jan, is bug 942804 also likely a regressor? (or related to bug 952992?)

Comment 1

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

Attachment: opt stack

This testcase also crashes opt shell at searchLinear with js::jit::IonBuilder::traverseBytecode on the stack - seemingly at null.

Configure parameters for Linux 32-bit:

CC="gcc -m32" AR=ar PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig CXX="g++ -m32" sh ./configure --target=i686-pc-linux --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

Comment 3

[Jan de Mooij [:jandem]]

Annoying bug. The array comprehension is compiled as a loop, so we add phis for everything on the stack. The INITPROP then expects MNewObject on the stack instead of MPhi.

Not sure how to fix. Maybe we can eliminate these redundant phis when we finish the loop. Or we could disable Ion-compilation of array comprehensions for now.

Comment 4

[Jan de Mooij [:jandem]]

Andy, I think we could fix this by storing the let variables in fixed slots or something instead of stack slots, as mentioned somewhere else. Then we can change Ion to not add phis for stack slots when it compiles a loop and everything should work I think. Is that a lot of work?

Comment 5

[Andy Wingo [:wingo]]

Incidentally I was working on that right now, after taking a look at bug 942810 and seeing that it was fairly impossible unless lets were part of the fixed part of the stack.

From what I can tell it's not a big change, but tricky to figure out what exactly needs changing.  The issue is that once you put let-bound vars in the fixed part of the stack, then you lose runtime information about scope nesting.  See js::UnwindScope() in Interpreter.cpp; how to know when to stop unwinding the stack?  Probably the solution involves some change to trynotes.

Comment 6

[Andy Wingo [:wingo]]

I still get this bug even after the patch in bug 962599 is applied.  Humm.  I guess we would need to change to declare a lexical scope with temporaries for the comprehension.

Comment 7

[Christian Holler (:decoder)]

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

Comment 8

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

=== Tinderbox Build Bisection Results by autoBisect ===

The "bad" changeset has the timestamp "20140306234357" and the hash "6a294c3292c3".
The "good" changeset has the timestamp "20140306234557" and the hash "b9fc2eb18bd1".

Likely fix window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6a294c3292c3&tochange=b9fc2eb18bd1

Very likely to be fixed by bug 980263. Testcases were landed there.

Andy, you might want to nominate the patches there for backport to aurora. Thanks for fixing this!

Comment 9

[Andy Wingo [:wingo]]

Thanks for finding the bug, Gary!  I've filed a followup for Aurora: bug 981522.  Cheers :)