Closed Bug 732861 Opened 12 years ago Closed 12 years ago

IonMonkey: Assertion failure: JSOp(*bodyStart) == JSOP_LOOPHEAD, at hg-archive/js/src/ion/IonBuilder.cpp:1805

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86_64
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: decoder, Assigned: nbp)

References

Details

(Keywords: assertion, testcase)

Attachments

(1 file)

The following testcase crashes on ionmonkey revision 1fd6c40d3852 (run with --ion -n -m):


j = 0;
out1:
for ((exception) = 0; ; j++)
    if (j == 50)
        break out1;
while (dbgeval("[]").proto ? g : 50) {}
Crash trace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000770154 in js::ion::MBasicBlock::id (this=0x0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/MIRGraph.h:243
243             return id_;
(gdb) bt
#0  0x0000000000770154 in js::ion::MBasicBlock::id (this=0x0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/MIRGraph.h:243
#1  0x000000000076dc6a in IntersectDominators (block1=0xce26c0, block2=0xce24b0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/IonAnalysis.cpp:541
#2  0x000000000076de48 in ComputeImmediateDominators (graph=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/IonAnalysis.cpp:590
#3  0x000000000076dff5 in js::ion::BuildDominatorTree (graph=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/IonAnalysis.cpp:618
#4  0x0000000000760698 in TestCompiler (builder=..., graph=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:653
#5  0x0000000000760d79 in IonCompile (cx=0xcd5d00, script=0x7ffff09071f0, fp=0x7ffff0beb030, osrPc=0xcdfa66  <incomplete sequence \344\232>)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:761
#6  0x0000000000761478 in js::ion::Compile (cx=0xcd5d00, script=0x7ffff09071f0, fp=0x7ffff0beb030, osrPc=0xcdfa66  <incomplete sequence \344\232>)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:896
#7  0x00000000007612b0 in js::ion::CanEnterAtBranch (cx=0xcd5d00, script=0x7ffff09071f0, fp=0x7ffff0beb030, pc=0xcdfa66  <incomplete sequence \344\232>)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:857
#8  0x0000000000504d33 in js::Interpret (cx=0xcd5d00, entryFrame=0x7ffff0beb030, interpMode=js::JSINTERP_NORMAL)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:1771
This is what I've found atm:

- fails on --ion -n too
- is related to ReorderBlocks

I looked to the generated Reversed Post Order of this graph.
There are different ways to construct the RPO (by switching the order of traversing the predecessors). But in this case a not valid RPO gets made.

(Block number 4 and 5 should get switched or block 5 should have the highest number.)

In ReorderBlock there is some logic for loopheaders and loopbackedges. I would pinpoint the fault to that. Because if I set this logic in comments the order is correct. And the testcase also works.
Now there is a reason for this code. So removing this code, will probably introduce other bugs. Today I have no time to look into it further. But I can look further into it tomorrow. But feel free to take this bug. (I remember jandem fixing some bugs here? So his expertise could probably solve this faster?)
Hannes, thanks for looking into this! I'm pretty sure this is a duplicate of bug 732120, we should retest when that bug lands.
Depends on: 732120
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
I tried to reproduce it locally with the last tip of ionmonkey.  I can get a segfault but it does not correspond to the one reported in this bug.  The test case ion/bug729814.js looks similar and raise the same assertion as the test case reported in this bug.

The assertion I got is also recently reported by my buildfarm as:

Assertion failure: JSOp(*bodyStart) == JSOP_LOOPHEAD, at hg-archive/js/src/ion/IonBuilder.cpp:1805

The reason seems to be related to the condition GOTO, which before was not-generated in case of condition-less for-loop and is now replaced by a NOP.
Attachment #604273 - Flags: review?(jdemooij)
Crash Signature: [@ js::ion::MBasicBlock::id]
Keywords: crashassertion
Summary: IonMonkey: Crash [@ js::ion::MBasicBlock::id] → IonMonkey: Assertion failure: JSOp(*bodyStart) == JSOP_LOOPHEAD, at hg-archive/js/src/ion/IonBuilder.cpp:1805
Replacing the if by an assertion seems to work fine and does not break any test.
Comment on attachment 604273 [details] [diff] [review]
Fix layout assumption of condition-less for-loop.

Review of attachment 604273 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with JS_ASSERT instead of |if|.

FWIW, |hg blame| points to http://hg.mozilla.org/mozilla-central/rev/54226ef0199e
Attachment #604273 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/bf54419b67a5
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.