Closed Bug 956166 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox29 --- affected
firefox30 --- fixed

People

(Reporter: gkw, Assigned: wingo)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(2 files)

Attached file 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?)
Flags: needinfo?(jdemooij)
Attached file 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>
Crash Signature: [@ searchLinear] [@ js::jit::IonBuilder::traverseBytecode]
Summary: Assertion failure: isNewObject(), at jit/MIR.h → Crash [@ searchLinear] or [@ js::jit::IonBuilder::traverseBytecode] or Assertion failure: isNewObject(), at jit/MIR.h
Duplicate of this bug: 957028
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.
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?
Flags: needinfo?(jdemooij)
Flags: needinfo?(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.
Flags: needinfo?(wingo)
Depends on: 962599
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.
Depends on: 980263
Crash Signature: [@ searchLinear] [@ js::jit::IonBuilder::traverseBytecode] → [@ searchLinear] [@ js::jit::IonBuilder::traverseBytecode]
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 15df39c10a17).
=== 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!
Assignee: nobody → wingo
Status: NEW → RESOLVED
Crash Signature: [@ searchLinear] [@ js::jit::IonBuilder::traverseBytecode] → [@ searchLinear] [@ js::jit::IonBuilder::traverseBytecode]
Closed: 6 years ago
Flags: needinfo?(wingo)
Resolution: --- → FIXED
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update]
Target Milestone: --- → mozilla30
Flags: in-testsuite+
Thanks for finding the bug, Gary!  I've filed a followup for Aurora: bug 981522.  Cheers :)
Flags: needinfo?(wingo)
You need to log in before you can comment on or make changes to this bug.