Bug 991752 - Always check has[G|S]etterObject before updating the Shape; r=jonco
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -695,18 +695,20 @@ js::Nursery::moveObjectToTenured(MinorCo
if (src->isNative()) {
NativeObject* ndst = &dst->as<NativeObject>();
NativeObject* nsrc = &src->as<NativeObject>();
tenuredSize += moveSlotsToTenured(ndst, nsrc, dstKind);
tenuredSize += moveElementsToTenured(ndst, nsrc, dstKind);
// The shape's list head may point into the old object. This can only
// happen for dictionaries, which are native objects.
- if (&nsrc->shape_ == ndst->shape_->listp)
+ if (&nsrc->shape_ == ndst->shape_->listp) {
+ MOZ_ASSERT(nsrc->shape_->inDictionary());
ndst->shape_->listp = &ndst->shape_;
+ }
}
if (src->is<InlineTypedObject>())
InlineTypedObject::objectMovedDuringMinorGC(trc, dst, src);
return tenuredSize;
}
--- a/js/src/jspropertytree.cpp
+++ b/js/src/jspropertytree.cpp
@@ -305,42 +305,36 @@ Shape::fixupAfterMovingGC()
{
if (inDictionary())
fixupDictionaryShapeAfterMovingGC();
else
fixupShapeTreeAfterMovingGC();
}
void
-ShapeGetterSetterRef::mark(JSTracer* trc)
+Shape::fixupGetterSetterForBarrier(JSTracer *trc)
{
- // Update the current shape's entry in the parent KidsHash table if needed.
- // This is necessary as the computed hash includes the getter/setter
- // pointers.
-
- JSObject* obj = *objp;
- JSObject* prior = obj;
- if (!prior)
- return;
-
- trc->setTracingLocation(&*prior);
- TraceManuallyBarrieredEdge(trc, &obj, "AccessorShape getter or setter");
- if (obj == *objp)
- return;
-
- Shape* parent = shape->parent;
- if (shape->inDictionary() || !parent->kids.isHash()) {
- *objp = obj;
- return;
+ // Relocating the getterObj or setterObj will change our location in our
+ // parent's KidsHash, so remove ourself first if we're going to get moved.
+ if (parent && !parent->inDictionary() && parent->kids.isHash()) {
+ KidsHash* kh = parent->kids.toHash();
+ MOZ_ASSERT(kh->lookup(StackShape(this)));
+ kh->remove(StackShape(this));
}
- KidsHash* kh = parent->kids.toHash();
- kh->remove(StackShape(shape));
- *objp = obj;
- MOZ_ALWAYS_TRUE(kh->putNew(StackShape(shape), shape));
+ if (hasGetterObject())
+ TraceManuallyBarrieredEdge(trc, &asAccessorShape().getterObj, "getterObj");
+ if (hasSetterObject())
+ TraceManuallyBarrieredEdge(trc, &asAccessorShape().setterObj, "setterObj");
+
+ if (parent && !parent->inDictionary() && parent->kids.isHash()) {
+ KidsHash* kh = parent->kids.toHash();
+ MOZ_ASSERT(!kh->lookup(StackShape(this)));
+ MOZ_ALWAYS_TRUE(kh->putNew(StackShape(this), this));
+ }
}
#ifdef DEBUG
void
KidsPointer::checkConsistency(Shape* aKid) const
{
if (isShape()) {
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -807,21 +807,18 @@ NativeObject::putProperty(ExclusiveConte
MOZ_ASSERT_IF(attrs & (JSPROP_GETTER | JSPROP_SETTER), attrs & JSPROP_SHARED);
shape->setSlot(slot);
shape->attrs = uint8_t(attrs);
shape->flags = flags | Shape::IN_DICTIONARY | (accessorShape ? Shape::ACCESSOR_SHAPE : 0);
if (shape->isAccessorShape()) {
AccessorShape& accShape = shape->asAccessorShape();
accShape.rawGetter = getter;
- if (accShape.hasGetterObject())
- GetterSetterWriteBarrierPost(&accShape, &accShape.getterObj);
accShape.rawSetter = setter;
- if (accShape.hasSetterObject())
- GetterSetterWriteBarrierPost(&accShape, &accShape.setterObj);
+ GetterSetterWriteBarrierPost(&accShape);
} else {
MOZ_ASSERT(!getter);
MOZ_ASSERT(!setter);
}
} else {
/*
* Updating the last property in a non-dictionary-mode object. Find an
* alternate shared child of the last property's previous shape.
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -314,49 +314,16 @@ class ShapeTable {
* property, however.
*/
class AccessorShape;
class Shape;
class UnownedBaseShape;
struct StackBaseShape;
-// This class is used to add a post barrier on the AccessorShape's getter/setter
-// objects. It updates the shape's entry in the parent's KidsHash table.
-class ShapeGetterSetterRef : public gc::BufferableRef
-{
- AccessorShape* shape;
- JSObject** objp;
-
- public:
- ShapeGetterSetterRef(AccessorShape* shape, JSObject** objp)
- : shape(shape), objp(objp)
- {}
-
- void mark(JSTracer* trc);
-};
-
-static inline void
-GetterSetterWriteBarrierPost(AccessorShape* shape, JSObject** objp)
-{
- MOZ_ASSERT(shape);
- MOZ_ASSERT(objp);
- MOZ_ASSERT(*objp);
- gc::Cell** cellp = reinterpret_cast<gc::Cell**>(objp);
- if (gc::StoreBuffer* sb = (*cellp)->storeBuffer())
- sb->putGeneric(ShapeGetterSetterRef(shape, objp));
-}
-
-static inline void
-GetterSetterWriteBarrierPostRemove(JSRuntime* rt, JSObject** objp)
-{
- JS::shadow::Runtime* shadowRuntime = JS::shadow::Runtime::asShadowRuntime(rt);
- shadowRuntime->gcStoreBufferPtr()->removeRelocatableCellFromAnyThread(reinterpret_cast<gc::Cell**>(objp));
-}
-
class BaseShape : public gc::TenuredCell
{
public:
friend class Shape;
friend struct StackBaseShape;
friend struct StackShape;
friend void gc::MergeCompartments(JSCompartment* source, JSCompartment* target);
@@ -595,17 +562,16 @@ class Shape : public gc::TenuredCell
{
friend class ::JSObject;
friend class ::JSFunction;
friend class Bindings;
friend class Nursery;
friend class NativeObject;
friend class PropertyTree;
friend class StaticBlockObject;
- friend class ShapeGetterSetterRef;
friend struct StackShape;
friend struct StackBaseShape;
protected:
HeapPtrBaseShape base_;
PreBarrieredId propid_;
enum SlotInfo : uint32_t
@@ -1000,16 +966,17 @@ class Shape : public gc::TenuredCell
static inline ThingRootKind rootKind() { return THING_ROOT_SHAPE; }
inline void markChildren(JSTracer* trc);
inline Shape* search(ExclusiveContext* cx, jsid id);
inline Shape* searchLinear(jsid id);
void fixupAfterMovingGC();
+ void fixupGetterSetterForBarrier(JSTracer *trc);
/* For JIT usage */
static inline size_t offsetOfBase() { return offsetof(Shape, base_); }
static inline size_t offsetOfSlotInfo() { return offsetof(Shape, slotInfo); }
static inline uint32_t fixedSlotsMask() { return FIXED_SLOTS_MASK; }
private:
void fixupDictionaryShapeAfterMovingGC();
@@ -1021,17 +988,16 @@ class Shape : public gc::TenuredCell
JS_STATIC_ASSERT(FIXED_SLOTS_SHIFT == js::shadow::Shape::FIXED_SLOTS_SHIFT);
}
};
/* Fat Shape used for accessor properties. */
class AccessorShape : public Shape
{
friend class Shape;
- friend class ShapeGetterSetterRef;
friend class NativeObject;
union {
GetterOp rawGetter; /* getter hook for shape */
JSObject* getterObj; /* user-defined callable "get" object or
null if shape->hasGetterValue() */
};
union {
@@ -1265,28 +1231,56 @@ Shape::Shape(const StackShape& other, ui
MOZ_ASSERT_IF(other.isAccessorShape(), allocKind == gc::AllocKind::ACCESSOR_SHAPE);
MOZ_ASSERT_IF(allocKind == gc::AllocKind::SHAPE, !other.isAccessorShape());
#endif
MOZ_ASSERT_IF(attrs & (JSPROP_GETTER | JSPROP_SETTER), attrs & JSPROP_SHARED);
kids.setNull();
}
+// This class is used to add a post barrier on the AccessorShape's getter/setter
+// objects. It updates the pointers and the shape's entry in the parent's
+// KidsHash table.
+class ShapeGetterSetterRef : public gc::BufferableRef
+{
+ AccessorShape* shape_;
+
+ public:
+ explicit ShapeGetterSetterRef(AccessorShape* shape) : shape_(shape) {}
+ void mark(JSTracer* trc) { shape_->fixupGetterSetterForBarrier(trc); }
+};
+
+static inline void
+GetterSetterWriteBarrierPost(AccessorShape* shape)
+{
+ MOZ_ASSERT(shape);
+ if (shape->hasGetterObject()) {
+ gc::StoreBuffer *sb = reinterpret_cast<gc::Cell*>(shape->getterObject())->storeBuffer();
+ if (sb) {
+ sb->putGeneric(ShapeGetterSetterRef(shape));
+ return;
+ }
+ }
+ if (shape->hasSetterObject()) {
+ gc::StoreBuffer *sb = reinterpret_cast<gc::Cell*>(shape->setterObject())->storeBuffer();
+ if (sb) {
+ sb->putGeneric(ShapeGetterSetterRef(shape));
+ return;
+ }
+ }
+}
+
inline
AccessorShape::AccessorShape(const StackShape& other, uint32_t nfixed)
: Shape(other, nfixed),
rawGetter(other.rawGetter),
rawSetter(other.rawSetter)
{
MOZ_ASSERT(getAllocKind() == gc::AllocKind::ACCESSOR_SHAPE);
-
- if ((attrs & JSPROP_GETTER) && rawGetter)
- GetterSetterWriteBarrierPost(this, &this->getterObj);
- if ((attrs & JSPROP_SETTER) && rawSetter)
- GetterSetterWriteBarrierPost(this, &this->setterObj);
+ GetterSetterWriteBarrierPost(this);
}
inline
Shape::Shape(UnownedBaseShape* base, uint32_t nfixed)
: base_(base),
propid_(JSID_EMPTY),
slotInfo(SHAPE_INVALID_SLOT | (nfixed << FIXED_SLOTS_SHIFT)),
attrs(JSPROP_SHARED),