"Assertion failure: !p,"

RESOLVED FIXED in mozilla2.0b3

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: gkw, Assigned: Waldo)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla2.0b3
x86_64
Linux
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 beta3+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
(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
(Reporter)

Comment 1

7 years ago
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...
(Reporter)

Comment 2

7 years ago
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
Blocks: 518663
var x =
  Proxy.create({ enumerateOwn: function() Object.getOwnPropertyDescriptor },
               []);
uneval(this);
Assignee: general → jwalden+bmo
Status: NEW → ASSIGNED
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?
Created attachment 458722 [details] [diff] [review]
Two bugs for the price of one!
Attachment #458722 - Flags: review?(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.
Attachment #458722 - Flags: review?(jorendorff) → review+

Comment 7

7 years ago
Tom, Mark, should the spec say something about this?

Comment 8

7 years ago
What assertion are we talking about? How do the above examples behave right now?
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?
http://hg.mozilla.org/tracemonkey/rev/40baa240265e
Whiteboard: fixed-in-tracemonkey
Target Milestone: --- → mozilla2.0b3

Comment 11

7 years ago
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

7 years ago
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?
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

7 years ago
Thanks Waldo. Filed Bug 581776

Comment 15

7 years ago
http://hg.mozilla.org/mozilla-central/rev/40baa240265e
Status: ASSIGNED → RESOLVED
blocking2.0: ? → beta3+
Last Resolved: 7 years ago
Resolution: --- → FIXED
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.