Bug 781855 - Fix incorrectly shadowing 'own' properties in the case of prototypal setters. (r=bhackett, a=akeybl) CALENDAR_1_8b1_BUILD1 CALENDAR_1_8b1_RELEASE THUNDERBIRD_16_0b1_BUILD1 THUNDERBIRD_16_0b1_RELEASE
authorEric Faust <efaustbmo@gmail.com>
Tue, 28 Aug 2012 13:06:58 -0400
changeset 104626 d77906f8ceee51292457e0c58434cd460f1bcac4
parent 104621 87a36e6132f2f240ab600971194654ad8fb78a58
child 104627 87929d2524ffebe9503830eea46bf3c4a46e4100
child 104628 feb9079efbf0c3322ca147a83aef342c06c648b9
child 104645 b8a7067db555b3b7696d1d3567e29b5ee2c5276e
push id1
push usersledru@mozilla.com
push dateThu, 04 Dec 2014 17:57:20 +0000
reviewersbhackett, akeybl
bugs781855
milestone16.0
Bug 781855 - Fix incorrectly shadowing 'own' properties in the case of prototypal setters. (r=bhackett, a=akeybl)
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
@@ -976,18 +976,24 @@ PropertyAccess(JSContext *cx, JSScript *
 
     /* Reads from objects with unknown properties are unknown, writes to such objects are ignored. */
     if (object->unknownProperties()) {
         if (!assign)
             MarkPropertyAccessUnknown(cx, script, pc, target);
         return;
     }
 
-    /* Capture the effects of a standard property access. */
-    TypeSet *types = object->getProperty(cx, id, assign);
+    /*
+     * Capture the effects of a standard property access.  For assignments, we do not
+     * automatically update the 'own' bit on accessed properties, except for indexed
+     * elements in dense arrays.  The latter exception allows for JIT fast paths to avoid
+     * testing the array's type when assigning to dense array elements.
+     */
+    bool markOwn = assign && JSID_IS_VOID(id);
+    TypeSet *types = object->getProperty(cx, id, markOwn);
     if (!types)
         return;
     if (assign) {
         target->addSubset(cx, types);
     } else {
         if (!types->hasPropagatedProperty())
             object->getFromPrototypes(cx, id, types);
         if (UsePropertyTypeBarrier(pc)) {
--- a/js/src/jsinfer.h
+++ b/js/src/jsinfer.h
@@ -782,17 +782,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 TypeSet *getProperty(JSContext *cx, jsid id, bool assign);
+    inline TypeSet *getProperty(JSContext *cx, jsid id, bool own);
 
     /* Get a property only if it already exists. */
     inline TypeSet *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
@@ -1212,17 +1212,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 TypeSet *
-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>
@@ -1243,47 +1243,18 @@ TypeObject::getProperty(JSContext *cx, j
             markUnknown(cx);
             TypeSet *types = TypeSet::make(cx, "propertyOverflow");
             types->addType(cx, Type::UnknownType());
             return types;
         }
     }
 
     TypeSet *types = &(*pprop)->types;
-
-    if (assign && !types->isOwnProperty(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 TypeSet *
 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
@@ -4611,25 +4611,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
@@ -4647,16 +4646,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(), *vp);
     }
 
     return true;
 }
 
 static JS_ALWAYS_INLINE JSBool
 js_GetPropertyHelperInline(JSContext *cx, HandleObject obj, HandleObject receiver, jsid id_,
--- a/js/src/methodjit/PolyIC.cpp
+++ b/js/src/methodjit/PolyIC.cpp
@@ -268,17 +268,17 @@ 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) {
             /*
              * 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