717249 - Assertion failure: !uses->next->next && uses->next->popped && script->code[uses->next->offset] == JSOP_SWAP, at ../jsanalyze.h:1064

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test asserts on mozilla-central revision 4de07a341aab (options -m -n -a):


[].__proto__();

Comment 1

[Jeff Walden [:Waldo]]

If I had to guess, I'm betting this has to do with the JSOP_QNAMEPART fugliness for property accesses for __proto__.  h8

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This has not been fixed on m-c changeset 7c7d2a8db7ff (which has bug 716713 included)

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   84036:7ab4f1ebc7cc
user:        Brian Hackett
date:        Mon Jan 09 06:29:50 2012 -0800
summary:     Backout 54cd89b0f1fa (bug 712714 backout).  Talos will probably report fake regressions for this patch, do not back out for this reason.

Comment 3

[Brian Hackett [Laid off!]]

Attachment: rm fugliness

Yeah, it's the __proto__ special casing.  This bug removes that stuff entirely.  It seems only necessary so that the interpreter does not have to check for __proto__ when doing GetProtoIfDenseArray.  Interpreter optimizations are no longer necessary/justifiable, and while PICs should still be able to do this stuff they can test for __proto__ before generating code.

Comment 4

[Brian Hackett [Laid off!]]

(In reply to Brian Hackett (:bhackett) from comment #3)
> Interpreter optimizations are no longer necessary/justifiable

Overstated this a bit.  Having significant complexity to occasionally save a few cycles while interpreting is not good.

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 587907 [details] [diff] [review]
rm fugliness

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

And there was much rejoicing.

Comment 6

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5cdf9574bede

Comment 7

[Brian Hackett [Laid off!]]

Backed out for not building on any platforms except the ones I tested on.

https://hg.mozilla.org/integration/mozilla-inbound/rev/408ab9247ef1

Comment 9

[Brian Hackett [Laid off!]]

Attachment: weaken assert

Unfortunately the QNAMEPART stuff is also necessary for correct behavior of __proto__ on primitives (which should get e.g. Number.prototype instead of Object.prototype) and there doesn't seem to be an easy/efficient way to fix this.  So here's a patch which weakens the assert so that the goofy SWAP; SWAP sequence of bytecodes emitted here is passed through.

Comment 10

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f2106545d9b3

Comment 11

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/f2106545d9b3

Comment 12

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929