652177 - JSObject::freezeOrSeal misbehavior with __proto__

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

Function.toLocaleString.__proto__ = null
y = {}.__proto__
y.p = function() {}
y.__defineSetter__("", function() {})
y.__proto__ = Function.toLocaleString
function b(a) {
    this.__proto__ = a;
    Object.freeze(this)
}
for each(z in []) {
    new b
}

asserts js debug shell on TM changeset d5fcfa622f16 without -m nor -j at Assertion failure: shape->writable(), seems to only occur when passed in as a CLI argument.

This was found using a combination of jsfunfuzz and jandem's method fuzzer.

This testcase seems to only be fairly reproducible hence autoBisect isn't yet returning reliable results.

Comment 1

[David Mandelin [:dmandelin]]

It repro'd for me every time on 64-bit linux (without -m and without -j). I bisected to this changeset, which kind of surprised me:

changeset:   62161:f569d49576bb
user:        Bill McCloskey <[e-mail]>
date:        Fri Feb 11 16:31:32 2011 -0800
summary:     Bug 631951 - Shrink methodjit memory usage by interpreting a few times before compiling (r=dvander)

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to comment #1)
> It repro'd for me every time on 64-bit linux (without -m and without -j). I
> bisected to this changeset, which kind of surprised me:

I just checked the changeset right before that one (bf89669b34cb) and it still asserts for me. I'll keep looking into it, but I think it's probably just a generic VM bug.

Comment 3

[Jeff Walden [:Waldo]]

JSObject::freezeOrSeal (or vice versa) does a loop over own properties to change attributes to reflect non-configurability and maybe non-writability.  It picks up shared-permanent properties in the process.  Since __proto__ is such, most of the time (modulo deliberately obnoxious prototype chain munging) this will do a setAttributes() with __proto__.  That hook, assuming the object is native, will then change the attributes of __proto__ *on the object with the property*.  That's |Object.prototype|.  That's clearly insane, of course.

The broader problem is with caching the first hit on assigning to __proto__.  I'm still not entirely sure what's up with that.

Comment 4

[Jason Orendorff [:jorendorff]]

Looks like there are some serious problems here, so this needs an owner. Anyone have spare time?

Comment 5

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

(In reply to Jason Orendorff [:jorendorff] from comment #4)
> Looks like there are some serious problems here, so this needs an owner.
> Anyone have spare time?

I don't seem to be able to reproduce this assert (it wasn't that reproducible for me in the first place), nonetheless setting js-triage-done to get some attention here, assuming comment 3 by Waldo is the triage.

Comment 6

[Jeff Walden [:Waldo]]

This was falling over for a couple different reasons: the shared-permanent hack and __proto__ being a PropertyOp property, at least.  I can't find the first one in enumeration code at all any more.  And the second one is no longer the case, since bug 770344.  Dunno exactly what fixed this, but it's definitely no longer an issue any more.

Comment 7

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

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