Bug 1397026 - Make sure JSOP_INIT* IC behavior matches [[DefineOwnProperty]] instead of [[Set]]. r=anba
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 07 Sep 2017 11:28:58 +0200
changeset 429008 509c46b010b6dc08e988d589bdfdd456d7277774
parent 429007 f090a933bbaeb03c0c5e41a654e226cf9ecf32df
child 429009 49fb9d3575019eba00e28cb7efb47ac70ecd68a4
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1397026
milestone57.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 1397026 - Make sure JSOP_INIT* IC behavior matches [[DefineOwnProperty]] instead of [[Set]]. r=anba
js/src/jit-test/tests/cacheir/bug1397026.js
js/src/jit/CacheIR.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/cacheir/bug1397026.js
@@ -0,0 +1,43 @@
+function f1() {
+    var o = {};
+    var values = [];
+    for (var i = 0; i < 6; ++i) {
+        var desc = {
+            value: i,
+            writable: true,
+            configurable: true,
+            enumerable: true
+        };
+        try {
+            Object.defineProperty(o, "p", desc);
+        } catch (e) {
+        }
+        if (i === 1) {
+            Object.defineProperty(o, "p", {configurable: false});
+        }
+        values.push(o.p);
+    }
+    assertEq(values.toString(), "0,1,1,1,1,1");
+}
+f1();
+
+function f2() {
+    var o = {};
+    for (var i = 0; i < 6; ++i) {
+        var desc = {
+            value: i,
+            writable: true,
+            configurable: true,
+            enumerable: true
+        };
+        try {
+            Object.defineProperty(o, "p", desc);
+        } catch (e) {
+        }
+        assertEq(Object.getOwnPropertyDescriptor(o, "p").enumerable, true);
+        if (i > 0) {
+            Object.defineProperty(o, "p", {enumerable: false});
+        }
+    }
+}
+f2();
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -2707,32 +2707,43 @@ EmitStoreSlotAndReturn(CacheIRWriter& wr
     } else {
         size_t offset = nobj->dynamicSlotIndex(shape->slot()) * sizeof(Value);
         writer.storeDynamicSlot(objId, offset, rhsId);
     }
     writer.returnFromIC();
 }
 
 static Shape*
-LookupShapeForSetSlot(NativeObject* obj, jsid id)
+LookupShapeForSetSlot(JSOp op, NativeObject* obj, jsid id)
 {
     Shape* shape = obj->lookupPure(id);
-    if (shape && shape->hasSlot() && shape->hasDefaultSetter() && shape->writable())
-        return shape;
-    return nullptr;
+    if (!shape || !shape->hasSlot() || !shape->hasDefaultSetter() || !shape->writable())
+        return nullptr;
+
+    // If this is an op like JSOP_INITELEM / [[DefineOwnProperty]], the
+    // property's attributes may have to be changed too, so make sure it's a
+    // simple data property.
+    if (IsPropertyInitOp(op) && (!shape->configurable() ||
+                                 !shape->enumerable() ||
+                                 !shape->hasDefaultGetter()))
+    {
+        return nullptr;
+    }
+
+    return shape;
 }
 
 static bool
-CanAttachNativeSetSlot(JSContext* cx, HandleObject obj, HandleId id,
+CanAttachNativeSetSlot(JSContext* cx, JSOp op, HandleObject obj, HandleId id,
                        bool* isTemporarilyUnoptimizable, MutableHandleShape propShape)
 {
     if (!obj->isNative())
         return false;
 
-    propShape.set(LookupShapeForSetSlot(&obj->as<NativeObject>(), id));
+    propShape.set(LookupShapeForSetSlot(op, &obj->as<NativeObject>(), id));
     if (!propShape)
         return false;
 
     ObjectGroup* group = JSObject::getGroup(cx, obj);
     if (!group) {
         cx->recoverFromOutOfMemory();
         return false;
     }
@@ -2750,17 +2761,17 @@ CanAttachNativeSetSlot(JSContext* cx, Ha
     return true;
 }
 
 bool
 SetPropIRGenerator::tryAttachNativeSetSlot(HandleObject obj, ObjOperandId objId, HandleId id,
                                            ValOperandId rhsId)
 {
     RootedShape propShape(cx_);
-    if (!CanAttachNativeSetSlot(cx_, obj, id, isTemporarilyUnoptimizable_, &propShape))
+    if (!CanAttachNativeSetSlot(cx_, JSOp(*pc_), obj, id, isTemporarilyUnoptimizable_, &propShape))
         return false;
 
     if (mode_ == ICState::Mode::Megamorphic && cacheKind_ == CacheKind::SetProp) {
         writer.megamorphicStoreSlot(objId, JSID_TO_ATOM(id)->asPropertyName(), rhsId,
                                     typeCheckInfo_.needsTypeBarrier());
         writer.returnFromIC();
         trackAttached("MegamorphicNativeSlot");
         return true;
@@ -2794,17 +2805,17 @@ SetPropIRGenerator::tryAttachUnboxedExpa
 {
     if (!obj->is<UnboxedPlainObject>())
         return false;
 
     UnboxedExpandoObject* expando = obj->as<UnboxedPlainObject>().maybeExpando();
     if (!expando)
         return false;
 
-    Shape* propShape = LookupShapeForSetSlot(expando, id);
+    Shape* propShape = LookupShapeForSetSlot(JSOp(*pc_), expando, id);
     if (!propShape)
         return false;
 
     maybeEmitIdGuard(id);
     writer.guardGroup(objId, obj->group());
     ObjOperandId expandoId = writer.guardAndLoadUnboxedExpando(objId);
     writer.guardShape(expandoId, expando->lastProperty());
 
@@ -3475,17 +3486,19 @@ SetPropIRGenerator::tryAttachDOMProxyExp
         MOZ_ASSERT(!expandoVal.isUndefined(),
                    "How did a missing expando manage to shadow things?");
         auto expandoAndGeneration = static_cast<ExpandoAndGeneration*>(expandoVal.toPrivate());
         MOZ_ASSERT(expandoAndGeneration);
         expandoObj = &expandoAndGeneration->expando.toObject();
     }
 
     RootedShape propShape(cx_);
-    if (CanAttachNativeSetSlot(cx_, expandoObj, id, isTemporarilyUnoptimizable_, &propShape)) {
+    if (CanAttachNativeSetSlot(cx_, JSOp(*pc_), expandoObj, id, isTemporarilyUnoptimizable_,
+                               &propShape))
+    {
         maybeEmitIdGuard(id);
         ObjOperandId expandoObjId =
             guardDOMProxyExpandoObjectAndShape(obj, objId, expandoVal, expandoObj);
 
         NativeObject* nativeExpandoObj = &expandoObj->as<NativeObject>();
         writer.guardGroup(expandoObjId, nativeExpandoObj->group());
         typeCheckInfo_.set(nativeExpandoObj->group(), id);
 
@@ -3616,18 +3629,21 @@ SetPropIRGenerator::tryAttachWindowProxy
     // those.
     MOZ_ASSERT(obj->getClass() == cx_->runtime()->maybeWindowProxyClass());
     MOZ_ASSERT(ToWindowIfWindowProxy(obj) == cx_->global());
 
     // Now try to do the set on the Window (the current global).
     Handle<GlobalObject*> windowObj = cx_->global();
 
     RootedShape propShape(cx_);
-    if (!CanAttachNativeSetSlot(cx_, windowObj, id, isTemporarilyUnoptimizable_, &propShape))
+    if (!CanAttachNativeSetSlot(cx_, JSOp(*pc_), windowObj, id, isTemporarilyUnoptimizable_,
+                                &propShape))
+    {
         return false;
+    }
 
     maybeEmitIdGuard(id);
 
     writer.guardClass(objId, GuardClassKind::WindowProxy);
     ObjOperandId windowObjId = writer.loadObject(windowObj);
 
     writer.guardShape(windowObjId, windowObj->lastProperty());
     writer.guardGroup(windowObjId, windowObj->group());