author | David Mandelin <dmandelin@mozilla.com> |
Mon, 06 Dec 2010 16:27:39 -0800 | |
changeset 58749 | 8cd47ad6b71d259c78747c96df4538f44bcebc72 |
parent 58748 | cc202b48606f1cbbbfe8d110579935ac41fa4cc5 |
child 58750 | 4e41063ec08f7a5b4a303422d0ab338b7de62144 |
push id | 17414 |
push user | rsayre@mozilla.com |
push date | Tue, 07 Dec 2010 03:47:09 +0000 |
treeherder | mozilla-central@37b29506a7d4 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | brendan |
bugs | 601046 |
milestone | 2.0b8pre |
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
|
new file mode 100644 --- /dev/null +++ b/js/src/jit-test/tests/basic/bug601046.js @@ -0,0 +1,8 @@ +// don't assert +var f = function(){}; +for (var p in f); +Object.defineProperty(f, "j", ({configurable: true, value: "a"})); +Object.defineProperty(f, "k", ({configurable: true, value: "b"})); +Object.defineProperty(f, "j", ({configurable: true, get: function() {}})); +delete f.k; +Object.defineProperty(f, "j", ({configurable: false}));
--- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -4279,17 +4279,17 @@ JSObject::allocSlot(JSContext *cx, uint3 return false; /* JSObject::growSlots or JSObject::freeSlot should set the free slots to void. */ JS_ASSERT(getSlot(slot).isUndefined()); *slotp = slot; return true; } -void +bool JSObject::freeSlot(JSContext *cx, uint32 slot) { uint32 limit = slotSpan(); JS_ASSERT(slot < limit); Value &vref = getSlotRef(slot); if (inDictionaryMode() && lastProp->table) { uint32 &last = lastProp->table->freelist; @@ -4302,20 +4302,21 @@ JSObject::freeSlot(JSContext *cx, uint32 * shape (and not a reserved slot; see bug 595230): push the slot onto * the dictionary property table's freelist. We want to let the last * slot be freed by shrinking the dslots vector; see js_TraceObject. */ if (JSSLOT_FREE(clasp) <= slot && slot + 1 < limit) { JS_ASSERT_IF(last != SHAPE_INVALID_SLOT, last < slotSpan()); vref.setPrivateUint32(last); last = slot; - return; + return true; } } vref.setUndefined(); + return false; } /* JSBOXEDWORD_INT_MAX as a string */ #define JSBOXEDWORD_INT_MAX_STRING "1073741823" /* * Convert string indexes that convert to int jsvals as ints to save memory. * Care must be taken to use this macro every time a property name is used, or
--- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -1026,17 +1026,20 @@ struct JSObject : js::gc::Cell { inline void freeSlotsArray(JSContext *cx); /* Free the slots array and copy slots that fit into the fixed array. */ inline void revertToFixedSlots(JSContext *cx); inline bool hasProperty(JSContext *cx, jsid id, bool *foundp, uintN flags = 0); bool allocSlot(JSContext *cx, uint32 *slotp); - void freeSlot(JSContext *cx, uint32 slot); + /* + * Return true iff this is a dictionary-mode object and the freed slot was + * added to the freelist. */ + bool freeSlot(JSContext *cx, uint32 slot); bool reportReadOnly(JSContext* cx, jsid id, uintN report = JSREPORT_ERROR); bool reportNotConfigurable(JSContext* cx, jsid id, uintN report = JSREPORT_ERROR); bool reportNotExtensible(JSContext *cx, uintN report = JSREPORT_ERROR); private: js::Shape *getChildProperty(JSContext *cx, js::Shape *parent, js::Shape &child);
--- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -1072,51 +1072,60 @@ JSObject::changeProperty(JSContext *cx, if (getter == PropertyStub) getter = NULL; if (setter == PropertyStub) setter = NULL; if (shape->attrs == attrs && shape->getter() == getter && shape->setter() == setter) return shape; - Shape child(shape->id, getter, setter, shape->slot, attrs, shape->flags, shape->shortid); - const Shape *newShape; + /* + * Dictionary-mode objects exclusively own their mutable shape structs, so + * we simply modify in place. + */ if (inDictionaryMode()) { - shape->removeFromDictionary(this); - newShape = Shape::newDictionaryShape(cx, child, &lastProp); - if (newShape) { - JS_ASSERT(newShape == lastProp); - - /* - * Let tableShape be the shape with non-null table, either the one - * we removed or the parent of lastProp. - */ - const Shape *tableShape = shape->table ? shape : lastProp->parent; + /* FIXME bug 593129 -- slot allocation and JSObject *this must move out of here! */ + uint32 slot = shape->slot; + if (slot == SHAPE_INVALID_SLOT && !(attrs & JSPROP_SHARED) && !(flags & Shape::ALIAS)) { + if (!allocSlot(cx, &slot)) + return NULL; + } - if (PropertyTable *table = tableShape->table) { - /* Overwrite shape with newShape in the property table. */ - Shape **spp = table->search(shape->id, true); - SHAPE_STORE_PRESERVING_COLLISION(spp, newShape); + Shape *mutableShape = const_cast<Shape *>(shape); + mutableShape->slot = slot; + if (slot != SHAPE_INVALID_SLOT && slot >= shape->slotSpan) { + mutableShape->slotSpan = slot + 1; - /* Hand the table off from tableShape to newShape. */ - tableShape->setTable(NULL); - newShape->setTable(table); - } - - updateFlags(newShape); - updateShape(cx); - - if (!js_UpdateWatchpointsForShape(cx, this, newShape)) { - METER(wrapWatchFails); - return NULL; + for (Shape *temp = lastProp; temp != shape; temp = temp->parent) { + if (temp->slotSpan <= slot) + temp->slotSpan = slot + 1; } } + + mutableShape->rawGetter = getter; + mutableShape->rawSetter = setter; + mutableShape->attrs = uint8(attrs); + + updateFlags(shape); + + /* See the corresponding code in putProperty. */ + lastProp->shape = js_GenerateShape(cx, false); + clearOwnShape(); + + if (!js_UpdateWatchpointsForShape(cx, this, shape)) { + METER(wrapWatchFails); + return NULL; + } + + newShape = mutableShape; } else if (shape == lastProp) { + Shape child(shape->id, getter, setter, shape->slot, attrs, shape->flags, shape->shortid); + newShape = getChildProperty(cx, shape->parent, child); #ifdef DEBUG if (newShape) { JS_ASSERT(newShape == lastProp); if (newShape->table) { Shape **spp = nativeSearch(shape->id); JS_ASSERT(SHAPE_FETCH(spp) == newShape); } @@ -1124,16 +1133,17 @@ JSObject::changeProperty(JSContext *cx, #endif } else { /* * Let JSObject::putProperty handle this |overwriting| case, including * the conservation of shape->slot (if it's valid). We must not call * removeProperty because it will free an allocated shape->slot, and * putProperty won't re-allocate it. */ + Shape child(shape->id, getter, setter, shape->slot, attrs, shape->flags, shape->shortid); newShape = putProperty(cx, child.id, child.rawGetter, child.rawSetter, child.slot, child.attrs, child.flags, child.shortid); #ifdef DEBUG if (newShape) METER(changePuts); #endif } @@ -1153,19 +1163,20 @@ JSObject::removeProperty(JSContext *cx, Shape **spp = nativeSearch(id); Shape *shape = SHAPE_FETCH(spp); if (!shape) { METER(uselessRemoves); return true; } /* First, if shape is unshared and not has a slot, free its slot number. */ + bool addedToFreelist = false; bool hadSlot = !shape->isAlias() && shape->hasSlot(); if (hadSlot) { - freeSlot(cx, shape->slot); + addedToFreelist = freeSlot(cx, shape->slot); JS_ATOMIC_INCREMENT(&cx->runtime->propertyRemovals); } /* If shape is not the last property added, switch to dictionary mode. */ if (shape != lastProp && !inDictionaryMode()) { if (!toDictionaryMode(cx)) return false; @@ -1220,22 +1231,30 @@ JSObject::removeProperty(JSContext *cx, if (table) { if (shape == oldLastProp) { JS_ASSERT(shape->table == table); JS_ASSERT(shape->parent == lastProp); JS_ASSERT(shape->slotSpan >= lastProp->slotSpan); JS_ASSERT_IF(hadSlot, shape->slot + 1 <= shape->slotSpan); /* - * If the dictionary table's freelist is non-empty, we must - * preserve lastProp->slotSpan. We can't reduce slotSpan even - * by one or we might lose non-decreasing slotSpan order. - */ - if (table->freelist != SHAPE_INVALID_SLOT) + * Maintain slot freelist consistency. The only constraint we + * have is that slot numbers on the freelist are less than + * lastProp->slotSpan. Thus, if the freelist is non-empty, + * then lastProp->slotSpan may not decrease. + */ + if (table->freelist != SHAPE_INVALID_SLOT) { lastProp->slotSpan = shape->slotSpan; + + /* Add the slot to the freelist if it wasn't added in freeSlot. */ + if (hadSlot && !addedToFreelist) { + getSlotRef(shape->slot).setPrivateUint32(table->freelist); + table->freelist = shape->slot; + } + } } /* Hand off table from old to new lastProp. */ oldLastProp->setTable(NULL); lastProp->setTable(table); } } else { /*