(function() { for (var a = 0; a < 9; a++) { if (c) { while (arguments[2])(function() {}) } } })() asserts js debug shell on IonMonkey changeset 23a84dbb258f with --ion-eager and -a at Assertion failure: false (unexpected constant type), Looking at the stack it seems to be accessing a weird address 0x13dfe0 so locking s-s.

This and bug 776748 are fuzzblockers - they should be the ones that create lots of dupes.

autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 98007:b02a7b214e49 user: Nicolas Pierron date: Fri Jun 15 07:37:06 2012 -0700 summary: Handle arguments[i] (Bug 735406 part 3, r=dvander)

The problem is caused by an MPhi which looks like x MPhi a x where a is the MConstant definition of the magic argument value which has to be captured inside the inner loop and which is created at the beginning. This assertion failure is a substitute to a compilation failure on release builds.

Luke has a demo which hits the same problem. Here's a reduced test case FWIW: function f() { while (true) { while (true) { arguments[0]; } } } f(1); (In reply to Nicolas B. Pierron [:pierron] from comment #3) > > This assertion failure is a substitute to a compilation failure on release > builds. No in release builds JS_NOT_REACHED calls abort() so this will crash..

Check for aliasing of Phi while eliminating Phis.

Use the Unused and the InWorklist flags to mark all used Phis while removing the redundant Phi not detected during the first traversal.

Comment on attachment 647693 [details] [diff] [review] Remove redundant MPhi created by the removal of redundant MPhi. Review of attachment 647693 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/IonAnalysis.cpp @@ +143,5 @@ > + // The removal of Phis can produce newly redundant phis. > + if (MDefinition *redundant = IsPhiRedundant(phi)) { > + phi->replaceAllUsesWith(redundant); > + if (redundant->isPhi() && !redundant->isUnused()) > + redundant->setUnused(); nit: you can remove the condition and use setUnusedUnchecked(). Is it safe to do this at all because we're guaranteed to enqueue the replacement below? @@ +396,5 @@ > for (ReversePostorderIterator block(graph.rpoBegin()); block != graph.rpoEnd(); block++) { > for (MPhiIterator phi(block->phisBegin()); phi != block->phisEnd();) { > if (phi->type() <= MIRType_Null) { > replaceRedundantPhi(*phi); > + phi->isUnused(); What is this call to phi->isUnused?

(In reply to David Anderson [:dvander] from comment #7) > Comment on attachment 647693 [details] [diff] [review] > Remove redundant MPhi created by the removal of redundant MPhi. > > Review of attachment 647693 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/ion/IonAnalysis.cpp > @@ +143,5 @@ > > + // The removal of Phis can produce newly redundant phis. > > + if (MDefinition *redundant = IsPhiRedundant(phi)) { > > + phi->replaceAllUsesWith(redundant); > > + if (redundant->isPhi() && !redundant->isUnused()) > > + redundant->setUnused(); > > nit: you can remove the condition and use setUnusedUnchecked(). Is it safe > to do this at all because we're guaranteed to enqueue the replacement below? Exactly, the following loop will check that we can enqueue the redundant element in the list. > @@ +396,5 @@ > > for (ReversePostorderIterator block(graph.rpoBegin()); block != graph.rpoEnd(); block++) { > > for (MPhiIterator phi(block->phisBegin()); phi != block->phisEnd();) { > > if (phi->type() <= MIRType_Null) { > > replaceRedundantPhi(*phi); > > + phi->isUnused(); > > What is this call to phi->isUnused? I will remove it, I think it was suppose to be phi->setUnused() but apparently it does not matter.

https://hg.mozilla.org/projects/ionmonkey/rev/7faf3ada4920 (patch) https://hg.mozilla.org/projects/ionmonkey/rev/2169bca0c9a5 (test case)

Fixed, so it can be opened.

JSBugMon: This bug has been automatically verified fixed.

This is a follow-up patch because the previous one is fixing things a bit by luck because of the order in which Phis are inserted in the queue.

The latest patch is failing for the same reason as reported in the first comment, but with the following test case when ran with --ion-eager: var SECTION = "lexical-015"; (function() { A: for (var a = 0; a < 9; a++) { B: for (var b = 0; b < a; b++) { while (SECTION[a - b]) { if (arguments[a - b]) break B; else continue A; } } } })() This Bug concern the removal MPhi with redundant operands, as opposed as Bug 779818 which also solve this issue but by working around the MPhi redundancy detection. A better MPhi redundancy detection will imply adding some graph analysis to detect redundancy at a deeper level than the current implementation, which seems overkill for the current purpose. In the mean time, the follow-up patch remove extra redundant phis and enable the previous test to PASS when Bug 779818 is not applied.

Comment on attachment 648617 [details] [diff] [review] Re-evaluate definitions in which we replaced the phi. https://hg.mozilla.org/projects/ionmonkey/rev/f823c4da0b25

JSBugMon: This bug has been automatically verified fixed.

A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug776687.js.