498395 - "Assertion failure: !(pn->pn_dflags & flag), at ../jsparse.h"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

eval("\
  function(){\
    for(\
      let[x]=[]\
      (\"\",function(){ x=[] })\
      in w\
    )\
  l}\
")

Assertion failure: !(pn->pn_dflags & flag), at ../jsparse.h:661

$ hg identify && hg log | head
a08e2e4571d6 tip
changeset:   25972:a08e2e4571d6

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]]

The eval is irrelevant:

function f() {
    for (let x = function(){ x = 0; } in w)
        ;
}

Assertion failure: !(pn->pn_dflags & flag), at ../jsparse.h:661

Comment 3

[Robert Sayre]

if this requires let, it's not a blocker

Comment 4

[Jason Orendorff [:jorendorff]]

Thanks sayrer.

Waldo and I are looking at this.  It involves RebindLets, which adjusts use-lists, apparently causing a definition to have an assignment in its use-list without setting the PND_ASSIGNED flag on that assignment.

If that's all there is to this bug, I can fix, but I don't know why this code is doing what it's doing, or if other LinkUseToDef users might have the same bug.

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: v1

I'm fairly confident that this much, at least, is needed, and it does fix the bug narrowly defined.

See JSDefinition::isFunArg and JSDefinition::isAssigned, both of which call test(), which asserts that if the definition lacks the PND_FUNARG/PND_ASSIGNED flag, then no use has it.

I don't know who should review this patch other than Brendan.

Comment 6

[Brendan Eich [:brendan]]

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

Blake reviewed the patch that introduced this code, so he's a candidate. But I'm in and out like lightning today.

There are three other places that preserve ASSIGNED | FUNARG in that order, so please use it here. There ought to be a method to common this expression, but that can wait for a followup bug (feel free to file; I'll do it when time allows otherwise).

r=me with this reordering of | terms. Thanks,

/be

Comment 7

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/0ae666deeea2

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/0ae666deeea2