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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Assigned: nbp)
References
Details
(Keywords: assertion, testcase)
Attachments
(1 file)
1.77 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
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) {}
Reporter | ||
Comment 1•12 years ago
|
||
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
Comment 2•12 years ago
|
||
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?)
Comment 3•12 years ago
|
||
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 | ||
Updated•12 years ago
|
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•12 years ago
|
||
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)
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 5•12 years ago
|
||
Replacing the if by an assertion seems to work fine and does not break any test.
Comment 6•12 years ago
|
||
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+
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/projects/ionmonkey/rev/bf54419b67a5
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 8•11 years ago
|
||
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.
Description
•