Last Comment Bug 580200 - "Assertion failure: !p,"
: "Assertion failure: !p,"
Status: RESOLVED FIXED
fixed-in-tracemonkey
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- critical (vote)
: mozilla2.0b3
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: jsfunfuzz 518663
  Show dependency treegraph
 
Reported: 2010-07-20 02:15 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-01-19 14:33 PST (History)
10 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta3+


Attachments
Two bugs for the price of one! (2.85 KB, patch)
2010-07-20 11:05 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
jorendorff: review+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2010-07-20 02:15:37 PDT
(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] 2010-07-20 02:41:53 PDT
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] 2010-07-20 02:52:51 PDT
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] (remove +bmo to email) 2010-07-20 08:42:03 PDT
var x =
  Proxy.create({ enumerateOwn: function() Object.getOwnPropertyDescriptor },
               []);
uneval(this);
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2010-07-20 09:12:44 PDT
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] (remove +bmo to email) 2010-07-20 11:05:48 PDT
Created attachment 458722 [details] [diff] [review]
Two bugs for the price of one!
Comment 6 Jason Orendorff [:jorendorff] 2010-07-22 08:15:15 PDT
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 2010-07-22 11:20:39 PDT
Tom, Mark, should the spec say something about this?
Comment 8 Tom Van Cutsem 2010-07-23 14:02:44 PDT
What assertion are we talking about? How do the above examples behave right now?
Comment 9 Jeff Walden [:Waldo] (remove +bmo to email) 2010-07-23 14:06:43 PDT
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] (remove +bmo to email) 2010-07-23 14:54:39 PDT
http://hg.mozilla.org/tracemonkey/rev/40baa240265e
Comment 11 Tom Van Cutsem 2010-07-23 15:45:20 PDT
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:] 2010-07-24 21:10:32 PDT
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] (remove +bmo to email) 2010-07-25 01:01:24 PDT
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:] 2010-07-25 01:33:20 PDT
Thanks Waldo. Filed Bug 581776
Comment 16 Christian Holler (:decoder) 2013-01-19 14:33:50 PST
Automatically extracted testcase for this bug was committed:

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

Note You need to log in before you can comment on or make changes to this bug.