"Assertion failure: !((attrs ^ shape->attrs) & 0x40) || !(attrs & 0x40),"

VERIFIED FIXED

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
7 years ago
5 years ago

People

(Reporter: jandem, Unassigned)

Tracking

({assertion, regression, testcase})

Trunk
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 .x+)

Details

(Whiteboard: js-triage-done)

Attachments

(2 attachments)

(Reporter)

Description

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

Updated

7 years ago
Severity: normal → critical
status2.0: --- → ?
(Reporter)

Comment 1

7 years ago
Created attachment 511003 [details]
Stacktrace
(Reporter)

Updated

7 years ago
blocking2.0: --- → ?
status2.0: ? → ---
blocking2.0: ? → .x
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
Blocks: 514581
Found this as well, I have a test case without "use strict" if that is relevant :)
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.
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 () {
    f1()
    f5(o6)
    o5 = wrap(f1)
})();
f4(o5)
Comment 5 reduced:

obj = wrap(Number.bind());
Object.defineProperty(obj, "caller", {set: function () {}});
Whiteboard: js-triage-needed
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?
Whiteboard: js-triage-needed → js-triage-done
(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),
Keywords: regression
OS: Mac OS X → All
Summary: Assertion failure: !((attrs ^ shape->attrs) & JSPROP_SHARED) || !(attrs & JSPROP_SHARED), at ../jsscope.cpp:1075 → "Assertion failure: !((attrs ^ shape->attrs) & 0x40) || !(attrs & 0x40),"
Version: unspecified → Trunk
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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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...
Attachment #636000 - Flags: review?(jorendorff)
Attachment #636000 - Flags: review?(jorendorff) → review+
Tests landed:

http://hg.mozilla.org/integration/mozilla-inbound/rev/df2e726ece1f
Flags: in-testsuite+
Test backed out, thanks Luke for pointing this out:

http://hg.mozilla.org/integration/mozilla-inbound/rev/627c49f46421

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'"
Flags: in-testsuite+
Landing take two:

http://hg.mozilla.org/integration/mozilla-inbound/rev/172ffbd87b1b

In the jit-test directory, I ran:

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

and they passed. \o/

Thanks jorendorff for helping me out on IRC.
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/172ffbd87b1b
VERIFIED based on landed test.
Status: RESOLVED → VERIFIED
Updated tests to use test metalines instead, since they are in jit-test:

http://hg.mozilla.org/integration/mozilla-inbound/rev/3bca687c261c
https://hg.mozilla.org/mozilla-central/rev/3bca687c261c
You need to log in before you can comment on or make changes to this bug.