Last Comment Bug 632778 - "Assertion failure: !((attrs ^ shape->attrs) & 0x40) || !(attrs & 0x40),"
: "Assertion failure: !((attrs ^ shape->attrs) & 0x40) || !(attrs & 0x40),"
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
-- critical (vote)
: ---
Assigned To: general
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: 514581 630996
  Show dependency treegraph
Reported: 2011-02-09 04:43 PST by Jan de Mooij [:jandem]
Modified: 2012-07-21 06:21 PDT (History)
10 users (show)
gary: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Stacktrace (3.20 KB, text/plain)
2011-02-09 04:50 PST, Jan de Mooij [:jandem]
no flags Details
tests from testcases in comment 0 and comment 6 (842 bytes, patch)
2012-06-22 19:01 PDT, Gary Kwong [:gkw] [:nth10sd]
jorendorff: review+
Details | Diff | Splinter Review

Description User image Jan de Mooij [:jandem] 2011-02-09 04:43:42 PST
function f() {
    "use strict";
g = wrap(f);
Object.defineProperty(g, "arguments", {set: function(){}});
Asserts in debug builds (interpreter/JM/TM):

Assertion failure: !((attrs ^ shape->attrs) & JSPROP_SHARED) || !(attrs & JSPROP_SHARED), at ../jsscope.cpp:1075
Comment 1 User image Jan de Mooij [:jandem] 2011-02-09 04:50:09 PST
Created attachment 511003 [details]
Comment 2 User image Gary Kwong [:gkw] [:nth10sd] 2011-02-24 18:42:23 PST
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   51090:8acc48c670d5
user:        Jeff Walden
date:        Mon Aug 02 23:52:12 2010 -0700
summary:     Bug 514581 - ES5: fun.caller and fun.arguments must throw when fun is strict-mode code.  r=jimb
Comment 3 User image Christian Holler (:decoder) 2011-03-04 03:19:00 PST
Found this as well, I have a test case without "use strict" if that is relevant :)
Comment 4 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-04 14:37:07 PST
Whether or not it's relevant, please post it.  :-)  If it is relevant, it's more coverage.  If it's not, it's a separate bug we should be sure to fix, although if so perhaps the fix might be better in a new bug.
Comment 5 User image Christian Holler (:decoder) 2011-07-30 11:28:53 PDT
I totally forgot this bug :) Here is the requested test case (tested on mozilla-inbound revision ec66137aed05 with options -j -m):

o5 = Namespace;
function f1(o) o6 = o5;
function f4(o) {
    _var_ = o
    var prop = Object.getOwnPropertyNames(f4)[4];
    Object.defineProperty(_var_, prop, {
        set: function () {},
function f5(o) f1 = o.bind();
(function () {
    o5 = wrap(f1)
Comment 6 User image Jason Orendorff [:jorendorff] 2011-08-01 14:35:38 PDT
Comment 5 reduced:

obj = wrap(Number.bind());
Object.defineProperty(obj, "caller", {set: function () {}});
Comment 7 User image Jason Orendorff [:jorendorff] 2011-08-04 20:22:24 PDT
I can't take this, but I can see what's happening.

Native objects are implemented in two "layers", a low-level physical layer (mostly in jsscope.cpp) and a sort of user-y API layer (mostly in jsobj.cpp). The former asserts things that the latter is supposed to enforce with a runtime check.

Object.defineProperty on a transparent proxy calls JS_DefinePropertyById on the wrapped object. We end up in js::DefineNativeProperty, here:

         * If we are defining a getter whose setter was already defined, or
         * vice versa, finish the job via obj->changeProperty, and refresh the
         * property cache line for (obj, id) to map shape.
        if (!js_LookupProperty(cx, obj, id, &pobj, &prop))
            return NULL;
        if (prop && pobj == obj) {
            shape = (const Shape *) prop;
-->         if (shape->isAccessorDescriptor()) {
                shape = obj->changeProperty(cx, shape, attrs,
                                            JSPROP_GETTER | JSPROP_SETTER,
                                            (attrs & JSPROP_GETTER)
                                            ? getter
                                            : shape->getter(),
                                            (attrs & JSPROP_SETTER)
                                            ? setter
                                            : shape->setter());
                if (!shape)
                    return NULL;
            } else {
                shape = NULL;

The attrs of the function.caller property include JSPROP_GETTER and JSPROP_SETTER but not JSPROP_SHARED. So isAccessorDescriptor() returns true, but when we get down to the physical layer in jsscope.cpp, we assert that we aren't trying to turn a slotful property into a slotless one.

It could be fixed by making this code check for more than just isAccessorDescriptor(), I guess. Maybe Jeff's willing to take this?
Comment 8 User image Gary Kwong [:gkw] [:nth10sd] 2012-04-23 11:58:44 PDT
(In reply to Jason Orendorff [:jorendorff] from comment #6)
> Comment 5 reduced:
> obj = wrap(Number.bind());
> Object.defineProperty(obj, "caller", {set: function () {}});

The assertion caused by this testcase as well as the one in comment 0 has mutated to:

Assertion failure: !((attrs ^ shape->attrs) & 0x40) || !(attrs & 0x40),
Comment 9 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-22 18:56:43 PDT
Bug 750307 probably fixed this. The testcases in comment 0 and comment 6 no longer assert.

autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   96576:2ddb6278d1de
user:        Jason Orendorff
date:        Wed Jun 13 03:11:18 2012 -0500
summary:     Bug 750307 - "Assertion failure: isBoolean()" in RegExpObject::ignoreCase after redefining nonconfigurable data property. r=Waldo. Second landing, test change rs=bholley on IRC.
Comment 10 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-22 19:01:59 PDT
Created attachment 636000 [details] [diff] [review]
tests from testcases in comment 0 and comment 6

I'm not sure if tests need r+ but running them through a second pair of eyes may be a good idea...
Comment 11 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-25 13:39:33 PDT
Tests landed:
Comment 12 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-25 14:12:49 PDT
Test backed out, thanks Luke for pointing this out:

The testcase no longer gives an assert but according to Luke shows a TypeError instead (which I think is correct):

"TypeError: can't redefine non-configurable property 'arguments'"
Comment 13 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-25 14:45:17 PDT
Landing take two:

In the jit-test directory, I ran:

python <path to js binary> bug632778-1.js bug632778-2.js

and they passed. \o/

Thanks jorendorff for helping me out on IRC.
Comment 14 User image Ed Morley [:emorley] 2012-06-26 01:57:47 PDT
Comment 15 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-28 14:08:05 PDT
VERIFIED based on landed test.
Comment 16 User image Gary Kwong [:gkw] [:nth10sd] 2012-07-20 23:05:20 PDT
Updated tests to use test metalines instead, since they are in jit-test:
Comment 17 User image Ryan VanderMeulen [:RyanVM] 2012-07-21 06:21:01 PDT

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