705879 - "Assertion failure: isGlobal(),"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, critical)

Description

[Gary Kwong [:gkw] [:nth10sd]]

Attachment: stack

f = eval("\
  (function() {\
    with({}) {\
      yield\
    }\
    for(let d in[gc()])\
    for(b in[0,function(){}]);\
  })\
")
for (e in f()) {}

asserts js debug shell on JM changeset 5546f57c9567 without any CLI flags at Assertion failure: isGlobal(),

Doesn't seem to occur with m-c changeset bc48009a6bbb.

(not sure if the following is correct):

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   79616:d6352d960dd2
parent:      79349:f951e9151626
parent:      79615:921e1db5cf11
user:        Brian Hackett
date:        Wed Nov 02 09:23:25 2011 -0700
summary:     Merge MC -> JM

Comment 1

[Brian Hackett (:bhackett)]

I can't repro on the named revision and architecture.

Comment 2

[Gary Kwong [:gkw] [:nth10sd]]

(In reply to Brian Hackett (:bhackett) from comment #1)
> I can't repro on the named revision and architecture.

I can definitely still reproduce this on a 32-bit debug shell on JM rev 1e8c03ba91d0 without any CLI flags.

Comment 3

[Gary Kwong [:gkw] [:nth10sd]]

> I can definitely still reproduce this on a 32-bit debug shell on JM rev
> 1e8c03ba91d0 without any CLI flags.

There was some offline discussion about this being possibly platform and configuration-specific and being potentially fixed by bug 692274.

Comment 4

[Gary Kwong [:gkw] [:nth10sd]]

> There was some offline discussion about this being possibly platform and
> configuration-specific and being potentially fixed by bug 692274.

This still reproduces with the patch in comment 35 in bug 692274 applied on m-c rev 7ab478082ca7. Testing m-c because ObjShrink has landed on m-c.

Luke, any ideas on why this still occurs? (when Brian and I last spoke, this doesn't really seem directly related to ObjShrink)

Comment 5

[Luke Wagner [:luke]]

(In reply to Gary Kwong [:gkw, :nth10sd] from comment #4)
> Luke, any ideas on why this still occurs? (when Brian and I last spoke, this
> doesn't really seem directly related to ObjShrink)

Yeah, it's ObjShrink: 'priv' is a stale stack frame stored in the generator object, this code should use the live frame (on the stack), i.e., 'fp'.

Comment 6

[Luke Wagner [:luke]]

Attachment: fix

Comment 7

[Gary Kwong [:gkw] [:nth10sd]]

(In reply to Luke Wagner [:luke] from comment #6)
> Created attachment 579881 [details] [diff] [review]
> fix

(setting assignee to Luke since he has an r+ patch - ready for checkin!)

Comment 9

[Gary Kwong [:gkw] [:nth10sd]]

The testcase in bug 709929 should also be landed.

Comment 10

[Luke Wagner [:luke]]

Righto.  Will land when m-i kerfuffle clears up; sorry for not landing earlier.

Comment 11

[Gary Kwong [:gkw] [:nth10sd]]

(In reply to Gary Kwong [:gkw, :nth10sd] from comment #9)
> The testcase in bug 709929 should also be landed.

Here's another testcase which crashes debug shell that is fixed by the patch here:

f = (function() {
    function shapeyConstructor(qvhvyl) {
        Object.defineProperty(qvhvyl, "", ({
            e: true
        }));
        gc();
    }
    for each(let x in [Number, Number, new Number]) {
        try {
            let g = shapeyConstructor(x);
            with({}) {
                with([]) yield
                }
            } catch (e) {
            }
        }
    })
for (i in f()) {}

Comment 12

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ebfc6414c4dc

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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

Comment 14

[Gary Kwong [:gkw] [:nth10sd]]

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