580200 - "Assertion failure: !p,"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

(function () {
    x = Proxy.create((function () {
        return {
            enumerateOwn: function () Object.getOwnPropertyDescriptor
        }
    })(), [])
})()(uneval(this))

asserts js debug shell on TM tip without -j at Assertion failure: !p, at ../jsiter.cpp:209

Comment 1

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

For TM:

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   47305:22b0c9175913
parent:      46587:281a4b6bb032
parent:      47304:f9e9f951c0ab
user:        Robert Sayre
date:        Fri Jul 02 17:25:52 2010 -0700
summary:     Merge mozilla-central to tracemonkey.


Let me see if I can further bisect on mozilla-central...

Comment 2

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

autoBisect shows this is probably related to the following changeset on m-c:

The first bad revision is:
changeset:   47569:f6e0fbe936bd
user:        Jeff Walden
date:        Thu Sep 24 14:33:14 2009 -0700
summary:     Bug 518663 - ES5: Object.getOwnPropertyNames.  r=jorendorff

Comment 3

[Jeff Walden [:Waldo]]

var x =
  Proxy.create({ enumerateOwn: function() Object.getOwnPropertyDescriptor },
               []);
uneval(this);

Comment 4

[Jeff Walden [:Waldo]]

var x = Proxy.create({ enumerateOwn: function() { return ["0","0"]; } }, []);
Object.getOwnPropertyNames(x);

I guess the assertion in question simply isn't valid here, for proxies.  Maybe time to specialize Enumerate even further?

Comment 5

[Jeff Walden [:Waldo]]

Attachment: Two bugs for the price of one!

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 458722 [details] [diff] [review]
Two bugs for the price of one!

I'm not sure how those Proxy examples ought to behave. Certainly either what you did (which seems fine) or throw an exception. The proposal
  http://wiki.ecmascript.org/doku.php?id=harmony:proxies_semantics
doesn't seem to mention enumerateOwn or Object.getOwnPropertyNames anywhere.

But r=me regardless, as this seems to be a strict improvement.

Comment 7

[Andreas Gal :gal]

Tom, Mark, should the spec say something about this?

Comment 8

[Tom Van Cutsem]

What assertion are we talking about? How do the above examples behave right now?

Comment 9

[Jeff Walden [:Waldo]]

This shows behavior after this patch lands:

https://bugzilla.mozilla.org/attachment.cgi?id=458722&action=diff#a/js/src/tests/js1_8_5/extensions/proxy-enumerateOwn-duplicates.js_sec1

Before landing, the first case would hit an assertion in debug builds and ignore the duplicated name in optimized  builds.  The second case would return the duplicated name twice (seems a clear bad result to me).

The question: should a duplicated name in the array returned by the enumerateOwn hook be ignored (as in this patch), or should such a bad array be treated as an error case, maybe by throwing an exception?

Comment 10

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/40baa240265e

Comment 11

[Tom Van Cutsem]

I just talked this through with MarkM: we would recommend throwing an exception if the array contains duplicate property names.

More generally, the issue here is: what should a proxy implementation do when the handler returns inconsistent data: normalize or throw? There's another case where this occurs, which I'll document shortly under the "Open Issues" section of the proxies proposal. Our recommendation is throwing, but it's definitely something to be discussed at the upcoming TC39 meeting.

In response to Comment 6: I updated the proxies wiki pages such that "enumerateOwn" is now consistently renamed to "keys", and I added the semantics for "Object.getOwnPropertyNames(proxy)".

Comment 12

[Bob Clary [:bc] (inactive)]

See bug 579551 comment 6.

With a fresh tracemonkey build, https://home.eease.com/recruit/?id=496990 still asserts on Mac OS X 105. intel (at least) with 

Assertion failure: !p, at /work/mozilla/builds/2.0.0-tracemonkey/mozilla/js/src/jsiter.cpp:209

New bug?

Comment 13

[Jeff Walden [:Waldo]]

Yes, new bug please -- looks like it might have something to do with an XPConnect enumerate implementation, or something, quite different from the proxy-specific bugs exposed here.

Comment 14

[Bob Clary [:bc] (inactive)]

Thanks Waldo. Filed Bug 581776

Comment 15

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/40baa240265e

Comment 16

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

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