Closed
Bug 729902
Opened 12 years ago
Closed 12 years ago
IonMonkey: Assertion failure: live->empty(), at ion/LinearScan.cpp:670
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Assigned: dvander)
References
Details
(Keywords: assertion, testcase)
Attachments
(1 file)
4.72 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
The following testcase asserts on ionmonkey revision 5a04fd69aa09 (run with --ion -n), tested on 64 bit: var x = 2; outer: while (x < 10) { while (x < 10) { if (x < (null )) continue outer; while (x < 10) { ((function() {}).abstract) = 0; } } }
Assignee | ||
Comment 1•12 years ago
|
||
Jan, this one is an LSRA assert, would you mind taking a look? It appears to work on GRA; gvn/licm aren't related. Usually this assert is from LIR having a bogus use but I didn't see anything obvious.
Comment 2•12 years ago
|
||
Tricky SSA-bug with break/continue. Here's a testcase that fails with GRA too: -- function f() { var i = 0; outer: for (var x = 0; x < 10; x++) { while (true) { if (i > 150) continue outer; i++; } } assertEq(i, 151); } f(); -- $ ./js --ion -n test.js test.js:11: Error: Assertion failed: got 38, expected 151 $ ./js --ion -n --ion-regalloc=greedy test.js test.js:11: Error: Assertion failed: got 38, expected 151 The problem is that the "continue" catch-block inherits from the block with the "continue outer;" statement, but this ignores the phi that was later added to the loop header.
Assignee | ||
Updated•12 years ago
|
Assignee: general → dvander
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•12 years ago
|
||
Thanks for the great analysis, Jan. I'm vaguely horrified by this bug - if this isn't the last of phi placement errors, we might want to think about an eager instead of lazy approach. This patch just forcefully updates the exit slots on dangling continue edges.
Attachment #602552 -
Flags: review?(jdemooij)
Comment 4•12 years ago
|
||
Comment on attachment 602552 [details] [diff] [review] fix Review of attachment 602552 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Can you also add the testcase in comment 2? It does not rely on an LSRA-assert so it's a bit more robust/explicit. ::: js/src/ion/IonBuilder.cpp @@ +1135,5 @@ > > +void > +IonBuilder::fixPendingContinues(MBasicBlock *header) > +{ > + // An edge case (bug 722669). We leave |continue| edges "dangling" in the Nit: bug 729902. Great comment!
Attachment #602552 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 5•12 years ago
|
||
pushed w/ nits fixed: http://hg.mozilla.org/projects/ionmonkey/rev/dce272d6ef91
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 6•11 years ago
|
||
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug729902-1.js.
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•