492714 - "Assertion failure: cg->staticLevel >= level, at ../jsemit.cpp" with genexp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

(function ({x}) x for each (x in y))

asserts debug js shell without -j at Assertion failure: cg->staticLevel >= level, at ../jsemit.cpp:2067

Comment 1

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

autoBisect shows this is probably related to bug 452498 :

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

Comment 2

[Jason Orendorff [:jorendorff]]

I'm not totally confident I know what's going on here, but here is my guess:

   ((function ({x}) x) for (y in z));

When BindDestructuringArg is first called, with atom "x", it assigns 0x10000 to pn->pn_cookie.  CompExprTransplanter calls BumpStaticLevel twice for this node, so UPVAR_FRAME_SKIP(pn->pn_cookie) ends up being 3.

I think this is because the node appears twice in the "tree".

The assertion later happens with level == 3 and cg->staticLevel == 2.

Comment 3

[Jason Orendorff [:jorendorff]]

In fact, unless I'm mistaken, we shouldn't be bumping the static level of this node at all, since it is local to a function that is, itself, being transplanted.

Comment 4

[Brendan Eich [:brendan]]

(In reply to comment #3)
> In fact, unless I'm mistaken, we shouldn't be bumping the static level of this
> node at all, since it is local to a function that is, itself, being
> transplanted.

pn_cookie's skip half is an absolute static level as reckoned by the parser, which sees all statically nested functions. So we need to bump, but you are right we should not double-bump in this case.

Destructuring parameters desugar to injected vars at the front of the function's body.

/be

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: v1

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 377293 [details] [diff] [review]
v1

Great, thanks. Alterna-patch, or followup bug fodder, would use the CloneParseTree function (new with upvar) to make right a clone of left.

/be

Comment 7

[Jason Orendorff [:jorendorff]]

Filed bug 492996.

http://hg.mozilla.org/tracemonkey/rev/7797c629c576

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/7797c629c576

Comment 9

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e29396a250b5