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 166814 1c9290033b333a3765ef16c45bf752fea0ba326f
parent 166813 b54e8c328c327943fe5662e9e84bf748a1f97564
child 166815 5e355d51e30f28d2d3254312922b6eaacc4a5914
push id39290
push usernmatsakis@mozilla.com
push dateTue, 04 Feb 2014 20:30:40 +0000
treeherdermozilla-inbound@1c9290033b33 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs950458
milestone30.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 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);