Bug 1065604 - Assert that JSPROP_SHARED is set on all properties defined with JSPROP_GETTER or JSPROP_SETTER. r=Waldo.
authorJason Orendorff <jorendorff@mozilla.com>
Wed, 10 Sep 2014 15:17:52 -0500
changeset 205506 3a59d92e7cb77181d4dd4d2d4cf8359641eb3186
parent 205505 cf9ed5c3532974b8b55d9b9ec6968d5c9b1e6026
child 205507 de8214005a4a33e7daa1aaae2b23896bd47fd7eb
push id49207
push userjorendorff@mozilla.com
push dateTue, 16 Sep 2014 17:27:14 +0000
treeherdermozilla-inbound@dc115b033048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1065604
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1065604 - Assert that JSPROP_SHARED is set on all properties defined with JSPROP_GETTER or JSPROP_SETTER. r=Waldo.
js/src/builtin/Intl.cpp
js/src/jsapi-tests/testDefineGetterSetterNonEnumerable.cpp
js/src/jsapi-tests/testDefinePropertyIgnoredAttributes.cpp
js/src/vm/Shape.cpp
js/src/vm/Shape.h
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
--- a/js/src/builtin/Intl.cpp
+++ b/js/src/builtin/Intl.cpp
@@ -700,17 +700,17 @@ InitCollatorClass(JSContext *cx, HandleO
      * comparison function for the specified Collator object (suitable for
      * passing to methods like Array.prototype.sort).
      */
     RootedValue getter(cx);
     if (!GlobalObject::getIntrinsicValue(cx, cx->global(), cx->names().CollatorCompareGet, &getter))
         return nullptr;
     if (!JSObject::defineProperty(cx, proto, cx->names().compare, UndefinedHandleValue,
                                   JS_DATA_TO_FUNC_PTR(JSPropertyOp, &getter.toObject()),
-                                  nullptr, JSPROP_GETTER))
+                                  nullptr, JSPROP_GETTER | JSPROP_SHARED))
     {
         return nullptr;
     }
 
     RootedValue options(cx);
     if (!CreateDefaultOptions(cx, &options))
         return nullptr;
 
@@ -1191,17 +1191,17 @@ InitNumberFormatClass(JSContext *cx, Han
      * bound formatting function for the specified NumberFormat object (suitable
      * for passing to methods like Array.prototype.map).
      */
     RootedValue getter(cx);
     if (!GlobalObject::getIntrinsicValue(cx, cx->global(), cx->names().NumberFormatFormatGet, &getter))
         return nullptr;
     if (!JSObject::defineProperty(cx, proto, cx->names().format, UndefinedHandleValue,
                                   JS_DATA_TO_FUNC_PTR(JSPropertyOp, &getter.toObject()),
-                                  nullptr, JSPROP_GETTER))
+                                  nullptr, JSPROP_GETTER | JSPROP_SHARED))
     {
         return nullptr;
     }
 
     RootedValue options(cx);
     if (!CreateDefaultOptions(cx, &options))
         return nullptr;
 
@@ -1647,17 +1647,17 @@ InitDateTimeFormatClass(JSContext *cx, H
      * bound formatting function for the specified DateTimeFormat object
      * (suitable for passing to methods like Array.prototype.map).
      */
     RootedValue getter(cx);
     if (!GlobalObject::getIntrinsicValue(cx, cx->global(), cx->names().DateTimeFormatFormatGet, &getter))
         return nullptr;
     if (!JSObject::defineProperty(cx, proto, cx->names().format, UndefinedHandleValue,
                                   JS_DATA_TO_FUNC_PTR(JSPropertyOp, &getter.toObject()),
-                                  nullptr, JSPROP_GETTER))
+                                  nullptr, JSPROP_GETTER | JSPROP_SHARED))
     {
         return nullptr;
     }
 
     RootedValue options(cx);
     if (!CreateDefaultOptions(cx, &options))
         return nullptr;
 
--- a/js/src/jsapi-tests/testDefineGetterSetterNonEnumerable.cpp
+++ b/js/src/jsapi-tests/testDefineGetterSetterNonEnumerable.cpp
@@ -30,23 +30,23 @@ BEGIN_TEST(testDefineGetterSetterNonEnum
     JSFunction *funSet = JS_NewFunction(cx, NativeGetterSetter, 1, 0, JS::NullPtr(), "set");
     CHECK(funSet);
     JS::RootedObject funSetObj(cx, JS_GetFunctionObject(funSet));
     JS::RootedValue vset(cx, OBJECT_TO_JSVAL(funSetObj));
 
     JS::RootedObject vObject(cx, vobj.toObjectOrNull());
     CHECK(JS_DefineProperty(cx, vObject, PROPERTY_NAME,
                             JS::UndefinedHandleValue,
-                            JSPROP_GETTER | JSPROP_SETTER | JSPROP_ENUMERATE,
+                            JSPROP_GETTER | JSPROP_SETTER | JSPROP_SHARED | JSPROP_ENUMERATE,
                             JS_DATA_TO_FUNC_PTR(JSPropertyOp, (JSObject*) funGetObj),
                             JS_DATA_TO_FUNC_PTR(JSStrictPropertyOp, (JSObject*) funSetObj)));
 
     CHECK(JS_DefineProperty(cx, vObject, PROPERTY_NAME,
                             JS::UndefinedHandleValue,
-                            JSPROP_GETTER | JSPROP_SETTER | JSPROP_PERMANENT,
+                            JSPROP_GETTER | JSPROP_SETTER | JSPROP_SHARED | JSPROP_PERMANENT,
                             JS_DATA_TO_FUNC_PTR(JSPropertyOp, (JSObject*) funGetObj),
                             JS_DATA_TO_FUNC_PTR(JSStrictPropertyOp, (JSObject*) funSetObj)));
 
     JS::Rooted<JSPropertyDescriptor> desc(cx);
     CHECK(JS_GetOwnPropertyDescriptor(cx, vObject, PROPERTY_NAME, &desc));
     CHECK(desc.object());
     CHECK(desc.hasGetterObject());
     CHECK(desc.hasSetterObject());
--- a/js/src/jsapi-tests/testDefinePropertyIgnoredAttributes.cpp
+++ b/js/src/jsapi-tests/testDefinePropertyIgnoredAttributes.cpp
@@ -42,36 +42,39 @@ CheckDescriptor(JS::Handle<JSPropertyDes
 BEGIN_TEST(testDefinePropertyIgnoredAttributes)
 {
     JS::RootedObject obj(cx, JS_NewObject(cx, nullptr, JS::NullPtr(), JS::NullPtr()));
     JS::Rooted<JSPropertyDescriptor> desc(cx);
     JS::RootedValue defineValue(cx);
 
     // Try a getter. Allow it to fill in the defaults.
     CHECK(JS_DefineProperty(cx, obj, "foo", defineValue,
-                            IgnoreAll | JSPROP_NATIVE_ACCESSORS, (JSPropertyOp)Getter));
+                            IgnoreAll | JSPROP_NATIVE_ACCESSORS | JSPROP_SHARED,
+                            (JSPropertyOp)Getter));
 
     CHECK(JS_GetPropertyDescriptor(cx, obj, "foo", &desc));
 
     // Note that since JSPROP_READONLY means nothing for accessor properties, we will actually
     // claim to be writable, since the flag is not included in the mask.
     CHECK(CheckDescriptor(desc, false, true, false));
 
     // Install another configurable property, so we can futz with it.
     CHECK(JS_DefineProperty(cx, obj, "bar", defineValue,
-                            AllowConfigure | JSPROP_NATIVE_ACCESSORS, (JSPropertyOp)Getter));
+                            AllowConfigure | JSPROP_NATIVE_ACCESSORS | JSPROP_SHARED,
+                            (JSPropertyOp)Getter));
     CHECK(JS_GetPropertyDescriptor(cx, obj, "bar", &desc));
     CHECK(CheckDescriptor(desc, false, true, true));
 
     // Rewrite the descriptor to now be enumerable, ensuring that the lack of
     // configurablity stayed.
     CHECK(JS_DefineProperty(cx, obj, "bar", defineValue,
                             AllowEnumerate |
                             JSPROP_ENUMERATE |
-                            JSPROP_NATIVE_ACCESSORS,
+                            JSPROP_NATIVE_ACCESSORS |
+                            JSPROP_SHARED,
                             (JSPropertyOp)Getter));
     CHECK(JS_GetPropertyDescriptor(cx, obj, "bar", &desc));
     CHECK(CheckDescriptor(desc, true, true, true));
 
     // Now try the same game with a value property
     defineValue.setObject(*obj);
     CHECK(JS_DefineProperty(cx, obj, "baz", defineValue, IgnoreWithValue));
     CHECK(JS_GetPropertyDescriptor(cx, obj, "baz", &desc));
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -902,16 +902,18 @@ JSObject::putProperty(typename Execution
                 return nullptr;
         }
 
         if (updateLast)
             shape->base()->adoptUnowned(nbase);
         else
             shape->base_ = nbase;
 
+        JS_ASSERT_IF(attrs & (JSPROP_GETTER | JSPROP_SETTER), attrs & JSPROP_SHARED);
+
         shape->setSlot(slot);
         shape->attrs = uint8_t(attrs);
         shape->flags = flags | Shape::IN_DICTIONARY;
     } else {
         /*
          * Updating the last property in a non-dictionary-mode object. Find an
          * alternate shared child of the last property's previous shape.
          */
@@ -974,16 +976,17 @@ template <ExecutionMode mode>
 JSObject::changeProperty(typename ExecutionModeTraits<mode>::ExclusiveContextType cx,
                          HandleObject obj, HandleShape shape, unsigned attrs,
                          unsigned mask, PropertyOp getter, StrictPropertyOp setter)
 {
     JS_ASSERT(cx->isThreadLocal(obj));
     JS_ASSERT(obj->nativeContainsPure(shape));
 
     attrs |= shape->attrs & mask;
+    JS_ASSERT_IF(attrs & (JSPROP_GETTER | JSPROP_SETTER), attrs & JSPROP_SHARED);
 
     /* Allow only shared (slotless) => unshared (slotful) transition. */
     JS_ASSERT(!((attrs ^ shape->attrs) & JSPROP_SHARED) ||
               !(attrs & JSPROP_SHARED));
 
     if (mode == ParallelExecution) {
         if (!types::IsTypePropertyIdMarkedNonData(obj, shape->propid()))
             return nullptr;
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -1242,16 +1242,17 @@ struct StackShape
         propid(propid),
         slot_(slot),
         attrs(uint8_t(attrs)),
         flags(uint8_t(flags))
     {
         JS_ASSERT(base);
         JS_ASSERT(!JSID_IS_VOID(propid));
         JS_ASSERT(slot <= SHAPE_INVALID_SLOT);
+        JS_ASSERT_IF(attrs & (JSPROP_GETTER | JSPROP_SETTER), attrs & JSPROP_SHARED);
     }
 
     explicit StackShape(Shape *shape)
       : base(shape->base()->unowned()),
         propid(shape->propidRef()),
         slot_(shape->maybeSlot()),
         attrs(shape->attrs),
         flags(shape->flags)
@@ -1350,16 +1351,17 @@ inline
 Shape::Shape(const StackShape &other, uint32_t nfixed)
   : base_(other.base),
     propid_(other.propid),
     slotInfo(other.maybeSlot() | (nfixed << FIXED_SLOTS_SHIFT)),
     attrs(other.attrs),
     flags(other.flags),
     parent(nullptr)
 {
+    JS_ASSERT_IF(attrs & (JSPROP_GETTER | JSPROP_SETTER), attrs & JSPROP_SHARED);
     kids.setNull();
 }
 
 inline
 Shape::Shape(UnownedBaseShape *base, uint32_t nfixed)
   : base_(base),
     propid_(JSID_EMPTY),
     slotInfo(SHAPE_INVALID_SLOT | (nfixed << FIXED_SLOTS_SHIFT)),
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -302,17 +302,17 @@ DefinePropertyIfFound(XPCCallContext& cc
 
             if (!fun)
                 return false;
 
             RootedObject funobj(ccx, JS_GetFunctionObject(fun));
             if (!funobj)
                 return false;
 
-            propFlags |= JSPROP_GETTER;
+            propFlags |= JSPROP_GETTER | JSPROP_SHARED;
             propFlags &= ~JSPROP_ENUMERATE;
 
             AutoResolveName arn(ccx, id);
             if (resolved)
                 *resolved = true;
             return JS_DefinePropertyById(ccx, obj, id, UndefinedHandleValue, propFlags,
                                          JS_DATA_TO_FUNC_PTR(JSPropertyOp, funobj.get()),
                                          nullptr);