From f93fe5bf9e59908d512b4bc00ed2ca326e74fe1d Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Wed, 11 Feb 2015 17:47:13 -0700 Subject: [PATCH] Bug 1127167 - Backout 825f6ee63f7f for causing massive regressions on a CLOSED TREE --- js/src/jit/BaselineIC.cpp | 36 ++++++++++++++---------------- js/src/jsobj.cpp | 36 +++++++----------------------- js/src/jsobj.h | 3 --- js/src/jsobjinlines.h | 5 +++++ js/src/vm/Shape.cpp | 34 ++++++++++++++++++---------- js/src/vm/Shape.h | 4 ---- js/src/vm/TypeInference.cpp | 44 +++++++++++++++++++++++++------------ js/src/vm/TypeInference.h | 2 ++ 8 files changed, 83 insertions(+), 81 deletions(-) diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp index 959061b845e2..fd93b0b115f5 100644 --- a/js/src/jit/BaselineIC.cpp +++ b/js/src/jit/BaselineIC.cpp @@ -9335,28 +9335,24 @@ TryAttachCallStub(JSContext *cx, ICCall_Fallback *stub, HandleScript script, jsb // as a constructor, for later use during Ion compilation. RootedObject templateObject(cx); if (constructing) { - // 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. + JSObject *thisObject = CreateThisForFunction(cx, fun, MaybeSingletonObject); + if (!thisObject) + return false; - // 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 (thisObject->is() || thisObject->is()) { + templateObject = thisObject; - 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"); + // 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()); return true; } } diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index c0f7fc2c4574..11a6b71bcd42 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -1305,13 +1305,13 @@ ClassProtoKeyOrAnonymousOrNull(const js::Class *clasp) } static inline bool -NativeGetPureInline(NativeObject *pobj, Shape *shape, Value *vp) +NativeGetPureInline(NativeObject *pobj, Shape *shape, MutableHandleValue vp) { if (shape->hasSlot()) { - *vp = pobj->getSlot(shape->slot()); - MOZ_ASSERT(!vp->isMagic()); + vp.set(pobj->getSlot(shape->slot())); + MOZ_ASSERT(!vp.isMagic()); } else { - vp->setUndefined(); + vp.setUndefined(); } /* Fail if we have a custom getter. */ @@ -1350,7 +1350,7 @@ FindClassPrototype(ExclusiveContext *cx, MutableHandleObject protop, const Class return false; } else { Shape *shape = nctor->lookup(cx, cx->names().prototype); - if (!shape || !NativeGetPureInline(nctor, shape, v.address())) + if (!shape || !NativeGetPureInline(nctor, shape, &v)) return false; } if (v.isObject()) @@ -1487,7 +1487,6 @@ js::NewObjectWithGroupCommon(JSContext *cx, HandleObjectGroup group, JSObject *p parent == group->proto().toObject()->getParent() && newKind == GenericObject && group->clasp()->isNative() && - (!group->newScript() || group->newScript()->analyzed()) && !cx->compartment()->hasObjectMetadataCallback()) { if (cache.lookupGroup(group, allocKind, &entry)) { @@ -2140,7 +2139,7 @@ js::CloneObjectLiteral(JSContext *cx, HandleObject parent, HandleObject srcObj) AllocKind kind = GetBackgroundAllocKind(GuessObjectGCKind(srcObj->as().numFixedSlots())); MOZ_ASSERT_IF(srcObj->isTenured(), kind == srcObj->asTenured().getAllocKind()); - RootedObject proto(cx, cx->global()->getOrCreateObjectPrototype(cx)); + JSObject *proto = cx->global()->getOrCreateObjectPrototype(cx); if (!proto) return nullptr; RootedObjectGroup group(cx, ObjectGroup::defaultNewGroup(cx, &PlainObject::class_, @@ -2148,17 +2147,8 @@ js::CloneObjectLiteral(JSContext *cx, HandleObject parent, HandleObject srcObj) if (!group) return nullptr; - RootedPlainObject res(cx, NewObjectWithGroup(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; + RootedShape shape(cx, srcObj->lastProperty()); + return NewReshapedObject(cx, group, parent, kind, shape); } RootedArrayObject srcArray(cx, &srcObj->as()); @@ -3012,16 +3002,6 @@ js::LookupPropertyPure(ExclusiveContext *cx, JSObject *obj, jsid id, JSObject ** 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(), shape, vp); -} - bool JSObject::reportReadOnly(JSContext *cx, jsid id, unsigned report) { diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 27ca10ebcc7a..d044b2cdd219 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -1199,9 +1199,6 @@ 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 desc); diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index 379fd76c0b0b..525b8f53a651 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -648,6 +648,11 @@ NewObjectWithGroup(JSContext *cx, HandleObjectGroup group, JSObject *parent, return NewObjectWithGroup(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 diff --git a/js/src/vm/Shape.cpp b/js/src/vm/Shape.cpp index 2655b1b36a6e..8c54d180d948 100644 --- a/js/src/vm/Shape.cpp +++ b/js/src/vm/Shape.cpp @@ -601,17 +601,21 @@ NativeObject::addPropertyInternal(ExclusiveContext *cx, return nullptr; } -Shape * -js::ReshapeForParentAndAllocKind(JSContext *cx, Shape *shape, TaggedProto proto, JSObject *parent, - gc::AllocKind allocKind) +JSObject * +js::NewReshapedObject(JSContext *cx, HandleObjectGroup group, JSObject *parent, + gc::AllocKind allocKind, HandleShape shape, NewObjectKind newKind) { - // Compute the number of fixed slots with the new allocation kind. - size_t nfixed = gc::GetGCKindSlots(allocKind, shape->getObjectClass()); + RootedPlainObject res(cx, NewObjectWithGroup(cx, group, parent, allocKind, newKind)); + if (!res) + return nullptr; - // Get all the ids in the shape, in order. + if (shape->isEmptyShape()) + return res; + + /* Get all the ids in the object, in order. */ js::AutoIdVector ids(cx); { - for (unsigned i = 0; i < shape->slotSpan(); i++) { + for (unsigned i = 0; i <= shape->slot(); i++) { if (!ids.append(JSID_VOID)) return nullptr; } @@ -622,13 +626,17 @@ js::ReshapeForParentAndAllocKind(JSContext *cx, Shape *shape, TaggedProto proto, } } - // 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, shape->getObjectClass(), - proto, parent, shape->getObjectMetadata(), - nfixed, shape->getObjectFlags())); + RootedShape newShape(cx, EmptyShape::getInitialShape(cx, res->getClass(), + res->getTaggedProto(), + res->getParent(), + res->getMetadata(), + res->numFixedSlots(), + 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); @@ -646,9 +654,11 @@ js::ReshapeForParentAndAllocKind(JSContext *cx, Shape *shape, TaggedProto proto, newShape = cx->compartment()->propertyTree.getChild(cx, newShape, child); if (!newShape) return nullptr; + if (!NativeObject::setLastProperty(cx, res, newShape)) + return nullptr; } - return newShape; + return res; } /* diff --git a/js/src/vm/Shape.h b/js/src/vm/Shape.h index 2d3ca3f7b421..f93fcdde5204 100644 --- a/js/src/vm/Shape.h +++ b/js/src/vm/Shape.h @@ -1553,10 +1553,6 @@ IsImplicitDenseOrTypedArrayElement(Shape *prop) return prop == reinterpret_cast(1); } -Shape * -ReshapeForParentAndAllocKind(JSContext *cx, Shape *shape, TaggedProto proto, JSObject *parent, - gc::AllocKind allocKind); - } // namespace js #ifdef _MSC_VER diff --git a/js/src/vm/TypeInference.cpp b/js/src/vm/TypeInference.cpp index 6496caadf09a..3717c9fbf288 100644 --- a/js/src/vm/TypeInference.cpp +++ b/js/src/vm/TypeInference.cpp @@ -3152,6 +3152,19 @@ PreliminaryObjectArray::registerNewObject(JSObject *res) 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 { @@ -3227,6 +3240,13 @@ TypeNewScript::registerNewObject(PlainObject *res) 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) @@ -3274,13 +3294,14 @@ ChangeObjectFixedSlotCount(JSContext *cx, PlainObject *obj, gc::AllocKind allocK { MOZ_ASSERT(OnlyHasDataProperties(obj->lastProperty())); - Shape *newShape = ReshapeForParentAndAllocKind(cx, obj->lastProperty(), - obj->getTaggedProto(), obj->getParent(), - allocKind); - if (!newShape) + // 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) return false; - obj->setLastPropertyShrinkFixedSlots(newShape); + obj->setLastPropertyShrinkFixedSlots(clone->lastProperty()); return true; } @@ -3468,15 +3489,10 @@ TypeNewScript::maybeAnalyze(JSContext *cx, ObjectGroup *group, bool *regenerate, MOZ_ASSERT(group->unboxedLayout().newScript() == this); destroyNewScript.group = nullptr; - // 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); + // 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. templateObject_ = nullptr; return true; diff --git a/js/src/vm/TypeInference.h b/js/src/vm/TypeInference.h index 96d5ee7a9516..ac618d05ce04 100644 --- a/js/src/vm/TypeInference.h +++ b/js/src/vm/TypeInference.h @@ -753,6 +753,7 @@ class PreliminaryObjectArray } void registerNewObject(JSObject *res); + void unregisterNewObject(JSObject *res); JSObject *get(size_t i) const { MOZ_ASSERT(i < COUNT); @@ -888,6 +889,7 @@ class TypeNewScript 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);