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)

Other Branch
x86_64
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: decoder, Assigned: dvander)

References

Details

(Keywords: assertion, testcase)

Attachments

(1 file)

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;
    }
  }
}
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.
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: general → dvander
Status: NEW → ASSIGNED
Attached patch fixSplinter Review
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 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+
pushed w/ nits fixed: http://hg.mozilla.org/projects/ionmonkey/rev/dce272d6ef91
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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.