Ensure consistency between an owned base shape and its unowned version, bug 712428. r=luke

This commit is contained in:
Brian Hackett 2011-12-24 06:32:27 -08:00
parent a14971ba6c
commit 889b33bcd1
7 changed files with 96 additions and 94 deletions

View File

@ -1726,7 +1726,8 @@ JSFunction::initBoundFunction(JSContext *cx, const Value &thisArg,
if (!toDictionaryMode(cx))
return false;
lastProperty()->base()->setObjectFlag(BaseShape::BOUND_FUNCTION);
if (!setFlag(cx, BaseShape::BOUND_FUNCTION))
return false;
if (!setSlotSpan(cx, BOUND_FUNCTION_RESERVED_SLOTS + argslen))
return false;

View File

@ -688,29 +688,26 @@ ScanShape(GCMarker *gcmarker, const Shape *shape)
static inline void
ScanBaseShape(GCMarker *gcmarker, BaseShape *base)
{
for (;;) {
if (base->hasGetterObject())
PushMarkStack(gcmarker, base->getterObject());
base->assertConsistency();
if (base->hasSetterObject())
PushMarkStack(gcmarker, base->setterObject());
if (base->hasGetterObject())
PushMarkStack(gcmarker, base->getterObject());
if (JSObject *parent = base->getObjectParent())
PushMarkStack(gcmarker, parent);
if (base->hasSetterObject())
PushMarkStack(gcmarker, base->setterObject());
if (base->isOwned()) {
/*
* Make sure that ScanBaseShape is not recursive so its inlining
* is possible.
*/
UnownedBaseShape *unowned = base->baseUnowned();
JS_SAME_COMPARTMENT_ASSERT(base, unowned);
if (unowned->markIfUnmarked(gcmarker->getMarkColor())) {
base = unowned;
continue;
}
}
break;
if (JSObject *parent = base->getObjectParent())
PushMarkStack(gcmarker, parent);
/*
* All children of the owned base shape are consistent with its
* unowned one, thus we do not need to trace through children of the
* unowned base shape.
*/
if (base->isOwned()) {
UnownedBaseShape *unowned = base->baseUnowned();
JS_SAME_COMPARTMENT_ASSERT(base, unowned);
unowned->markIfUnmarked(gcmarker->getMarkColor());
}
}
@ -952,6 +949,13 @@ MarkCycleCollectorChildren(JSTracer *trc, BaseShape *base, JSObject **prevParent
{
JS_ASSERT(base);
/*
* The cycle collector does not need to trace unowned base shapes,
* as they have the same getter, setter and parent as the original
* base shape.
*/
base->assertConsistency();
MarkBaseShapeGetterSetter(trc, base);
JSObject *parent = base->getObjectParent();
@ -959,17 +963,6 @@ MarkCycleCollectorChildren(JSTracer *trc, BaseShape *base, JSObject **prevParent
MarkObjectUnbarriered(trc, parent, "parent");
*prevParent = parent;
}
// An owned base shape has the same parent, getter and setter as
// its baseUnowned().
#ifdef DEBUG
if (base->isOwned()) {
UnownedBaseShape *unowned = base->baseUnowned();
JS_ASSERT_IF(base->hasGetterObject(), base->getterObject() == unowned->getterObject());
JS_ASSERT_IF(base->hasSetterObject(), base->setterObject() == unowned->setterObject());
JS_ASSERT(base->getObjectParent() == unowned->getObjectParent());
}
#endif
}
/*

View File

@ -615,11 +615,8 @@ struct JSObject : js::gc::Cell
/* Whether method shapes can be added to this object. */
inline bool canHaveMethodBarrier() const;
/* Whether there may be indexed properties on this object. */
inline bool isIndexed() const;
inline bool setIndexed(JSContext *cx);
/* Set the indexed flag on this object if id is an indexed property. */
inline bool maybeSetIndexed(JSContext *cx, jsid id);
/*
* Return true if this object is a native one that has been converted from
@ -789,9 +786,6 @@ struct JSObject : js::gc::Cell
inline void setFixedSlot(uintN slot, const js::Value &value);
inline void initFixedSlot(uintN slot, const js::Value &value);
/* Extend this object to have shape as its last-added property. */
inline bool extend(JSContext *cx, const js::Shape *shape, bool isDefinitelyAtom = false);
/*
* Whether this is the only object which has its specified type. This
* object will have its type constructed lazily as needed by analysis.

View File

@ -893,11 +893,6 @@ inline bool JSObject::setDelegate(JSContext *cx)
return setFlag(cx, js::BaseShape::DELEGATE, GENERATE_SHAPE);
}
inline bool JSObject::setIndexed(JSContext *cx)
{
return setFlag(cx, js::BaseShape::INDEXED);
}
inline bool JSObject::isVarObj() const
{
return lastProperty()->hasObjectFlag(js::BaseShape::VAROBJ);

View File

@ -654,18 +654,18 @@ JSObject::addPropertyInternal(JSContext *cx, jsid id,
}
}
if (!maybeSetIndexed(cx, id))
return NULL;
/* Find or create a property tree node labeled by our arguments. */
Shape *shape;
{
jsuint index;
bool indexed = js_IdIsIndex(id, &index);
UnownedBaseShape *nbase;
if (lastProperty()->base()->matchesGetterSetter(getter, setter)) {
if (lastProperty()->base()->matchesGetterSetter(getter, setter) && !indexed) {
nbase = lastProperty()->base()->unowned();
} else {
BaseShape base(getClass(), getParent(), lastProperty()->getObjectFlags(),
attrs, getter, setter);
uint32_t flags = lastProperty()->getObjectFlags()
| (indexed ? BaseShape::INDEXED : 0);
BaseShape base(getClass(), getParent(), flags, attrs, getter, setter);
nbase = BaseShape::getUnowned(cx, base);
if (!nbase)
return NULL;
@ -764,8 +764,11 @@ JSObject::putProperty(JSContext *cx, jsid id,
UnownedBaseShape *nbase;
{
BaseShape base(getClass(), getParent(), lastProperty()->getObjectFlags(),
attrs, getter, setter);
jsuint index;
bool indexed = js_IdIsIndex(id, &index);
uint32_t flags = lastProperty()->getObjectFlags()
| (indexed ? BaseShape::INDEXED : 0);
BaseShape base(getClass(), getParent(), flags, attrs, getter, setter);
nbase = BaseShape::getUnowned(cx, base);
if (!nbase)
return NULL;
@ -824,16 +827,6 @@ JSObject::putProperty(JSContext *cx, jsid id,
shape->attrs = uint8_t(attrs);
shape->flags = flags | Shape::IN_DICTIONARY;
shape->shortid_ = int16_t(shortid);
/*
* We are done updating shape and the last property. Now we may need to
* update flags. In the last non-dictionary property case in the else
* clause just below, getChildProperty handles this for us. First update
* flags.
*/
jsuint index;
if (js_IdIsIndex(shape->propid(), &index))
shape->base()->setObjectFlag(BaseShape::INDEXED);
} else {
/*
* Updating the last property in a non-dictionary-mode object. Such
@ -956,6 +949,21 @@ JSObject::removeProperty(JSContext *cx, jsid id)
if (!spare)
return false;
new (spare) Shape(shape->base()->unowned(), 0);
if (shape == lastProperty()) {
/*
* Get an up to date unowned base shape for the new last property
* when removing the dictionary's last property. Information in
* base shapes for non-last properties may be out of sync with the
* object's state.
*/
Shape *previous = lastProperty()->parent;
BaseShape base(getClass(), getParent(), lastProperty()->getObjectFlags(),
previous->attrs, previous->getter(), previous->setter());
BaseShape *nbase = BaseShape::getUnowned(cx, base);
if (!nbase)
return false;
previous->base_ = nbase;
}
}
/* If shape has a slot, free its slot number. */
@ -1143,7 +1151,13 @@ JSObject::setParent(JSContext *cx, JSObject *parent)
return false;
if (inDictionaryMode()) {
lastProperty()->base()->setParent(parent);
BaseShape base(*lastProperty()->base()->unowned());
base.setObjectParent(parent);
UnownedBaseShape *nbase = BaseShape::getUnowned(cx, base);
if (!nbase)
return false;
lastProperty()->base()->adoptUnowned(nbase);
return true;
}
@ -1157,7 +1171,7 @@ Shape::setObjectParent(JSContext *cx, JSObject *parent, JSObject *proto, HeapPtr
return true;
BaseShape base(*(*listp)->base()->unowned());
base.setParent(parent);
base.setObjectParent(parent);
return replaceLastProperty(cx, base, proto, listp);
}
@ -1196,7 +1210,14 @@ JSObject::setFlag(JSContext *cx, /*BaseShape::Flag*/ uint32_t flag_, GenerateSha
if (inDictionaryMode()) {
if (generateShape == GENERATE_SHAPE && !generateOwnShape(cx))
return false;
lastProperty()->base()->setObjectFlag(flag);
BaseShape base(*lastProperty()->base()->unowned());
base.setObjectFlag(flag);
UnownedBaseShape *nbase = BaseShape::getUnowned(cx, base);
if (!nbase)
return false;
lastProperty()->base()->adoptUnowned(nbase);
return true;
}

View File

@ -416,10 +416,11 @@ class BaseShape : public js::gc::Cell
inline void adoptUnowned(UnownedBaseShape *other);
inline void setOwned(UnownedBaseShape *unowned);
inline void setParent(JSObject *obj);
inline void setObjectParent(JSObject *obj);
JSObject *getObjectParent() { return parent; }
void setObjectFlag(Flag flag) { JS_ASSERT(!(flag & ~OBJECT_FLAG_MASK)); flags |= flag; }
uint32_t getObjectFlags() const { return flags & OBJECT_FLAG_MASK; }
bool hasGetterObject() const { return !!(flags & HAS_GETTER_OBJECT); }
JSObject *getterObject() const { JS_ASSERT(hasGetterObject()); return getterObj; }
@ -446,6 +447,9 @@ class BaseShape : public js::gc::Cell
/* Get the canonical base shape for an unowned one (i.e. identity). */
inline UnownedBaseShape *toUnowned();
/* Check that an owned base shape is consistent with its unowned base. */
inline void assertConsistency();
/* For JIT usage */
static inline size_t offsetOfClass() { return offsetof(BaseShape, clasp); }
static inline size_t offsetOfParent() { return offsetof(BaseShape, parent); }
@ -631,7 +635,7 @@ struct Shape : public js::gc::Cell
static bool setObjectParent(JSContext *cx, JSObject *obj, JSObject *proto, HeapPtrShape *listp);
static bool setObjectFlag(JSContext *cx, BaseShape::Flag flag, JSObject *proto, HeapPtrShape *listp);
uint32_t getObjectFlags() const { return base()->flags & BaseShape::OBJECT_FLAG_MASK; }
uint32_t getObjectFlags() const { return base()->getObjectFlags(); }
bool hasObjectFlag(BaseShape::Flag flag) const {
JS_ASSERT(!(flag & ~BaseShape::OBJECT_FLAG_MASK));
return !!(base()->flags & flag);

View File

@ -58,27 +58,6 @@
#include "jsgcinlines.h"
#include "jsobjinlines.h"
inline bool
JSObject::maybeSetIndexed(JSContext *cx, jsid id)
{
jsuint index;
if (js_IdIsIndex(id, &index)) {
if (!setIndexed(cx))
return false;
}
return true;
}
inline bool
JSObject::extend(JSContext *cx, const js::Shape *shape, bool isDefinitelyAtom)
{
if (!isDefinitelyAtom && !maybeSetIndexed(cx, shape->propid()))
return false;
if (!setLastProperty(cx, shape))
return false;
return true;
}
namespace js {
inline
@ -119,7 +98,7 @@ BaseShape::matchesGetterSetter(PropertyOp rawGetter, StrictPropertyOp rawSetter)
}
inline void
BaseShape::setParent(JSObject *obj)
BaseShape::setObjectParent(JSObject *obj)
{
parent = obj;
}
@ -132,19 +111,18 @@ BaseShape::adoptUnowned(UnownedBaseShape *other)
* unowned base shape of a new last property.
*/
JS_ASSERT(isOwned());
JSObject *parent = this->parent;
uint32_t flags = (this->flags & OBJECT_FLAG_MASK);
DebugOnly<uint32_t> flags = getObjectFlags();
JS_ASSERT((flags & other->getObjectFlags()) == flags);
uint32_t span = slotSpan();
PropertyTable *table = &this->table();
*this = *other;
setOwned(other);
this->parent = parent;
this->flags |= flags;
setTable(table);
setSlotSpan(span);
assertConsistency();
}
inline void
@ -154,6 +132,22 @@ BaseShape::setOwned(UnownedBaseShape *unowned)
this->unowned_ = unowned;
}
inline void
BaseShape::assertConsistency()
{
#ifdef DEBUG
if (isOwned()) {
UnownedBaseShape *unowned = baseUnowned();
JS_ASSERT(hasGetterObject() == unowned->hasGetterObject());
JS_ASSERT(hasSetterObject() == unowned->hasSetterObject());
JS_ASSERT_IF(hasGetterObject(), getterObject() == unowned->getterObject());
JS_ASSERT_IF(hasSetterObject(), setterObject() == unowned->setterObject());
JS_ASSERT(getObjectParent() == unowned->getObjectParent());
JS_ASSERT(getObjectFlags() == unowned->getObjectFlags());
}
#endif
}
inline
Shape::Shape(UnownedBaseShape *base, jsid propid, uint32_t slot, uint32_t nfixed,
uintN attrs, uintN flags, intN shortid)