From b3f929ee8d11993fa252fa9cbdd354d8efe529b7 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Thu, 7 Oct 2010 12:30:13 -0700 Subject: [PATCH] Bug 602441 - JM: Botched extensibility checking when attempting to add a new property to an object. r=dmandelin --HG-- extra : rebase_source : 1676262c875cd2d283f63c28ae86c9230b7b942a --- js/src/jsinterp.cpp | 4 +- js/src/methodjit/PolyIC.cpp | 2 + js/src/methodjit/StubCalls.cpp | 4 +- .../Object/add-property-non-extensible.js | 118 ++++++++++++++++++ js/src/tests/ecma_5/Object/jstests.list | 1 + js/src/tests/js1_8_5/extensions/jstests.list | 2 +- .../extensions/set-property-non-extensible.js | 14 +-- 7 files changed, 129 insertions(+), 16 deletions(-) create mode 100644 js/src/tests/ecma_5/Object/add-property-non-extensible.js diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 23280911d768..75d2c6f92caf 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -4287,8 +4287,6 @@ BEGIN_CASE(JSOP_SETMETHOD) JSObject *obj2; JSAtom *atom; if (cache->testForSet(cx, regs.pc, obj, &entry, &obj2, &atom)) { - JS_ASSERT(obj->isExtensible()); - /* * Fast property cache hit, only partially confirmed by * testForSet. We know that the entry applies to regs.pc and @@ -4328,6 +4326,8 @@ BEGIN_CASE(JSOP_SETMETHOD) break; } } else { + JS_ASSERT(obj->isExtensible()); + if (obj->nativeEmpty()) { /* * We check that cx owns obj here and will continue to own diff --git a/js/src/methodjit/PolyIC.cpp b/js/src/methodjit/PolyIC.cpp index 42aa6f6ebdb7..a672f6473e20 100644 --- a/js/src/methodjit/PolyIC.cpp +++ b/js/src/methodjit/PolyIC.cpp @@ -585,6 +585,8 @@ class SetPropCompiler : public PICStubCompiler /* Adding a property to the object. */ if (obj->isDelegate()) return disable("delegate"); + if (!obj->isExtensible()) + return disable("not extensible"); if (clasp->addProperty != PropertyStub) return disable("add property hook"); diff --git a/js/src/methodjit/StubCalls.cpp b/js/src/methodjit/StubCalls.cpp index a1ae52945d9a..12ba0a0e66cc 100644 --- a/js/src/methodjit/StubCalls.cpp +++ b/js/src/methodjit/StubCalls.cpp @@ -143,8 +143,6 @@ stubs::SetName(VMFrame &f, JSAtom *origAtom) JSObject *obj2; JSAtom *atom; if (cache->testForSet(cx, f.regs.pc, obj, &entry, &obj2, &atom)) { - JS_ASSERT(obj->isExtensible()); - /* * Fast property cache hit, only partially confirmed by * testForSet. We know that the entry applies to regs.pc and @@ -184,6 +182,8 @@ stubs::SetName(VMFrame &f, JSAtom *origAtom) break; } } else { + JS_ASSERT(obj->isExtensible()); + if (obj->nativeEmpty()) { /* * We check that cx owns obj here and will continue to own diff --git a/js/src/tests/ecma_5/Object/add-property-non-extensible.js b/js/src/tests/ecma_5/Object/add-property-non-extensible.js new file mode 100644 index 000000000000..2089a5678b6b --- /dev/null +++ b/js/src/tests/ecma_5/Object/add-property-non-extensible.js @@ -0,0 +1,118 @@ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + * Contributor: + * Jeff Walden + */ + +var gTestfile = 'add-property-non-extensible.js'; +//----------------------------------------------------------------------------- +var BUGNUMBER = 602144; +var summary = + 'Properly method-compile attempted addition of properties to ' + + 'non-extensible objects'; + +print(BUGNUMBER + ": " + summary); + +/************** + * BEGIN TEST * + **************/ + +// No property + +var o1 = {}; +for (var i = 0; i < 5; i++) + o1.a = 3; +assertEq(o1.a, 3); + +var o2 = Object.preventExtensions({}); +for (var i = 0; i < 5; i++) + o2.a = 3; +assertEq(Object.getOwnPropertyDescriptor(o2, "a"), undefined); + +var o3 = Object.seal({}); +for (var i = 0; i < 5; i++) + o3.a = 3; +assertEq(Object.getOwnPropertyDescriptor(o3, "a"), undefined); + +var o4 = Object.freeze({}); +for (var i = 0; i < 5; i++) + o4.a = 3; +assertEq(Object.getOwnPropertyDescriptor(o4, "a"), undefined); + + +// Has property + +var o5 = { a: 2 }; +for (var i = 0; i < 5; i++) + o5.a = 3; +assertEq(o5.a, 3); + +var o6 = Object.preventExtensions({ a: 2 }); +for (var i = 0; i < 5; i++) + o6.a = 3; +assertEq(o6.a, 3); + +var o7 = Object.seal({ a: 2 }); +for (var i = 0; i < 5; i++) + o7.a = 3; +assertEq(o7.a, 3); + +var o8 = Object.freeze({ a: 2 }); +for (var i = 0; i < 5; i++) + o8.a = 3; +assertEq(o8.a, 2); + + +// Extensible, hit up the prototype chain + +var o9 = Object.create({ a: 2 }); +for (var i = 0; i < 5; i++) + o9.a = 3; +assertEq(o9.a, 3); + +var o10 = Object.create(Object.preventExtensions({ a: 2 })); +for (var i = 0; i < 5; i++) + o10.a = 3; +assertEq(o10.a, 3); + +var o11 = Object.create(Object.seal({ a: 2 })); +for (var i = 0; i < 5; i++) + o11.a = 3; +assertEq(o11.a, 3); + +var o12 = Object.create(Object.freeze({ a: 2 })); +for (var i = 0; i < 5; i++) + o12.a = 3; +assertEq(Object.getOwnPropertyDescriptor(o12, "a"), undefined); + + +// Not extensible, hit up the prototype chain + +var o13 = Object.preventExtensions(Object.create({ a: 2 })); +for (var i = 0; i < 5; i++) + o13.a = 3; +assertEq(Object.getOwnPropertyDescriptor(o13, "a"), undefined); + +var o14 = + Object.preventExtensions(Object.create(Object.preventExtensions({ a: 2 }))); +for (var i = 0; i < 5; i++) + o14.a = 3; +assertEq(Object.getOwnPropertyDescriptor(o14, "a"), undefined); + +var o15 = Object.preventExtensions(Object.create(Object.seal({ a: 2 }))); +for (var i = 0; i < 5; i++) + o15.a = 3; +assertEq(Object.getOwnPropertyDescriptor(o15, "a"), undefined); + +var o16 = Object.preventExtensions(Object.create(Object.freeze({ a: 2 }))); +for (var i = 0; i < 5; i++) + o16.a = 3; +assertEq(Object.getOwnPropertyDescriptor(o16, "a"), undefined); + + +/******************************************************************************/ + +reportCompare(true, true); + +print("All tests passed!"); diff --git a/js/src/tests/ecma_5/Object/jstests.list b/js/src/tests/ecma_5/Object/jstests.list index 628ea88b493e..4ded7a889474 100644 --- a/js/src/tests/ecma_5/Object/jstests.list +++ b/js/src/tests/ecma_5/Object/jstests.list @@ -39,3 +39,4 @@ script 15.2.3.6-define-over-method.js script mutation-prevention-methods.js script object-toString-01.js script vacuous-accessor-unqualified-name.js +script add-property-non-extensible.js diff --git a/js/src/tests/js1_8_5/extensions/jstests.list b/js/src/tests/js1_8_5/extensions/jstests.list index ba12a1cdecfa..89c116e60076 100644 --- a/js/src/tests/js1_8_5/extensions/jstests.list +++ b/js/src/tests/js1_8_5/extensions/jstests.list @@ -20,4 +20,4 @@ skip-if(!xulRuntime.shell) script clone-regexp.js skip-if(!xulRuntime.shell) script clone-object.js skip-if(!xulRuntime.shell) script clone-typed-array.js skip-if(!xulRuntime.shell) script clone-errors.js -skip-if(!xulRuntime.shell) script set-property-non-extensible.js # methodjit mis-implements extensibility checking, bug 602441 +script set-property-non-extensible.js diff --git a/js/src/tests/js1_8_5/extensions/set-property-non-extensible.js b/js/src/tests/js1_8_5/extensions/set-property-non-extensible.js index 79ec7b18b612..200c0e36a40b 100644 --- a/js/src/tests/js1_8_5/extensions/set-property-non-extensible.js +++ b/js/src/tests/js1_8_5/extensions/set-property-non-extensible.js @@ -12,17 +12,9 @@ print(BUGNUMBER + ": " + summary); * BEGIN TEST * **************/ -if (typeof options === "function" && - options().split(",").indexOf("methodjit") < 0) -{ - var o = Object.freeze({}); - for (var i = 0; i < 10; i++) - print(o.u = ""); -} -else -{ - print("non-extensible+loop adding property disabled, bug 602441"); -} +var o = Object.freeze({}); +for (var i = 0; i < 10; i++) + print(o.u = ""); Object.freeze(this); for (let j = 0; j < 10; j++)