Bug 950458 - Emit type barrier for derived typed objects r=jandem
authorNicholas D. Matsakis <nmatsakis@mozilla.com>
Thu, 30 Jan 2014 23:52:25 -0500
changeset 167176 1c9290033b333a3765ef16c45bf752fea0ba326f
parent 167175 b54e8c328c327943fe5662e9e84bf748a1f97564
child 167177 5e355d51e30f28d2d3254312922b6eaacc4a5914
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersjandem
bugs950458
milestone30.0a1
Bug 950458 - Emit type barrier for derived typed objects r=jandem
js/src/jit-test/tests/TypedObject/bug950458.js
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/TypedObject/bug950458.js
@@ -0,0 +1,23 @@
+if (!this.hasOwnProperty("TypedObject"))
+  quit();
+
+// Test for fuzz condition failure. Cause of the failure
+// was that we were not adding a type barrier after the
+// creation of derived typed objects. When run in --ion-eager
+// mode, arr[i] (below) would yield a derived typed object
+// with an empty type set, which would then fail sanity
+// checking assertions.
+//
+// Public domain.
+
+var N = 100;
+var T = TypedObject;
+var Point = new T.StructType({x: T.uint32, y: T.uint32, z: T.uint32});
+var PointArray = Point.array();
+function foo(arr) {
+  var sum = 0;
+  for (var i = 0; i < N; i++) {
+    sum += arr[i].x + arr[i].y + arr[i].z;
+  }
+}
+foo(new PointArray(N));
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -6724,34 +6724,48 @@ IonBuilder::getElemTryScalarElemOfTypedO
         return true;
     ScalarTypeRepresentation *elemTypeRepr = elemTypeReprs.getTypeRepresentation()->asScalar();
     JS_ASSERT(elemSize == elemTypeRepr->alignment());
 
     MDefinition *indexAsByteOffset;
     if (!checkTypedObjectIndexInBounds(elemSize, obj, index, &indexAsByteOffset, objTypeReprs))
         return false;
 
+    return pushScalarLoadFromTypedObject(emitted, obj, indexAsByteOffset, elemTypeRepr);
+}
+
+bool
+IonBuilder::pushScalarLoadFromTypedObject(bool *emitted,
+                                          MDefinition *obj,
+                                          MDefinition *offset,
+                                          ScalarTypeRepresentation *elemTypeRepr)
+{
+    size_t size = elemTypeRepr->size();
+    JS_ASSERT(size == elemTypeRepr->alignment());
+
     // Find location within the owner object.
     MDefinition *elements, *scaledOffset;
-    loadTypedObjectElements(obj, indexAsByteOffset, elemSize, &elements, &scaledOffset);
+    loadTypedObjectElements(obj, offset, size, &elements, &scaledOffset);
 
     // Load the element.
     MLoadTypedArrayElement *load = MLoadTypedArrayElement::New(alloc(), elements, scaledOffset, elemTypeRepr->type());
     current->add(load);
     current->push(load);
 
     // If we are reading in-bounds elements, we can use knowledge about
     // the array type to determine the result type, even if the opcode has
     // never executed. The known pushed type is only used to distinguish
     // uint32 reads that may produce either doubles or integers.
     types::TemporaryTypeSet *resultTypes = bytecodeTypes(pc);
     bool allowDouble = resultTypes->hasType(types::Type::DoubleType());
+
     // Note: knownType is not necessarily in resultTypes; e.g. if we
     // have only observed integers coming out of float array.
     MIRType knownType = MIRTypeForTypedArrayRead(elemTypeRepr->type(), allowDouble);
+
     // Note: we can ignore the type barrier here, we know the type must
     // be valid and unbarriered. Also, need not set resultTypeSet,
     // because knownType is scalar and a resultTypeSet would provide
     // no useful additional info.
     load->setResultType(knownType);
 
     *emitted = true;
     return true;
@@ -6763,37 +6777,68 @@ IonBuilder::getElemTryComplexElemOfTyped
                                                MDefinition *index,
                                                TypeRepresentationSet objTypeReprs,
                                                TypeRepresentationSet elemTypeReprs,
                                                size_t elemSize)
 {
     JS_ASSERT(objTypeReprs.allOfArrayKind());
 
     MDefinition *type = loadTypedObjectType(obj);
-    MDefinition *elemType = typeObjectForElementFromArrayStructType(type);
+    MDefinition *elemTypeObj = typeObjectForElementFromArrayStructType(type);
 
     MDefinition *indexAsByteOffset;
     if (!checkTypedObjectIndexInBounds(elemSize, obj, index, &indexAsByteOffset, objTypeReprs))
         return false;
 
+    return pushDerivedTypedObject(emitted, obj, indexAsByteOffset,
+                                  elemTypeReprs, elemTypeObj);
+}
+
+bool
+IonBuilder::pushDerivedTypedObject(bool *emitted,
+                                   MDefinition *obj,
+                                   MDefinition *offset,
+                                   TypeRepresentationSet derivedTypeReprs,
+                                   MDefinition *derivedTypeObj)
+{
     // Find location within the owner object.
     MDefinition *owner, *ownerOffset;
-    loadTypedObjectData(obj, indexAsByteOffset, &owner, &ownerOffset);
-
-    // Create the derived type object.
-    MInstruction *derived = MNewDerivedTypedObject::New(alloc(),
-                                                        elemTypeReprs,
-                                                        elemType,
-                                                        owner,
-                                                        ownerOffset);
-
+    loadTypedObjectData(obj, offset, &owner, &ownerOffset);
+
+    // Create the derived datum.
+    MInstruction *derivedTypedObj = MNewDerivedTypedObject::New(alloc(),
+                                                                derivedTypeReprs,
+                                                                derivedTypeObj,
+                                                                owner,
+                                                                ownerOffset);
+    current->add(derivedTypedObj);
+    current->push(derivedTypedObj);
+
+    // Insert a barrier. This is necessary because, while we know from
+    // the inputs that the result of this access operation will be a
+    // derived typed object, and we know the set of type descriptor(s)
+    // it will be associated with (`derivedDescrs`), we do *not* know
+    // precisely what TI type object the result will have at
+    // runtime. The observed type set could be incomplete for two
+    // reasons:
+    //
+    // 1. We may simply not have executed this instruction yet.
+    //    This occurs frequently with --ion-eager but can happen
+    //    under other scenarios as well.
+    //
+    // 2. Users can mutate the prototypes of descriptors,
+    //    and hence a single descriptor can be associated with multiple
+    //    type objects over the course of the execution. Therefore,
+    //    even if we have executed this instruction, the TI type object
+    //    of the result might be different this time around from
+    //    previous executions.
     types::TemporaryTypeSet *resultTypes = bytecodeTypes(pc);
-    derived->setResultTypeSet(resultTypes);
-    current->add(derived);
-    current->push(derived);
+    if (!pushTypeBarrier(derivedTypedObj, resultTypes, true))
+        return false;
+
     *emitted = true;
     return true;
 }
 
 bool
 IonBuilder::getElemTryDense(bool *emitted, MDefinition *obj, MDefinition *index)
 {
     JS_ASSERT(*emitted == false);
@@ -8446,39 +8491,18 @@ IonBuilder::getPropTryScalarPropOfTypedO
     if (!fieldTypeReprs.singleton())
         return true;
     ScalarTypeRepresentation *fieldTypeRepr = fieldTypeReprs.getTypeRepresentation()->asScalar();
 
     // OK, perform the optimization
 
     MDefinition *typedObj = current->pop();
 
-    // Find location within the owner object.
-    MDefinition *elements, *scaledOffset;
-    loadTypedObjectElements(typedObj, constantInt(fieldOffset),
-                            fieldTypeRepr->alignment(),
-                            &elements, &scaledOffset);
-
-    // Reading from an Uint32Array will result in a double for values
-    // that don't fit in an int32. We have to bailout if this happens
-    // and the instruction is not known to return a double.
-    bool allowDouble = resultTypes->hasType(types::Type::DoubleType());
-    MIRType knownType = MIRTypeForTypedArrayRead(fieldTypeRepr->type(), allowDouble);
-
-    MLoadTypedArrayElement *load =
-        MLoadTypedArrayElement::New(alloc(), elements, scaledOffset,
-                                    fieldTypeRepr->type());
-
-    // Note: need not set resultTypeSet because knownType is scalar
-    // and a resultTypeSet would provide no useful additional info.
-    load->setResultType(knownType);
-    current->add(load);
-    current->push(load);
-    *emitted = true;
-    return true;
+    return pushScalarLoadFromTypedObject(emitted, typedObj, constantInt(fieldOffset),
+                                         fieldTypeRepr);
 }
 
 bool
 IonBuilder::getPropTryComplexPropOfTypedObject(bool *emitted,
                                                int32_t fieldOffset,
                                                TypeRepresentationSet fieldTypeReprs,
                                                size_t fieldIndex,
                                                types::TemporaryTypeSet *resultTypes)
@@ -8489,34 +8513,20 @@ IonBuilder::getPropTryComplexPropOfTyped
         return true;
 
     // OK, perform the optimization
 
     MDefinition *typedObj = current->pop();
 
     // Identify the type object for the field.
     MDefinition *type = loadTypedObjectType(typedObj);
-    MDefinition *fieldType = typeObjectForFieldFromStructType(type, fieldIndex);
-
-    // Find location within the owner object.
-    MDefinition *owner, *ownerOffset;
-    loadTypedObjectData(typedObj, constantInt(fieldOffset),
-                        &owner, &ownerOffset);
-
-    // Create the derived type object.
-    MInstruction *derived = MNewDerivedTypedObject::New(alloc(),
-                                                        fieldTypeReprs,
-                                                        fieldType,
-                                                        owner,
-                                                        ownerOffset);
-    derived->setResultTypeSet(resultTypes);
-    current->add(derived);
-    current->push(derived);
-    *emitted = true;
-    return true;
+    MDefinition *fieldTypeObj = typeObjectForFieldFromStructType(type, fieldIndex);
+
+    return pushDerivedTypedObject(emitted, typedObj, constantInt(fieldOffset),
+                                  fieldTypeReprs, fieldTypeObj);
 }
 
 bool
 IonBuilder::getPropTryDefiniteSlot(bool *emitted, PropertyName *name,
                                    bool barrier, types::TemporaryTypeSet *types)
 {
     JS_ASSERT(*emitted == false);
     types::HeapTypeSetKey property;
@@ -9890,17 +9900,16 @@ IonBuilder::loadTypedObjectData(MDefinit
     JS_ASSERT(offset->type() == MIRType_Int32);
 
     // Shortcircuit derived type objects, meaning the intermediate
     // objects created to represent `a.b` in an expression like
     // `a.b.c`. In that case, the owned and a base offset can be
     // pulled from the operands of the instruction and combined with
     // `offset`.
     if (typedObj->isNewDerivedTypedObject()) {
-        // If we see that the
         MNewDerivedTypedObject *ins = typedObj->toNewDerivedTypedObject();
 
         MAdd *offsetAdd = MAdd::NewAsmJS(alloc(), ins->offset(), offset, MIRType_Int32);
         current->add(offsetAdd);
 
         *owner = ins->owner();
         *ownerOffset = offsetAdd;
         return;
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -457,16 +457,25 @@ class IonBuilder : public MIRGenerator
                                      MDefinition *offset,
                                      ScalarTypeRepresentation *typeRepr,
                                      MDefinition *value);
     bool checkTypedObjectIndexInBounds(size_t elemSize,
                                        MDefinition *obj,
                                        MDefinition *index,
                                        MDefinition **indexAsByteOffset,
                                        TypeRepresentationSet objTypeReprs);
+    bool pushDerivedTypedObject(bool *emitted,
+                                MDefinition *obj,
+                                MDefinition *offset,
+                                TypeRepresentationSet derivedTypeReprs,
+                                MDefinition *derivedTypeObj);
+    bool pushScalarLoadFromTypedObject(bool *emitted,
+                                       MDefinition *obj,
+                                       MDefinition *offset,
+                                       ScalarTypeRepresentation* type);
 
     // jsop_setelem() helpers.
     bool setElemTryTypedArray(bool *emitted, MDefinition *object,
                          MDefinition *index, MDefinition *value);
     bool setElemTryTypedObject(bool *emitted, MDefinition *obj,
                                MDefinition *index, MDefinition *value);
     bool setElemTryTypedStatic(bool *emitted, MDefinition *object,
                                MDefinition *index, MDefinition *value);