Bug 1148973 - When skipping shape guards in Ion common getter/setter code because the object has a non-configurable property, first verify that its current shape matches the shape we're using to compile our code. r=jandem, a=sledru
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 30 Mar 2015 23:44:01 -0400
changeset 258250 d6ec30c02b8d
parent 258249 8d84399a000b
child 258251 125623fcc804
push id4628
push userryanvm@gmail.com
push date2015-04-03 20:32 +0000
treeherdermozilla-beta@e4566e5991e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, sledru
bugs1148973
milestone38.0
Bug 1148973 - When skipping shape guards in Ion common getter/setter code because the object has a non-configurable property, first verify that its current shape matches the shape we're using to compile our code. r=jandem, a=sledru
js/src/jit-test/tests/ion/bug1148973-1.js
js/src/jit-test/tests/ion/bug1148973-2.js
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1148973-1.js
@@ -0,0 +1,14 @@
+Object.defineProperty(this, "x", { get: decodeURI, configurable: true })
+try {
+    String(b = Proxy.createFunction(function() {
+        return {
+            get: function(r, z) {
+                return x[z]
+            }
+        }
+    }(), function() {}))
+} catch (e) {};
+try {
+    function x() {}
+    assertEq(String(b), "function () {}");
+} catch (e) { throw (e); }
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1148973-2.js
@@ -0,0 +1,8 @@
+var proto = {};
+var obj = Object.create(proto);
+Object.defineProperty(proto, "x", { get: decodeURI, configurable: true });
+Object.defineProperty(obj, "z", { get: function () { return this.x; } });
+assertEq(obj.z, "undefined");
+
+Object.defineProperty(proto, "x", { get: Math.sin, configurable: false });
+assertEq(obj.z, NaN);
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -9465,16 +9465,17 @@ IonBuilder::freezePropertiesForCommonPro
             key = TypeSet::ObjectKey::get(key->proto().toObjectOrNull());
         }
     }
 }
 
 bool
 IonBuilder::testCommonGetterSetter(TemporaryTypeSet* types, PropertyName* name,
                                    bool isGetter, JSObject* foundProto, Shape* lastProperty,
+                                   JSFunction* getterOrSetter,
                                    MDefinition** guard,
                                    Shape* globalShape/* = nullptr*/,
                                    MDefinition** globalGuard/* = nullptr */)
 {
     MOZ_ASSERT_IF(globalShape, globalGuard);
     bool guardGlobal;
 
     // Check if all objects being accessed will lookup the name through foundProto.
@@ -9499,19 +9500,27 @@ IonBuilder::testCommonGetterSetter(Tempo
     // are no lookup hooks for this property.
     if (guardGlobal) {
         JSObject* obj = &script()->global();
         MDefinition* globalObj = constant(ObjectValue(*obj));
         *globalGuard = addShapeGuard(globalObj, globalShape, Bailout_ShapeGuard);
     }
 
     if (foundProto->isNative()) {
-        Shape* propShape = foundProto->as<NativeObject>().lookupPure(name);
-        if (propShape && !propShape->configurable())
-            return true;
+        NativeObject& nativeProto = foundProto->as<NativeObject>();
+        if (nativeProto.lastProperty() == lastProperty) {
+            // The proto shape is the same as it was at the point when we
+            // created the baseline IC, so looking up the prop on the object as
+            // it is now should be safe.
+            Shape* propShape = nativeProto.lookupPure(name);
+            MOZ_ASSERT_IF(isGetter, propShape->getterObject() == getterOrSetter);
+            MOZ_ASSERT_IF(!isGetter, propShape->setterObject() == getterOrSetter);
+            if (propShape && !propShape->configurable())
+                return true;
+        }
     }
 
     MInstruction* wrapper = constantMaybeNursery(foundProto);
     *guard = addShapeGuard(wrapper, lastProperty, Bailout_ShapeGuard);
     return true;
 }
 
 void
@@ -10241,18 +10250,18 @@ IonBuilder::getPropTryCommonGetter(bool*
         return true;
     }
 
     TemporaryTypeSet* objTypes = obj->resultTypeSet();
     MDefinition* guard = nullptr;
     MDefinition* globalGuard = nullptr;
     bool canUseTIForGetter =
         testCommonGetterSetter(objTypes, name, /* isGetter = */ true,
-                               foundProto, lastProperty, &guard, globalShape,
-                               &globalGuard);
+                               foundProto, lastProperty, commonGetter, &guard,
+                               globalShape, &globalGuard);
     if (!canUseTIForGetter) {
         // If type information is bad, we can still optimize the getter if we
         // shape guard.
         obj = addShapeGuardsForGetterSetter(obj, foundProto, lastProperty, receiverShapes,
                                             isOwnProperty);
         if (!obj)
             return false;
     }
@@ -10753,17 +10762,17 @@ IonBuilder::setPropTryCommonSetter(bool*
         trackOptimizationOutcome(TrackedOutcome::NoProtoFound);
         return true;
     }
 
     TemporaryTypeSet* objTypes = obj->resultTypeSet();
     MDefinition* guard = nullptr;
     bool canUseTIForSetter =
         testCommonGetterSetter(objTypes, name, /* isGetter = */ false,
-                               foundProto, lastProperty, &guard);
+                               foundProto, lastProperty, commonSetter, &guard);
     if (!canUseTIForSetter) {
         // If type information is bad, we can still optimize the setter if we
         // shape guard.
         obj = addShapeGuardsForGetterSetter(obj, foundProto, lastProperty, receiverShapes,
                                             isOwnProperty);
         if (!obj)
             return false;
     }
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -866,16 +866,17 @@ class IonBuilder
                                     bool isGetter, JSObject* foundProto, bool* guardGlobal);
     void freezePropertiesForCommonPrototype(TemporaryTypeSet* types, PropertyName* name,
                                             JSObject* foundProto, bool allowEmptyTypesForGlobal = false);
     /*
      * Callers must pass a non-null globalGuard if they pass a non-null globalShape.
      */
     bool testCommonGetterSetter(TemporaryTypeSet* types, PropertyName* name,
                                 bool isGetter, JSObject* foundProto, Shape* lastProperty,
+                                JSFunction* getterOrSetter,
                                 MDefinition** guard, Shape* globalShape = nullptr,
                                 MDefinition** globalGuard = nullptr);
     bool testShouldDOMCall(TypeSet* inTypes,
                            JSFunction* func, JSJitInfo::OpType opType);
 
     MDefinition* addShapeGuardsForGetterSetter(MDefinition* obj, JSObject* holder, Shape* holderShape,
                                                const BaselineInspector::ShapeVector& receiverShapes,
                                                bool isOwnProperty);