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

@@ -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

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");

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

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 <jwalden+code@mit.edu>
+ */
+
+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!");

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

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

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++)