Bug 1065604 - Assert that JSPROP_SHARED is set on all properties defined with JSPROP_GETTER or JSPROP_SETTER. r=Waldo, a=lmandel
authorJason Orendorff <jorendorff@mozilla.com>
Wed, 10 Sep 2014 15:17:52 -0500
changeset 225856 ecaedd858fd0
parent 225855 b58f505f18df
child 225857 d38dd7de64b3
push id4041
push userryanvm@gmail.com
push date2014-10-29 18:20 +0000
treeherdermozilla-beta@79567465c505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo, lmandel
bugs1065604
milestone34.0
Bug 1065604 - Assert that JSPROP_SHARED is set on all properties defined with JSPROP_GETTER or JSPROP_SETTER. r=Waldo, a=lmandel
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);