Dense arrays should have numFixedSlots() == 0, regardless of size class. bug 704348

js/src/jsarray.cpp

@@ -1376,7 +1376,7 @@ JSObject::makeDenseArraySlow(JSContext *cx)
                                                oldShape->getObjectParent(), kind);
     if (!shape)
         return false;
-    setLastPropertyInfallible(shape);
+    this->shape_ = shape;
 
     /* Take ownership of the dense elements, reset to an empty dense array. */
     HeapValue *elems = elements;
@@ -1390,7 +1390,7 @@ JSObject::makeDenseArraySlow(JSContext *cx)
      * The getter/setter here will directly access the object's private value.
      */
     if (!AddLengthProperty(cx, this)) {
-        setLastPropertyInfallible(oldShape);
+        this->shape_ = oldShape;
         cx->free_(getElementsHeader());
         elements = elems;
         return false;
@@ -1410,7 +1410,7 @@ JSObject::makeDenseArraySlow(JSContext *cx)
             continue;
 
         if (!addDataProperty(cx, id, next, JSPROP_ENUMERATE)) {
-            JS_ALWAYS_TRUE(setLastProperty(cx, oldShape));
+            this->shape_ = oldShape;
             cx->free_(getElementsHeader());
             elements = elems;
             return false;
@@ -3900,8 +3900,12 @@ NewArray(JSContext *cx, jsuint length, JSObject *proto)
     if (!type)
         return NULL;
 
+    /*
+     * Get a shape with zero fixed slots, regardless of the size class.
+     * See JSObject::createDenseArray.
+     */
     Shape *shape = EmptyShape::getInitialShape(cx, &ArrayClass, proto,
-                                               proto->getParent(), kind);
+                                               proto->getParent(), gc::FINALIZE_OBJECT0);
     if (!shape)
         return NULL;
 

js/src/jsobj.h

@@ -423,9 +423,9 @@ extern HeapValue *emptyObjectElements;
  * object and the possible types of its properties.
  *
  * The rest of the object stores its named properties and indexed elements.
- * These are stored separately from one another. Objects are followed by a
- * variable-sized array of fixed slots, which may be used by either properties
- * or elements.
+ * These are stored separately from one another. Objects are followed by an
+ * variable-sized array of values for inline storage, which may be used by
+ * either properties of native objects (fixed slots) or by elements.
  *
  * Two native objects with the same shape are guaranteed to have the same
  * number of fixed slots.
@@ -437,9 +437,9 @@ extern HeapValue *emptyObjectElements;
  * 'slots' member is NULL.
  *
  * Elements are indexed via the 'elements' member. This member can point to
- * either the shared emptyObjectElements singleton, into the fixed slots (the
- * address of the third fixed slot, to leave room for a ObjectElements header)
- * or to a dynamically allocated array.
+ * either the shared emptyObjectElements singleton, into the inline value array
+ * (the address of the third value, to leave room for a ObjectElements header;
+ * in this case numFixedSlots() is zero) or to a dynamically allocated array.
  *
  * Only certain combinations of properties and elements storage are currently
  * possible. This will be changing soon :XXX: bug 586842.

js/src/jsobjinlines.h

@@ -1068,10 +1068,20 @@ JSObject::createDenseArray(JSContext *cx, js::gc::AllocKind kind,
 {
     JS_ASSERT(shape && type);
     JS_ASSERT(shape->getObjectClass() == &js::ArrayClass);
-    JS_ASSERT(js::gc::GetGCKindSlots(kind, shape->getObjectClass()) == shape->numFixedSlots());
 
-    JS_STATIC_ASSERT(sizeof(js::ObjectElements) == 2 * sizeof(js::Value));
-    JS_ASSERT(shape->numFixedSlots() >= 2);
+    /*
+     * Dense arrays are non-native, and never have properties to store.
+     * The number of fixed slots in the shape of such objects is zero.
+     */
+    JS_ASSERT(shape->numFixedSlots() == 0);
+
+    /*
+     * The array initially stores its elements inline, there must be enough
+     * space for an elements header.
+     */
+    JS_ASSERT(js::gc::GetGCKindSlots(kind) >= js::ObjectElements::VALUES_PER_HEADER);
+
+    uint32 capacity = js::gc::GetGCKindSlots(kind) - js::ObjectElements::VALUES_PER_HEADER;
 
     JSObject *obj = js_NewGCObject(cx, kind);
     if (!obj)
@@ -1081,7 +1091,7 @@ JSObject::createDenseArray(JSContext *cx, js::gc::AllocKind kind,
     obj->type_.init(type);
     obj->slots = NULL;
     obj->setFixedElements();
-    new (obj->getElementsHeader()) js::ObjectElements(shape->numFixedSlots() - 2, length);
+    new (obj->getElementsHeader()) js::ObjectElements(capacity, length);
 
     return obj;
 }
@@ -1241,10 +1251,7 @@ JSObject::hasPropertyTable() const
 inline size_t
 JSObject::structSize() const
 {
-    if (isFunction())
-        return sizeof(JSFunction);
-    uint32 nfixed = numFixedSlots() + (hasPrivate() ? 1 : 0);
-    return sizeof(JSObject) + (nfixed * sizeof(js::Value));
+    return arenaHeader()->getThingSize();
 }
 
 inline size_t

js/src/methodjit/BaseAssembler.h

@@ -188,9 +188,12 @@ static const JSC::MacroAssembler::RegisterID JSParamReg_Argc  = JSC::SparcRegist
         loadPtr(Address(obj, JSObject::offsetOfShape()), shape);
     }
 
+    Jump guardShape(RegisterID objReg, const Shape *shape) {
+        return branchPtr(NotEqual, Address(objReg, JSObject::offsetOfShape()), ImmPtr(shape));
+    }
+
     Jump guardShape(RegisterID objReg, JSObject *obj) {
-        return branchPtr(NotEqual, Address(objReg, JSObject::offsetOfShape()),
-                         ImmPtr(obj->lastProperty()));
+        return guardShape(objReg, obj->lastProperty());
     }
 
     /*

js/src/methodjit/FastOps.cpp

@@ -1481,6 +1481,20 @@ mjit::Compiler::tryConvertInteger(FrameEntry *fe, Uses uses)
     frame.learnType(fe, JSVAL_TYPE_INT32, reg);
 }
 
+/* Get the common shape used by all dense arrays with a prototype at globalObj. */
+static inline Shape *
+GetDenseArrayShape(JSContext *cx, JSObject *globalObj)
+{
+    JS_ASSERT(globalObj);
+
+    JSObject *proto;
+    if (!js_GetClassPrototype(cx, globalObj, JSProto_Array, &proto, NULL))
+        return false;
+
+    return EmptyShape::getInitialShape(cx, &ArrayClass, proto,
+                                       proto->getParent(), gc::FINALIZE_OBJECT0);
+}
+
 bool
 mjit::Compiler::jsop_setelem(bool popGuaranteed)
 {
@@ -1520,7 +1534,7 @@ mjit::Compiler::jsop_setelem(bool popGuaranteed)
 #endif
     }
 
-    if (id->isType(JSVAL_TYPE_DOUBLE)) {
+    if (id->isType(JSVAL_TYPE_DOUBLE) || !globalObj) {
         jsop_setelem_slow();
         return true;
     }
@@ -1616,11 +1630,12 @@ mjit::Compiler::jsop_setelem(bool popGuaranteed)
     ic.slowPathStart = stubcc.syncExit(Uses(3));
 
     // Guard obj is a dense array.
-    ic.shapeGuard = masm.testObjClass(Assembler::NotEqual, ic.objReg, ic.objReg, &ArrayClass);
+    Shape *shape = GetDenseArrayShape(cx, globalObj);
+    if (!shape)
+        return false;
+    ic.shapeGuard = masm.guardShape(ic.objReg, shape);
     stubcc.linkExitDirect(ic.shapeGuard, ic.slowPathStart);
 
-    masm.rematPayload(ic.objRemat, ic.objReg);
-
     // Load the dynamic elements vector.
     masm.loadPtr(Address(ic.objReg, JSObject::offsetOfElements()), ic.objReg);
 
@@ -2115,7 +2130,7 @@ mjit::Compiler::jsop_getelem(bool isCall)
 
     frame.forgetMismatchedObject(obj);
 
-    if (id->isType(JSVAL_TYPE_DOUBLE)) {
+    if (id->isType(JSVAL_TYPE_DOUBLE) || !globalObj) {
         if (isCall)
             jsop_callelem_slow();
         else
@@ -2192,7 +2207,10 @@ mjit::Compiler::jsop_getelem(bool isCall)
         }
 
         // Guard obj is a dense array.
-        ic.shapeGuard = masm.testObjClass(Assembler::NotEqual, ic.objReg, ic.typeReg, &ArrayClass);
+        Shape *shape = GetDenseArrayShape(cx, globalObj);
+        if (!shape)
+            return false;
+        ic.shapeGuard = masm.guardShape(ic.objReg, shape);
         stubcc.linkExitDirect(ic.shapeGuard, ic.slowPathStart);
 
         Int32Key key = id->isConstant()