Closed Bug 498395 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: JavaScript Engine, defect)

1.9.1 Branch
x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: gkw, Assigned: jorendorff)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

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
Flags: blocking1.9.1?
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).
Blocks: upvar2
Keywords: regression
The eval is irrelevant:

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

Assertion failure: !(pn->pn_dflags & flag), at ../jsparse.h:661
if this requires let, it's not a blocker
Group: core-security
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
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.
Attached patch v1Splinter Review
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.
Assignee: general → jorendorff
Attachment #383324 - Flags: review?(mrbkap)
Attachment #383324 - Flags: review+
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
Attachment #383324 - Flags: review?(mrbkap) → review+
http://hg.mozilla.org/tracemonkey/rev/0ae666deeea2
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/0ae666deeea2
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Group: core-security
You need to log in before you can comment on or make changes to this bug.