633828 - "Assertion failure: !pn->isOp(JSOP_NOP),"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

(function() {
  function a() {}
  function a() {}
}
for

asserts js debug shell on TM changeset 8c7a2550e761 without -m nor -j at Assertion failure: pn->pn_op != JSOP_NOP


autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   27012:2cf0bbe3772a
parent:      26975:7d681f116714
user:        Brendan Eich
date:        Sun Apr 05 21:17:22 2009 -0700
summary:     upvar2, aka the big one take 2 (452498, r=mrbkap).

Comment 1

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

Attachment: corrected stack

previous stack was incomplete :(

Comment 2

[Mike Shaver (:shaver emeritus)]

If it was a regression in *2009*, we're not going to block on it for FF4.

Comment 3

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

(In reply to comment #2)
> If it was a regression in *2009*, we're not going to block on it for FF4.

Having .x would be nice.

Comment 5

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

Still occurs in m-c changeset 5c8405e6226e which has now morphed to:

Assertion failure: !pn->isOp(JSOP_NOP),

Comment 6

[Tom S. (please needinfo tschuster)]

I looked into this and this assert seems to be too strict. MakeDefIntoUse checks that this kind of function statments are NOP.

Comment 7

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

(In reply to Tom Schuster (evilpie) from comment #6)
> I looked into this and this assert seems to be too strict. MakeDefIntoUse
> checks that this kind of function statments are NOP.

Any hope of a quick patch? ;-)

Comment 8

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

Attachment: Remove bogus assert

Luke, asking for review here since this somewhat looks like code you recently touched, please feel free to switch reviewers if not correct.

Comment 9

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

I based the patch on evilpie's analysis in comment 6.

Comment 10

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

Attachment: same as v1, now with a a test

Comment 11

[Luke Wagner [:luke]]

Comment on attachment 644596 [details] [diff] [review]
same as v1, now with a a test

That's right, function definitions get a NOP.

Comment 12

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

Fantastic, thanks for the quick review!

http://hg.mozilla.org/integration/mozilla-inbound/rev/e9e2767a4275

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

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

-> VERIFIED based on landed test.

Comment 15

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

Test was nitpicked in preparation for the landing of another test in another bug.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8758f47b0175

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8758f47b0175