650618 - Crash [@ js_ErrorToException] or "Assertion failure: obj->isGlobal()," or "Assertion failure: isGlobal(),"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

print(evalcx("#1=@o"))

asserts js debug shell on TM changeset 242947d76f73 without -m nor -j at Assertion failure: obj->isGlobal(),

Comment 1

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   67971:f2dca3c21175
user:        Luke Wagner
date:        Fri Apr 08 10:52:48 2011 -0700
summary:     Bug 602994 - Preparatory syntactic cleanup (r=waldo)

Comment 2

[Luke Wagner [:luke]]

Pre-existing bug (or not?) caught by new assertions.  The class of the object-with-null-parent-but-not-global is js_AttributeNameClass.  Hey, Waldo is already cc'd!

Comment 3

[Luke Wagner [:luke]]

Which follows from the fact that NewXMLAttributeName passes NULL as parent.  Looks like other XML things too.  Can we just give these suckers JS_GetGlobalForScopeChain as parent or do XML things really want NULL parents?

Comment 4

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

This now asserts at:

Assertion failure: isGlobal(),

Comment 5

[Jeff Walden [:Waldo]]

I'd expect we can give them regular parents like everything else.

Comment 6

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

Comment 0 also crashes js opt shell without any CLI arguments at js_ErrorToException on TM changeset f59568ec0513

Comment 7

[Luke Wagner [:luke]]

Attachment: give xml objects a parent

Oops, let this drop.

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 545404 [details] [diff] [review]
give xml objects a parent

Review of attachment 545404 [details] [diff] [review]:
-----------------------------------------------------------------

It's facially possible for GetGlobalForScopeChain to return null, so add null-checks to the calls you're adding, just in case it's possible that might be hit.  (It doesn't seem productive to actually check whether that possibility is real, just for E4X.)

Comment 9

[Luke Wagner [:luke]]

I don't think that its actually possible for GetGlobalForScopeChain to return NULL at these sites since, even if cx->globalObject is left NULL, entering a compartment (which must be done to be executing any of this code) will push a dummy frame which provides the scope chain.

Comment 10

[(no longer active)]

I took the liberty of backing out this patch from inbound <http://hg.mozilla.org/integration/mozilla-inbound/rev/66fd359c514e> because I seriously question if we're going to remove the DOM Worker support for at least a while!

Comment 11

[Luke Wagner [:luke]]

In my defense, it all looked fine before the pull rebase :/

Comment 12

[(no longer active)]

(In reply to comment #11)
> In my defense, it all looked fine before the pull rebase :/

Yeah, hg rebase still has some bugs, I guess.  Also, you should reland your patch.  :-)

Comment 13

[Luke Wagner [:luke]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/777a3c916936

Comment 14

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/777a3c916936

Comment 15

[Christian Holler (:decoder)]

E4X has been removed, not adding this test.