Bug 781855 - Fix incorrectly shadowing 'own' properties in the case of prototypal setters. (r=bhackett)
authorEric Faust <efaustbmo@gmail.com>
Wed, 22 Aug 2012 22:05:21 -0700
changeset 105174 455ed4a415aa930e7bd3916362a0dd6b2e32c8e0
parent 105173 759b5d914905b34cee5884d97a09c876eec44dda
child 105175 cd86e0d61c3facabea95de41947273a143c9d95c
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewersbhackett
bugs781855
milestone17.0a1
Bug 781855 - Fix incorrectly shadowing 'own' properties in the case of prototypal setters. (r=bhackett)
js/src/jsinfer.cpp
js/src/jsinfer.h
js/src/jsinferinlines.h
js/src/jsobj.cpp
js/src/methodjit/PolyIC.cpp
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@@ -1083,18 +1083,19 @@ PropertyAccess(JSContext *cx, JSScript *
     if (access != PROPERTY_WRITE) {
         if (JSObject *singleton = object->singleton ? object->singleton : object->proto) {
             Type type = GetSingletonPropertyType(cx, singleton, id);
             if (!type.isUnknown())
                 target->addType(cx, type);
         }
     }
 
-    /* Capture the effects of a standard property access. */
-    HeapTypeSet *types = object->getProperty(cx, id, access == PROPERTY_WRITE);
+    /* Capture the effects of a standard property access. Never mark the
+     * property as own, as it may have inherited accessors. */
+    HeapTypeSet *types = object->getProperty(cx, id, false);
     if (!types)
         return;
     if (access == PROPERTY_WRITE) {
         target->addSubset(cx, types);
     } else {
         JS_ASSERT_IF(script->hasAnalysis(),
                      target == script->analysis()->bytecodeTypes(pc));
         if (!types->hasPropagatedProperty())
--- a/js/src/jsinfer.h
+++ b/js/src/jsinfer.h
@@ -875,17 +875,17 @@ struct TypeObject : gc::Cell
     }
 
     /*
      * Get or create a property of this object. Only call this for properties which
      * a script accesses explicitly. 'assign' indicates whether this is for an
      * assignment, and the own types of the property will be used instead of
      * aggregate types.
      */
-    inline HeapTypeSet *getProperty(JSContext *cx, jsid id, bool assign);
+    inline HeapTypeSet *getProperty(JSContext *cx, jsid id, bool own);
 
     /* Get a property only if it already exists. */
     inline HeapTypeSet *maybeGetProperty(JSContext *cx, jsid id);
 
     inline unsigned getPropertyCount();
     inline Property *getProperty(unsigned i);
 
     /* Set flags on this object which are implied by the specified key. */
--- a/js/src/jsinferinlines.h
+++ b/js/src/jsinferinlines.h
@@ -1370,17 +1370,17 @@ inline void
 TypeObject::setBasePropertyCount(uint32_t count)
 {
     JS_ASSERT(count <= OBJECT_FLAG_PROPERTY_COUNT_LIMIT);
     flags = (flags & ~OBJECT_FLAG_PROPERTY_COUNT_MASK)
           | (count << OBJECT_FLAG_PROPERTY_COUNT_SHIFT);
 }
 
 inline HeapTypeSet *
-TypeObject::getProperty(JSContext *cx, jsid id, bool assign)
+TypeObject::getProperty(JSContext *cx, jsid id, bool own)
 {
     JS_ASSERT(cx->compartment->activeInference);
     JS_ASSERT(JSID_IS_VOID(id) || JSID_IS_EMPTY(id) || JSID_IS_STRING(id));
     JS_ASSERT_IF(!JSID_IS_EMPTY(id), id == MakeTypeId(cx, id));
     JS_ASSERT(!unknownProperties());
 
     uint32_t propertyCount = basePropertyCount();
     Property **pprop = HashSetInsert<jsid,Property,Property>
@@ -1411,47 +1411,18 @@ TypeObject::getProperty(JSContext *cx, j
             }
 
             JS_NOT_REACHED("Missing property");
             return NULL;
         }
     }
 
     HeapTypeSet *types = &(*pprop)->types;
-
-    if (assign && !types->ownProperty(false)) {
-        /*
-         * Normally, we just want to set the property as being an own property
-         * when we got a set to it. The exception is when the set is actually
-         * calling a setter higher on the prototype chain. Check to see if there
-         * is a setter higher on the prototype chain, setter the property as an
-         * own property if that is not the case.
-         */
-        bool foundSetter = false;
-
-        JSObject *protoWalk = proto;
-        while (protoWalk) {
-            if (!protoWalk->isNative()) {
-                protoWalk = protoWalk->getProto();
-                continue;
-            }
-
-            Shape *shape = protoWalk->nativeLookup(cx, id);
-
-            foundSetter = shape &&
-                          !shape->hasDefaultSetter();
-            if (foundSetter)
-                break;
-
-            protoWalk = protoWalk->getProto();
-        }
-
-        if (!foundSetter)
-            types->setOwnProperty(cx, false);
-    }
+    if (own)
+        types->setOwnProperty(cx, false);
 
     return types;
 }
 
 inline HeapTypeSet *
 TypeObject::maybeGetProperty(JSContext *cx, jsid id)
 {
     JS_ASSERT(JSID_IS_VOID(id) || JSID_IS_EMPTY(id) || JSID_IS_STRING(id));
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -4504,25 +4504,24 @@ js_NativeGet(JSContext *cx, Handle<JSObj
 {
     return js_NativeGetInline(cx, obj, obj, pobj, shape, getHow, vp);
 }
 
 JSBool
 js_NativeSet(JSContext *cx, Handle<JSObject*> obj, Handle<JSObject*> receiver,
              Shape *shape, bool added, bool strict, Value *vp)
 {
-    AddTypePropertyId(cx, obj, shape->propid(), *vp);
-
     JS_ASSERT(obj->isNative());
 
     if (shape->hasSlot()) {
         uint32_t slot = shape->slot();
 
         /* If shape has a stub setter, just store *vp. */
         if (shape->hasDefaultSetter()) {
+            AddTypePropertyId(cx, obj, shape->propid(), *vp);
             obj->nativeSetSlot(slot, *vp);
             return true;
         }
     } else {
         /*
          * Allow API consumers to create shared properties with stub setters.
          * Such properties effectively function as data descriptors which are
          * not writable, so attempting to set such a property should do nothing
@@ -4541,16 +4540,17 @@ js_NativeSet(JSContext *cx, Handle<JSObj
 
     /*
      * Update any slot for the shape with the value produced by the setter,
      * unless the setter deleted the shape.
      */
     if (shapeRoot->hasSlot() &&
         (JS_LIKELY(cx->runtime->propertyRemovals == sample) ||
          obj->nativeContains(cx, shapeRoot))) {
+        AddTypePropertyId(cx, obj, shape->propid(), *vp);
         obj->setSlot(shapeRoot->slot(), nvp);
     }
 
     *vp = nvp;
     return true;
 }
 
 static JS_ALWAYS_INLINE JSBool
--- a/js/src/methodjit/PolyIC.cpp
+++ b/js/src/methodjit/PolyIC.cpp
@@ -269,26 +269,32 @@ class SetPropCompiler : public PICStubCo
         Label start = masm.label();
         Jump shapeGuard = masm.branchPtr(Assembler::NotEqual, pic.shapeReg,
                                          ImmPtr(initialShape));
 
         Label stubShapeJumpLabel = masm.label();
 
         pic.setPropLabels().setStubShapeJump(masm, start, stubShapeJumpLabel);
 
-        if (pic.typeMonitored) {
+        if (pic.typeMonitored || adding) {
             /*
+             * There are now two reasons we would want a type barrier. The
+             * first, if we are Type Monitored:
              * Inference does not know the type of the object being updated,
              * and we need to make sure that the updateMonitoredTypes() call
              * covers this stub, i.e. we will be writing to an object with the
              * same type. Add a type guard in addition to the shape guard.
              * Note: it is possible that this test gets a spurious hit if the
              * object has a lazy type, but in such cases no analyzed scripts
              * depend on the object and we will reconstruct its type from the
              * value being written here.
+             * We could also want to add a type monitor if we are simulating
+             * adding the property to the object. This is to ensure that for
+             * objects of different type, but the same shape, we ensure that the
+             * property gets marked as 'own' on the other type objects.
              */
             Jump typeGuard = masm.branchPtr(Assembler::NotEqual,
                                             Address(pic.objReg, JSObject::offsetOfType()),
                                             ImmPtr(obj->getType(cx)));
             if (!otherGuards.append(typeGuard))
                 return error();
         }