Bug 1127167 - Avoid creating mutant half-native half-non-native objects when making unboxed layouts, r=jandem.
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 11 Feb 2015 12:44:26 -0700
changeset 255854 825f6ee63f7f56db6c7c59ed7e852c9d430def01
parent 255853 141b3ae089e91d065c3e1c79ac21e74cac86854e
child 255855 8e4b8596954f36a823c750f43ebe354ef92c4dd4
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1127167
milestone38.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 1127167 - Avoid creating mutant half-native half-non-native objects when making unboxed layouts, r=jandem.
js/src/jit/BaselineIC.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/vm/Shape.cpp
js/src/vm/Shape.h
js/src/vm/TypeInference.cpp
js/src/vm/TypeInference.h
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -9330,34 +9330,38 @@ TryAttachCallStub(JSContext *cx, ICCall_
         // information, for use during Ion compilation.
         if (IsIonEnabled(cx))
             EnsureTrackPropertyTypes(cx, fun, NameToId(cx->names().prototype));
 
         // Remember the template object associated with any script being called
         // as a constructor, for later use during Ion compilation.
         RootedObject templateObject(cx);
         if (constructing) {
-            JSObject *thisObject = CreateThisForFunction(cx, fun, MaybeSingletonObject);
-            if (!thisObject)
-                return false;
-
-            if (thisObject->is<PlainObject>() || thisObject->is<UnboxedPlainObject>()) {
-                templateObject = thisObject;
-
-                // If we are calling a constructor for which the new script
-                // properties analysis has not been performed yet, don't attach a
-                // stub. After the analysis is performed, CreateThisForFunction may
-                // start returning objects with a different type, and the Ion
-                // compiler might get confused.
-                TypeNewScript *newScript = templateObject->group()->newScript();
-                if (newScript && !newScript->analyzed()) {
-                    // Clear the object just created from the preliminary objects
-                    // on the TypeNewScript, as it will not be used or filled in by
-                    // running code.
-                    newScript->unregisterNewObject(&templateObject->as<PlainObject>());
+            // If we are calling a constructor for which the new script
+            // properties analysis has not been performed yet, don't attach a
+            // stub. After the analysis is performed, CreateThisForFunction may
+            // start returning objects with a different type, and the Ion
+            // compiler will get confused.
+
+            // Only attach a stub if the function already has a prototype and
+            // we can look it up without causing side effects.
+            RootedValue protov(cx);
+            if (!GetPropertyPure(cx, fun, NameToId(cx->names().prototype), protov.address())) {
+                JitSpew(JitSpew_BaselineIC, "  Can't purely lookup function prototype");
+                return true;
+            }
+
+            if (protov.isObject()) {
+                TaggedProto proto(&protov.toObject());
+                ObjectGroup *group = ObjectGroup::defaultNewGroup(cx, nullptr, proto, fun);
+                if (!group)
+                    return false;
+
+                if (group->newScript() && !group->newScript()->analyzed()) {
+                    JitSpew(JitSpew_BaselineIC, "  Function newScript has not been analyzed");
                     return true;
                 }
             }
         }
 
         JitSpew(JitSpew_BaselineIC,
                 "  Generating Call_Scripted stub (fun=%p, %s:%d, cons=%s, spread=%s)",
                 fun.get(), fun->nonLazyScript()->filename(), fun->nonLazyScript()->lineno(),
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1349,23 +1349,23 @@ ClassProtoKeyOrAnonymousOrNull(const js:
     if (key != JSProto_Null)
         return key;
     if (clasp->flags & JSCLASS_IS_ANONYMOUS)
         return JSProto_Object;
     return JSProto_Null;
 }
 
 static inline bool
-NativeGetPureInline(NativeObject *pobj, Shape *shape, MutableHandleValue vp)
+NativeGetPureInline(NativeObject *pobj, Shape *shape, Value *vp)
 {
     if (shape->hasSlot()) {
-        vp.set(pobj->getSlot(shape->slot()));
-        MOZ_ASSERT(!vp.isMagic());
+        *vp = pobj->getSlot(shape->slot());
+        MOZ_ASSERT(!vp->isMagic());
     } else {
-        vp.setUndefined();
+        vp->setUndefined();
     }
 
     /* Fail if we have a custom getter. */
     return shape->hasDefaultGetter();
 }
 
 static bool
 FindClassPrototype(ExclusiveContext *cx, MutableHandleObject protop, const Class *clasp)
@@ -1394,17 +1394,17 @@ FindClassPrototype(ExclusiveContext *cx,
     if (ctor && ctor->is<JSFunction>()) {
         JSFunction *nctor = &ctor->as<JSFunction>();
         RootedValue v(cx);
         if (cx->isJSContext()) {
             if (!GetProperty(cx->asJSContext(), ctor, ctor, cx->names().prototype, &v))
                 return false;
         } else {
             Shape *shape = nctor->lookup(cx, cx->names().prototype);
-            if (!shape || !NativeGetPureInline(nctor, shape, &v))
+            if (!shape || !NativeGetPureInline(nctor, shape, v.address()))
                 return false;
         }
         if (v.isObject())
             protop.set(&v.toObject());
     }
     return true;
 }
 
@@ -1533,16 +1533,17 @@ js::NewObjectWithGroupCommon(JSContext *
 
     NewObjectCache &cache = cx->runtime()->newObjectCache;
 
     NewObjectCache::EntryIndex entry = -1;
     if (group->proto().isObject() &&
         parent == group->proto().toObject()->getParent() &&
         newKind == GenericObject &&
         group->clasp()->isNative() &&
+        (!group->newScript() || group->newScript()->analyzed()) &&
         !cx->compartment()->hasObjectMetadataCallback())
     {
         if (cache.lookupGroup(group, allocKind, &entry)) {
             JSObject *obj = cache.newObjectFromHit<NoGC>(cx, entry, GetInitialHeap(newKind, group->clasp()));
             if (obj) {
                 return obj;
             } else {
                 obj = cache.newObjectFromHit<CanGC>(cx, entry, GetInitialHeap(newKind, group->clasp()));
@@ -2184,26 +2185,35 @@ js::XDRObjectLiteral(XDRState<XDR_DECODE
 
 JSObject *
 js::CloneObjectLiteral(JSContext *cx, HandleObject parent, HandleObject srcObj)
 {
     if (srcObj->is<PlainObject>()) {
         AllocKind kind = GetBackgroundAllocKind(GuessObjectGCKind(srcObj->as<PlainObject>().numFixedSlots()));
         MOZ_ASSERT_IF(srcObj->isTenured(), kind == srcObj->asTenured().getAllocKind());
 
-        JSObject *proto = cx->global()->getOrCreateObjectPrototype(cx);
+        RootedObject proto(cx, cx->global()->getOrCreateObjectPrototype(cx));
         if (!proto)
             return nullptr;
         RootedObjectGroup group(cx, ObjectGroup::defaultNewGroup(cx, &PlainObject::class_,
                                                                  TaggedProto(proto)));
         if (!group)
             return nullptr;
 
-        RootedShape shape(cx, srcObj->lastProperty());
-        return NewReshapedObject(cx, group, parent, kind, shape);
+        RootedPlainObject res(cx, NewObjectWithGroup<PlainObject>(cx, group, parent, kind,
+                                                                  MaybeSingletonObject));
+        if (!res)
+            return nullptr;
+
+        RootedShape newShape(cx, ReshapeForParentAndAllocKind(cx, srcObj->lastProperty(),
+                                                              TaggedProto(proto), parent, kind));
+        if (!newShape || !NativeObject::setLastProperty(cx, res, newShape))
+            return nullptr;
+
+        return res;
     }
 
     RootedArrayObject srcArray(cx, &srcObj->as<ArrayObject>());
     MOZ_ASSERT(srcArray->denseElementsAreCopyOnWrite());
     MOZ_ASSERT(srcArray->getElementsHeader()->ownerObject() == srcObj);
 
     size_t length = srcArray->as<ArrayObject>().length();
     RootedArrayObject res(cx, NewDenseFullyAllocatedArray(cx, length, nullptr, MaybeSingletonObject));
@@ -3048,16 +3058,26 @@ js::LookupPropertyPure(ExclusiveContext 
     } while (obj);
 
     *objp = nullptr;
     *propp = nullptr;
     return true;
 }
 
 bool
+js::GetPropertyPure(ExclusiveContext *cx, JSObject *obj, jsid id, Value *vp)
+{
+    JSObject *pobj;
+    Shape *shape;
+    if (!LookupPropertyPure(cx, obj, id, &pobj, &shape))
+        return false;
+    return pobj->isNative() && NativeGetPureInline(&pobj->as<NativeObject>(), shape, vp);
+}
+
+bool
 JSObject::reportReadOnly(JSContext *cx, jsid id, unsigned report)
 {
     RootedValue val(cx, IdToValue(id));
     return js_ReportValueErrorFlags(cx, report, JSMSG_READ_ONLY,
                                     JSDVG_IGNORE_STACK, val, js::NullPtr(),
                                     nullptr, nullptr);
 }
 
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1202,16 +1202,19 @@ js_FindVariableScope(JSContext *cx, JSFu
 
 namespace js {
 
 bool
 LookupPropertyPure(ExclusiveContext *cx, JSObject *obj, jsid id, JSObject **objp,
                    Shape **propp);
 
 bool
+GetPropertyPure(ExclusiveContext *cx, JSObject *obj, jsid id, Value *vp);
+
+bool
 GetOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id,
                          MutableHandle<PropertyDescriptor> desc);
 
 bool
 GetOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id, MutableHandleValue vp);
 
 bool
 NewPropertyDescriptorObject(JSContext *cx, Handle<PropertyDescriptor> desc, MutableHandleValue vp);
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -651,21 +651,16 @@ template <typename T>
 inline T *
 NewObjectWithGroup(JSContext *cx, HandleObjectGroup group, JSObject *parent,
                    NewObjectKind newKind = GenericObject)
 {
     gc::AllocKind allocKind = gc::GetGCObjectKind(group->clasp());
     return NewObjectWithGroup<T>(cx, group, parent, allocKind, newKind);
 }
 
-JSObject *
-NewReshapedObject(JSContext *cx, HandleObjectGroup group, JSObject *parent,
-                  gc::AllocKind allocKind, HandleShape shape,
-                  NewObjectKind newKind = GenericObject);
-
 /*
  * As for gc::GetGCObjectKind, where numSlots is a guess at the final size of
  * the object, zero if the final size is unknown. This should only be used for
  * objects that do not require any fixed slots.
  */
 static inline gc::AllocKind
 GuessObjectGCKind(size_t numSlots)
 {
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -596,52 +596,44 @@ NativeObject::addPropertyInternal(Exclus
         obj->checkShapeConsistency();
         return shape;
     }
 
     obj->checkShapeConsistency();
     return nullptr;
 }
 
-JSObject *
-js::NewReshapedObject(JSContext *cx, HandleObjectGroup group, JSObject *parent,
-                      gc::AllocKind allocKind, HandleShape shape, NewObjectKind newKind)
+Shape *
+js::ReshapeForParentAndAllocKind(JSContext *cx, Shape *shape, TaggedProto proto, JSObject *parent,
+                                 gc::AllocKind allocKind)
 {
-    RootedPlainObject res(cx, NewObjectWithGroup<PlainObject>(cx, group, parent, allocKind, newKind));
-    if (!res)
-        return nullptr;
+    // Compute the number of fixed slots with the new allocation kind.
+    size_t nfixed = gc::GetGCKindSlots(allocKind, shape->getObjectClass());
 
-    if (shape->isEmptyShape())
-        return res;
-
-    /* Get all the ids in the object, in order. */
+    // Get all the ids in the shape, in order.
     js::AutoIdVector ids(cx);
     {
-        for (unsigned i = 0; i <= shape->slot(); i++) {
+        for (unsigned i = 0; i < shape->slotSpan(); i++) {
             if (!ids.append(JSID_VOID))
                 return nullptr;
         }
         Shape *nshape = shape;
         while (!nshape->isEmptyShape()) {
             ids[nshape->slot()].set(nshape->propid());
             nshape = nshape->previous();
         }
     }
 
-    /* Construct the new shape, without updating type information. */
+    // Construct the new shape, without updating type information.
     RootedId id(cx);
-    RootedShape newShape(cx, EmptyShape::getInitialShape(cx, res->getClass(),
-                                                         res->getTaggedProto(),
-                                                         res->getParent(),
-                                                         res->getMetadata(),
-                                                         res->numFixedSlots(),
-                                                         shape->getObjectFlags()));
+    RootedShape newShape(cx, EmptyShape::getInitialShape(cx, shape->getObjectClass(),
+                                                         proto, parent, shape->getObjectMetadata(),
+                                                         nfixed, shape->getObjectFlags()));
     for (unsigned i = 0; i < ids.length(); i++) {
         id = ids[i];
-        MOZ_ASSERT(!res->contains(cx, id));
 
         uint32_t index;
         bool indexed = js_IdIsIndex(id, &index);
 
         Rooted<UnownedBaseShape*> nbase(cx, newShape->base()->unowned());
         if (indexed) {
             StackBaseShape base(nbase);
             base.flags |= BaseShape::INDEXED;
@@ -649,21 +641,19 @@ js::NewReshapedObject(JSContext *cx, Han
             if (!nbase)
                 return nullptr;
         }
 
         StackShape child(nbase, id, i, JSPROP_ENUMERATE, 0);
         newShape = cx->compartment()->propertyTree.getChild(cx, newShape, child);
         if (!newShape)
             return nullptr;
-        if (!NativeObject::setLastProperty(cx, res, newShape))
-            return nullptr;
     }
 
-    return res;
+    return newShape;
 }
 
 /*
  * Check and adjust the new attributes for the shape to make sure that our
  * slot access optimizations are sound. It is responsibility of the callers to
  * enforce all restrictions from ECMA-262 v5 8.12.9 [[DefineOwnProperty]].
  */
 static inline bool
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -1548,16 +1548,20 @@ MarkDenseOrTypedArrayElementFound(typena
 }
 
 static inline bool
 IsImplicitDenseOrTypedArrayElement(Shape *prop)
 {
     return prop == reinterpret_cast<Shape*>(1);
 }
 
+Shape *
+ReshapeForParentAndAllocKind(JSContext *cx, Shape *shape, TaggedProto proto, JSObject *parent,
+                             gc::AllocKind allocKind);
+
 } // namespace js
 
 #ifdef _MSC_VER
 #pragma warning(pop)
 #pragma warning(pop)
 #endif
 
 // JS::ubi::Nodes can point to Shapes and BaseShapes; they're js::gc::Cell
--- a/js/src/vm/TypeInference.cpp
+++ b/js/src/vm/TypeInference.cpp
@@ -3147,29 +3147,16 @@ PreliminaryObjectArray::registerNewObjec
             objects[i] = res;
             return;
         }
     }
 
     MOZ_CRASH("There should be room for registering the new object");
 }
 
-void
-PreliminaryObjectArray::unregisterNewObject(JSObject *res)
-{
-    for (size_t i = 0; i < COUNT; i++) {
-        if (objects[i] == res) {
-            objects[i] = nullptr;
-            return;
-        }
-    }
-
-    MOZ_CRASH("The object should be one of the preliminary objects");
-}
-
 bool
 PreliminaryObjectArray::full() const
 {
     for (size_t i = 0; i < COUNT; i++) {
         if (!objects[i])
             return false;
     }
     return true;
@@ -3235,23 +3222,16 @@ TypeNewScript::registerNewObject(PlainOb
     // New script objects must have the maximum number of fixed slots, so that
     // we can adjust their shape later to match the number of fixed slots used
     // by the template object we eventually create.
     MOZ_ASSERT(res->numFixedSlots() == NativeObject::MAX_FIXED_SLOTS);
 
     preliminaryObjects->registerNewObject(res);
 }
 
-void
-TypeNewScript::unregisterNewObject(PlainObject *res)
-{
-    MOZ_ASSERT(!analyzed());
-    preliminaryObjects->unregisterNewObject(res);
-}
-
 // Return whether shape consists entirely of plain data properties.
 static bool
 OnlyHasDataProperties(Shape *shape)
 {
     MOZ_ASSERT(!shape->inDictionary());
 
     while (!shape->isEmptyShape()) {
         if (!shape->isDataDescriptor() ||
@@ -3289,24 +3269,23 @@ CommonPrefix(Shape *first, Shape *second
     return first;
 }
 
 static bool
 ChangeObjectFixedSlotCount(JSContext *cx, PlainObject *obj, gc::AllocKind allocKind)
 {
     MOZ_ASSERT(OnlyHasDataProperties(obj->lastProperty()));
 
-    // Make a clone of the object, with the new allocation kind.
-    RootedShape oldShape(cx, obj->lastProperty());
-    RootedObjectGroup group(cx, obj->group());
-    JSObject *clone = NewReshapedObject(cx, group, obj->getParent(), allocKind, oldShape);
-    if (!clone)
+    Shape *newShape = ReshapeForParentAndAllocKind(cx, obj->lastProperty(),
+                                                   obj->getTaggedProto(), obj->getParent(),
+                                                   allocKind);
+    if (!newShape)
         return false;
 
-    obj->setLastPropertyShrinkFixedSlots(clone->lastProperty());
+    obj->setLastPropertyShrinkFixedSlots(newShape);
     return true;
 }
 
 namespace {
 
 struct DestroyTypeNewScript
 {
     JSContext *cx;
@@ -3484,20 +3463,25 @@ TypeNewScript::maybeAnalyze(JSContext *c
     preliminaryObjects = nullptr;
 
     if (group->maybeUnboxedLayout()) {
         // An unboxed layout was constructed for the group, and this has already
         // been hooked into it.
         MOZ_ASSERT(group->unboxedLayout().newScript() == this);
         destroyNewScript.group = nullptr;
 
-        // Clear out the template object. This is not used for TypeNewScripts
-        // with an unboxed layout, and additionally this template is now a
-        // mutant object with a non-native class and native shape, and must be
-        // collected by the next GC.
+        // Clear out the template object, which is not used for TypeNewScripts
+        // with an unboxed layout. Currently it is a mutant object with a
+        // non-native group and native shape, so make it safe for GC by changing
+        // its group to the default for its prototype.
+        ObjectGroup *plainGroup = ObjectGroup::defaultNewGroup(cx, &PlainObject::class_,
+                                                               group->proto());
+        if (!plainGroup)
+            CrashAtUnhandlableOOM("TypeNewScript::maybeAnalyze");
+        templateObject_->setGroup(plainGroup);
         templateObject_ = nullptr;
 
         return true;
     }
 
     if (prefixShape->slotSpan() == templateObject()->slotSpan()) {
         // The definite properties analysis found exactly the properties that
         // are held in common by the preliminary objects. No further analysis
--- a/js/src/vm/TypeInference.h
+++ b/js/src/vm/TypeInference.h
@@ -748,17 +748,16 @@ class PreliminaryObjectArray
     JSObject *objects[COUNT];
 
   public:
     PreliminaryObjectArray() {
         mozilla::PodZero(this);
     }
 
     void registerNewObject(JSObject *res);
-    void unregisterNewObject(JSObject *res);
 
     JSObject *get(size_t i) const {
         MOZ_ASSERT(i < COUNT);
         return objects[i];
     }
 
     bool full() const;
     void sweep();
@@ -884,17 +883,16 @@ class TypeNewScript
     JSFunction *function() const {
         return function_;
     }
 
     void trace(JSTracer *trc);
     void sweep();
 
     void registerNewObject(PlainObject *res);
-    void unregisterNewObject(PlainObject *res);
     bool maybeAnalyze(JSContext *cx, ObjectGroup *group, bool *regenerate, bool force = false);
 
     bool rollbackPartiallyInitializedObjects(JSContext *cx, ObjectGroup *group);
 
     static void make(JSContext *cx, ObjectGroup *group, JSFunction *fun);
 
     size_t sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 };