Closed
Bug 692215
Opened 13 years ago
Closed 13 years ago
IM: Assertion failure: backing.firstCopy == stackPosition_, at ion/MIRGraph.cpp:362
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Assigned: dvander)
Details
(Keywords: assertion, testcase)
Attachments
(1 file)
3.01 KB,
patch
|
cdleary
:
review+
|
Details | Diff | Splinter Review |
The following testcase asserts on ionmonkey revision acf3c1fb7c94 (run with --ion-eager), tested on 64 bit: function test1() { var src = "switch(x) {\n"; for (var i=-1; i<4; i++) { src += (i >= 0) ? src : "default:\n"; } } test1();
Assignee | ||
Comment 1•13 years ago
|
||
We were missing code to handle the case where we set a stack slot, but it's in the middle of a copy list. The copy list would become corrupted. This patch rewrites the nodes around the entry that is about to be overwritten.
Comment 2•13 years ago
|
||
Comment on attachment 566384 [details] [diff] [review] fix Review of attachment 566384 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/MIRGraph.cpp @@ +226,5 @@ > slots_[lowest].copyOf = NotACopy; > slots_[lowest].firstCopy = prev; > + } else if (var.isCopy()) { > + uint32 prev = var.copyOf; > + if (slots_[prev].firstCopy != slot) { I think we would hit this case less if we iterated over the stack positions backwards in the addPredecessor. Probably doesn't matter though. @@ +234,5 @@ > + prev = slots_[prev].nextCopy; > + JS_ASSERT(prev != NotACopy); > + } > + } > + slots_[prev].firstCopy = var.nextCopy; I kind of dislike this because we're abusing the firstCopy/nextCopy union, which is confusing to read. When you take the conditional you're actually clobbering a nextCopy field, and when you don't you're really setting the firstCopy field. Can we do a positive test? Like, uint32 backing = var.copyOf; if (slots_[backing].firstCopy == slot) { slots_[backing].firstCopy = var.nextCopy; } else { // ... iterative thing to set nextCopy on a list member to var.nextCopy... } That way if they were made into non-union members it would still work alright.
Attachment #566384 -
Flags: review?(cdleary) → review+
Assignee | ||
Comment 3•13 years ago
|
||
> That way if they were made into non-union members it would still work alright. Thanks, I think this is clearer too. http://hg.mozilla.org/projects/ionmonkey/rev/c7c0d34dc0d9
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 4•11 years ago
|
||
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug692215.js.
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•