488963 - "Assertion failure: cg->lexdeps.lookup(atom), at ../jsemit.cpp"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

for(let x in []) {((x = [] for(x in [])) for(y in 0))}

asserts debug js shell without -j at Assertion failure: cg->lexdeps.lookup(atom), at ../jsemit.cpp:2106

Due to morphing of assertion, autoBisect reports this can probably due to any one of:

Due to skipped revisions, the first bad revision could be any of:
changeset:   27205:78a21b8efe1b
user:        Brendan Eich
date:        Wed Apr 15 01:57:13 2009 -0700
summary:     Bug 488015 - Crash [@ js_GetUpvar ] (also bogus JS errors, also probably Crash [@js_Interpret]) (future r=mrbkap, see bug).

changeset:   27206:5d76df9bab84
user:        Igor Bukanov
date:        Wed Apr 15 16:09:58 2009 +0200
summary:     bug 488285 - fixing strict mode warnings with DOm window object. r=brendan

changeset:   27207:65b7d018df1a
user:        Brendan Eich
date:        Wed Apr 15 09:30:46 2009 -0700
summary:     Bug 488475 - "Assertion failure: \!(pn->pn_dflags & flag), at ../jsparse.h" (r=me).

Comment 1

[Brendan Eich [:brendan]]

This is helpful -- generator expression in a for (let...) loop but no RebindLets. It may help with bug 488690, more when I know more.

/be

Comment 2

[Brendan Eich [:brendan]]

Gary, what was (were?) the pre-morphed assertion(s)?

/be

Comment 3

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

(In reply to comment #2)
> Gary, what was (were?) the pre-morphed assertion(s)?
> 
> /be

At changeset 78a21b8efe1b, the assertion is:

$ ./js-dbg-tm-intelmac 
js> for(let x in []) {((x = [] for(x in [])) for(y in 0))}
Assertion failure: !(pn->pn_dflags & flag), at ../jsparse.h:660
Trace/BPT trap

which was apparently fixed in bug 488475. I suppose it morphed to this lexdeps.lookup assertion after that patch.

Comment 4

[Brendan Eich [:brendan]]

Attachment: easy fix

Generator expressions look like

<comp-expr> for ( <left-side> in <iter-expr> )

ignoring optional extra 'for' heads and optional final 'if (cond)'. The entire expression becomes the body of a lambda that yields <comp-expr> from within the for-in loop.

The parser eagerly links uses to defs, so it has to revise the AST once the 'for' is parsed. Doing so may involve splitting a def-use chain. The split must use source coordinates to decide where to split.

Given a <left-side> of 'x', new definition for uses of x within the synthesized lambda needs to start at source coordinates no less than (operator < is defined in jsscan.h, along with the rest, for JSTokenPos and JSTokenPtr) the starting position of the entire expression, wherefore this patch.

/be

Comment 5

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/e0e3d87da9ad

mrbkap is playing hockey (same team as sicking) -- I presume victory on the ice and anticipate r+ here. I meant to fix this in the last jsparse.cpp patch (for bug 488690) but committed before making this fix.

/be

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/e0e3d87da9ad

Comment 7

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2708bfadf83f

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with testcase in comment 0 with the following debug builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090522 Minefield/3.6a1pre ID:20090522133810

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre)
Gecko/20090522 Shiretoko/3.5pre ID:20090522153422

Comment 9

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

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