Bug 1103368, part 1 - Forbid stub getter/setter arguments to NativeObject::{add,change,put}Property. r=bhackett.
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 21 Nov 2014 19:33:45 -0600
changeset 219194 f57597056a607a064c05ccedf6cb527f46517f82
parent 219193 a8e3b6311afac01ed6bc189965152c3680ee6ea2
child 219195 5814a172384247c021279efd6d240a0576d27173
push id27958
push userkwierso@gmail.com
push dateFri, 12 Dec 2014 01:30:39 +0000
treeherdermozilla-central@5288b15d22de [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1103368
milestone37.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 1103368, part 1 - Forbid stub getter/setter arguments to NativeObject::{add,change,put}Property. r=bhackett.
js/src/jsobj.cpp
js/src/vm/NativeObject.cpp
js/src/vm/ScopeObject.cpp
js/src/vm/Shape.cpp
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -2412,17 +2412,17 @@ DefineStandardSlot(JSContext *cx, Handle
          * reserved slot. Otherwise, go through the normal property path.
          */
         Rooted<GlobalObject*> global(cx, &obj->as<GlobalObject>());
 
         if (!global->lookup(cx, id)) {
             global->setConstructorPropertySlot(key, v);
 
             uint32_t slot = GlobalObject::constructorPropertySlot(key);
-            if (!NativeObject::addProperty(cx, global, id, JS_PropertyStub, JS_StrictPropertyStub, slot, attrs, 0))
+            if (!NativeObject::addProperty(cx, global, id, nullptr, nullptr, slot, attrs, 0))
                 return false;
 
             named = true;
             return true;
         }
     }
 
     named = JSObject::defineGeneric(cx, obj, id,
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1257,16 +1257,20 @@ DefinePropertyOrElement(typename Executi
     if (IsAnyTypedArray(obj)) {
         uint64_t index;
         if (IsTypedArrayIndex(id, &index))
             return true;
     }
 
     AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter);
 
+    if (getter == JS_PropertyStub)
+        getter = nullptr;
+    if (setter == JS_StrictPropertyStub)
+        setter = nullptr;
     RootedShape shape(cx, NativeObject::putProperty<mode>(cx, obj, id, getter, setter,
                                                           SHAPE_INVALID_SLOT, attrs, 0));
     if (!shape)
         return false;
 
     if (!UpdateShapeTypeAndValue<mode>(cx, obj, shape, value))
         return false;
 
@@ -1283,25 +1287,25 @@ DefinePropertyOrElement(typename Executi
 
         ExclusiveContext *ncx = cx->asExclusiveContext();
         uint32_t index = JSID_TO_INT(id);
         NativeObject::removeDenseElementForSparseIndex(ncx, obj, index);
         NativeObject::EnsureDenseResult result = NativeObject::maybeDensifySparseElements(ncx, obj);
         if (result == NativeObject::ED_FAILED)
             return false;
         if (result == NativeObject::ED_OK) {
-            MOZ_ASSERT(setter == JS_StrictPropertyStub);
+            MOZ_ASSERT(!setter);
             return CallAddPropertyHookDense<mode>(cx, obj->getClass(), obj, index, value);
         }
     }
 
     if (!CallAddPropertyHook<mode>(cx, obj->getClass(), obj, shape, value))
         return false;
 
-    if (callSetterAfterwards && setter != JS_StrictPropertyStub) {
+    if (callSetterAfterwards && setter) {
         if (!cx->shouldBeJSContext())
             return false;
         RootedValue nvalue(cx, value);
         return NativeSet<mode>(ExecutionModeTraits<mode>::toContextType(cx),
                                obj, obj, shape, setterIsStrict, &nvalue);
     }
     return true;
 }
@@ -1451,16 +1455,20 @@ js::DefineNativeProperty(ExclusiveContex
                 if (!NativeObject::sparsifyDenseElement(cx, obj, JSID_TO_INT(id)))
                     return false;
                 shape = obj->lookup(cx, id);
             }
             if (shape->isAccessorDescriptor()) {
                 if (!CheckAccessorRedefinition(cx, obj, shape, getter, setter, id, attrs))
                     return false;
                 attrs = ApplyOrDefaultAttributes(attrs, shape);
+                if (getter == JS_PropertyStub)
+                    getter = nullptr;
+                if (setter == JS_StrictPropertyStub)
+                    setter = nullptr;
                 shape = NativeObject::changeProperty<SequentialExecution>(cx, obj, shape, attrs,
                                                                           JSPROP_GETTER | JSPROP_SETTER,
                                                                           (attrs & JSPROP_GETTER)
                                                                           ? getter
                                                                           : shape->getter(),
                                                                           (attrs & JSPROP_SETTER)
                                                                           ? setter
                                                                           : shape->setter());
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -333,18 +333,26 @@ DeclEnvObject::createTemplateObject(JSCo
                                                                   emptyDeclEnvShape, type)));
     if (!obj)
         return nullptr;
 
     // Assign a fixed slot to a property with the same name as the lambda.
     Rooted<jsid> id(cx, AtomToId(fun->atom()));
     const Class *clasp = obj->getClass();
     unsigned attrs = JSPROP_ENUMERATE | JSPROP_PERMANENT | JSPROP_READONLY;
-    if (!NativeObject::putProperty<SequentialExecution>(cx, obj, id, clasp->getProperty,
-                                                        clasp->setProperty, lambdaSlot(), attrs, 0)) {
+
+    JSPropertyOp getter = clasp->getProperty;
+    if (getter == JS_PropertyStub)
+        getter = nullptr;
+    JSStrictPropertyOp setter = clasp->setProperty;
+    if (setter == JS_StrictPropertyStub)
+        setter = nullptr;
+
+    if (!NativeObject::putProperty<SequentialExecution>(cx, obj, id, getter, setter, lambdaSlot(),
+                                                        attrs, 0)) {
         return nullptr;
     }
 
     MOZ_ASSERT(!obj->hasDynamicSlots());
     return &obj->as<DeclEnvObject>();
 }
 
 DeclEnvObject *
@@ -702,17 +710,17 @@ StaticBlockObject::create(ExclusiveConte
 StaticBlockObject::addVar(ExclusiveContext *cx, Handle<StaticBlockObject*> block, HandleId id,
                           bool constant, unsigned index, bool *redeclared)
 {
     MOZ_ASSERT(JSID_IS_ATOM(id));
     MOZ_ASSERT(index < LOCAL_INDEX_LIMIT);
 
     *redeclared = false;
 
-    /* Inline JSObject::addProperty in order to trap the redefinition case. */
+    /* Inline NativeObject::addProperty in order to trap the redefinition case. */
     Shape **spp;
     if (Shape::search(cx, block->lastProperty(), id, &spp, true)) {
         *redeclared = true;
         return nullptr;
     }
 
     /*
      * Don't convert this object to dictionary mode so that we can clone the
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -495,57 +495,35 @@ js::NativeObject::toDictionaryMode(Threa
     self->shape_ = root;
 
     MOZ_ASSERT(self->inDictionaryMode());
     root->base()->setSlotSpan(span);
 
     return true;
 }
 
-/*
- * Normalize stub getter and setter values for faster is-stub testing in the
- * SHAPE_CALL_[GS]ETTER macros.
- */
-static inline bool
-NormalizeGetterAndSetter(JSObject *obj,
-                         jsid id, unsigned attrs, unsigned flags,
-                         PropertyOp &getter,
-                         StrictPropertyOp &setter)
-{
-    if (setter == JS_StrictPropertyStub) {
-        MOZ_ASSERT(!(attrs & JSPROP_SETTER));
-        setter = nullptr;
-    }
-    if (getter == JS_PropertyStub) {
-        MOZ_ASSERT(!(attrs & JSPROP_GETTER));
-        getter = nullptr;
-    }
-
-    return true;
-}
-
 /* static */ Shape *
 NativeObject::addProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId id,
                           PropertyOp getter, StrictPropertyOp setter,
                           uint32_t slot, unsigned attrs,
                           unsigned flags, bool allowDictionary)
 {
     MOZ_ASSERT(!JSID_IS_VOID(id));
+    MOZ_ASSERT(getter != JS_PropertyStub);
+    MOZ_ASSERT(setter != JS_StrictPropertyStub);
 
     bool extensible;
     if (!JSObject::isExtensible(cx, obj, &extensible))
         return nullptr;
     if (!extensible) {
         if (cx->isJSContext())
             obj->reportNotExtensible(cx->asJSContext());
         return nullptr;
     }
 
-    NormalizeGetterAndSetter(obj, id, attrs, flags, getter, setter);
-
     Shape **spp = nullptr;
     if (obj->inDictionaryMode())
         spp = obj->lastProperty()->table().search(id, true);
 
     return addPropertyInternal<SequentialExecution>(cx, obj, id, getter, setter, slot, attrs,
                                                     flags, spp, allowDictionary);
 }
 
@@ -577,16 +555,18 @@ NativeObject::addPropertyInternal(typena
                                   HandleNativeObject obj, HandleId id,
                                   PropertyOp getter, StrictPropertyOp setter,
                                   uint32_t slot, unsigned attrs,
                                   unsigned flags, Shape **spp,
                                   bool allowDictionary)
 {
     MOZ_ASSERT(cx->isThreadLocal(obj));
     MOZ_ASSERT_IF(!allowDictionary, !obj->inDictionaryMode());
+    MOZ_ASSERT(getter != JS_PropertyStub);
+    MOZ_ASSERT(setter != JS_StrictPropertyStub);
 
     AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter);
 
     /*
      * The code below deals with either converting obj to dictionary mode or
      * growing an object that's already in dictionary mode. Either way,
      * dictionray operations are safe if thread local.
      */
@@ -762,28 +742,28 @@ template <ExecutionMode mode>
 /* static */ Shape *
 NativeObject::putProperty(typename ExecutionModeTraits<mode>::ExclusiveContextType cx,
                           HandleNativeObject obj, HandleId id,
                           PropertyOp getter, StrictPropertyOp setter,
                           uint32_t slot, unsigned attrs, unsigned flags)
 {
     MOZ_ASSERT(cx->isThreadLocal(obj));
     MOZ_ASSERT(!JSID_IS_VOID(id));
+    MOZ_ASSERT(getter != JS_PropertyStub);
+    MOZ_ASSERT(setter != JS_StrictPropertyStub);
 
 #ifdef DEBUG
     if (obj->is<ArrayObject>()) {
         ArrayObject *arr = &obj->as<ArrayObject>();
         uint32_t index;
         if (js_IdIsIndex(id, &index))
             MOZ_ASSERT(index < arr->length() || arr->lengthIsWritable());
     }
 #endif
 
-    NormalizeGetterAndSetter(obj, id, attrs, flags, getter, setter);
-
     AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter);
 
     /*
      * Search for id in order to claim its entry if table has been allocated.
      *
      * Note that we can only try to claim an entry in a table that is thread
      * local. An object may be thread local *without* its shape being thread
      * local. The only thread local objects that *also* have thread local
@@ -977,36 +957,33 @@ NativeObject::putProperty<ParallelExecut
 template <ExecutionMode mode>
 /* static */ Shape *
 NativeObject::changeProperty(typename ExecutionModeTraits<mode>::ExclusiveContextType cx,
                              HandleNativeObject obj, HandleShape shape, unsigned attrs,
                              unsigned mask, PropertyOp getter, StrictPropertyOp setter)
 {
     MOZ_ASSERT(cx->isThreadLocal(obj));
     MOZ_ASSERT(obj->containsPure(shape));
+    MOZ_ASSERT(getter != JS_PropertyStub);
+    MOZ_ASSERT(setter != JS_StrictPropertyStub);
 
     attrs |= shape->attrs & mask;
     MOZ_ASSERT_IF(attrs & (JSPROP_GETTER | JSPROP_SETTER), attrs & JSPROP_SHARED);
 
     /* Allow only shared (slotless) => unshared (slotful) transition. */
     MOZ_ASSERT(!((attrs ^ shape->attrs) & JSPROP_SHARED) ||
                !(attrs & JSPROP_SHARED));
 
     if (mode == ParallelExecution) {
         if (!types::IsTypePropertyIdMarkedNonData(obj, shape->propid()))
             return nullptr;
     } else {
         types::MarkTypePropertyNonData(cx->asExclusiveContext(), obj, shape->propid());
     }
 
-    if (getter == JS_PropertyStub)
-        getter = nullptr;
-    if (setter == JS_StrictPropertyStub)
-        setter = nullptr;
-
     if (!CheckCanChangeAttrs(cx, obj, shape, &attrs))
         return nullptr;
 
     if (shape->attrs == attrs && shape->getter() == getter && shape->setter() == setter)
         return shape;
 
     /*
      * Let JSObject::putProperty handle this |overwriting| case, including