561279 - TM: "Assertion failure: obj->getPrivate() == fp, at ../jstracer.cpp" or "Assertion failure: obj->getPrivate() == js_FloatingFrameIfGenerator(cx, fp), at ../jstracer.cpp"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

for (let y in eval("\
    for (z = 0; z < 6; z++) {\
        a = z\
    }\
"))(y)

asserts js debug shell on TM tip with -j at Assertion failure: obj->getPrivate() == fp, at ../jstracer.cpp:13732

Found via the compareJIT part of jsfunfuzz.

Comment 1

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

autoBisect shows this is probably related to bug 469237:

The first bad revision is:
changeset:   27482:ca40d9bb0954
user:        jimb
date:        Tue Apr 21 22:42:43 2009 -0700
summary:     Bug 469237: Only trace where BINDNAME will choose the global object.

Comment 2

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

This affects 1.9.1, 1.9.2 and TM tip js shells. Nominating to get it on the radar.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Wanted, not blocking if it's only in a debug shell.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

As far as I can tell, this is a bug in the assertion: it assumes that there can only be one frame for a toplevel script. But eval and debugger frames defeat this assumption. The values are still on the stack, so this just widens the assertion.

Comment 5

[Jason Orendorff [:jorendorff]]

Comment on attachment 444198 [details] [diff] [review]
Proposed fix

That needs to be a while loop instead of an if, because...

{
    let y;
    eval('eval("for (z = 0; z < 6; z++) a = z;")');
}

And this needs tests. r+ with those changes.

Comment 6

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

This has now morphed into the following assertion:

Assertion failure: obj->getPrivate() == js_FloatingFrameIfGenerator(cx, fp), at ../jstracer.cpp:13885

Comment 7

[Damon Sicore (:damons)]

Doesn't block beta 1.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/769cce825017

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/74e8869eb66a

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/769cce825017

Comment 11

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

A type of test for this bug has already been landed because it is already marked in-testsuite+ -> VERIFIED.