Bug 1312485: Properly sparsify frozen objects. r?jandem
MozReview-Commit-ID: CqGZuZqTBfa
--- a/js/src/tests/ecma_5/Array/frozen-dense-array.js
+++ b/js/src/tests/ecma_5/Array/frozen-dense-array.js
@@ -4,39 +4,58 @@
* Author: Emilio Cobos Álvarez <ecoal95@gmail.com>
*/
var BUGNUMBER = 1310744;
var summary = "Dense array properties shouldn't be modified when they're frozen";
print(BUGNUMBER + ": " + summary);
var a = Object.freeze([4, 5, 1]);
-var methods = [
- 'reverse',
- 'shift',
- 'pop',
-];
+
+function assertArrayIsExpected() {
+ assertEq(a.length, 3);
+ assertEq(a[0], 4);
+ assertEq(a[1], 5);
+ assertEq(a[2], 1);
+}
assertThrowsInstanceOf(() => a.reverse(), TypeError);
assertThrowsInstanceOf(() => a.shift(), TypeError);
assertThrowsInstanceOf(() => a.unshift(0), TypeError);
assertThrowsInstanceOf(() => a.sort(function() {}), TypeError);
assertThrowsInstanceOf(() => a.pop(), TypeError);
assertThrowsInstanceOf(() => a.fill(0), TypeError);
assertThrowsInstanceOf(() => a.splice(0, 1, 1), TypeError);
assertThrowsInstanceOf(() => a.push("foo"), TypeError);
assertThrowsInstanceOf(() => { "use strict"; a.length = 5; }, TypeError);
+assertThrowsInstanceOf(() => { "use strict"; a[2] = "foo"; }, TypeError);
assertThrowsInstanceOf(() => { "use strict"; delete a[0]; }, TypeError);
assertThrowsInstanceOf(() => a.splice(Math.a), TypeError);
// Shouldn't throw, since this is not strict mode, but shouldn't change the
// value of the property.
a.length = 5;
+a[2] = "foo";
assertEq(delete a[0], false);
+assertArrayIsExpected();
-assertEq(a.length, 3);
-assertEq(a[0], 4);
-assertEq(a[1], 5);
-assertEq(a[2], 1);
+var watchpointCalled = false;
+// NOTE: Be careful with the position of this test, since this sparsifies the
+// elements and you might not test what you think you're testing otherwise.
+a.watch(2, function(prop, oldValue, newValue) {
+ watchpointCalled = true;
+ assertEq(prop, 2);
+ assertEq(oldValue, 1);
+ assertEq(newValue, "foo");
+});
+
+assertArrayIsExpected();
+
+a.length = 5;
+a[2] = "foo";
+assertEq(watchpointCalled, true);
+assertEq(delete a[0], false);
+
+assertArrayIsExpected();
if (typeof reportCompare === "function")
reportCompare(true, true);
--- a/js/src/vm/NativeObject-inl.h
+++ b/js/src/vm/NativeObject-inl.h
@@ -94,17 +94,17 @@ NativeObject::setDenseElementHole(Exclus
}
/* static */ inline void
NativeObject::removeDenseElementForSparseIndex(ExclusiveContext* cx,
HandleNativeObject obj, uint32_t index)
{
MarkObjectGroupFlags(cx, obj, OBJECT_FLAG_NON_PACKED | OBJECT_FLAG_SPARSE_INDEXES);
if (obj->containsDenseElement(index))
- obj->setDenseElement(index, MagicValue(JS_ELEMENTS_HOLE));
+ obj->setDenseElementUnchecked(index, MagicValue(JS_ELEMENTS_HOLE));
}
inline bool
NativeObject::writeToIndexWouldMarkNotPacked(uint32_t index)
{
return getElementsHeader()->initializedLength < index;
}
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -514,18 +514,29 @@ NativeObject::sparsifyDenseElement(Exclu
return false;
RootedValue value(cx, obj->getDenseElement(index));
MOZ_ASSERT(!value.isMagic(JS_ELEMENTS_HOLE));
removeDenseElementForSparseIndex(cx, obj, index);
uint32_t slot = obj->slotSpan();
- if (!obj->addDataProperty(cx, INT_TO_JSID(index), slot, JSPROP_ENUMERATE)) {
- obj->setDenseElement(index, value);
+
+ RootedId id(cx, INT_TO_JSID(index));
+
+ ShapeTable::Entry* entry = nullptr;
+ if (obj->inDictionaryMode())
+ entry = &obj->lastProperty()->table().search<MaybeAdding::Adding>(id);
+
+ // NOTE: We don't use addDataProperty because we don't want the
+ // extensibility check if we're, for example, sparsifying frozen objects..
+ if (!addPropertyInternal(cx, obj, id, nullptr, nullptr, slot,
+ obj->getElementsHeader()->elementAttributes(),
+ 0, entry, true)) {
+ obj->setDenseElementUnchecked(index, value);
return false;
}
MOZ_ASSERT(slot == obj->slotSpan() - 1);
obj->initSlot(slot, value);
return true;
}
@@ -543,17 +554,17 @@ NativeObject::sparsifyDenseElements(js::
if (obj->getDenseElement(i).isMagic(JS_ELEMENTS_HOLE))
continue;
if (!sparsifyDenseElement(cx, obj, i))
return false;
}
if (initialized)
- obj->setDenseInitializedLength(0);
+ obj->setDenseInitializedLengthUnchecked(0);
/*
* Reduce storage for dense elements which are now holes. Explicitly mark
* the elements capacity as zero, so that any attempts to add dense
* elements will be caught in ensureDenseElements.
*/
if (obj->getDenseCapacity()) {
obj->shrinkElements(cx, 0);
@@ -1003,17 +1014,17 @@ NativeObject::addDataProperty(ExclusiveC
MOZ_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER)));
RootedNativeObject self(cx, this);
RootedId id(cx, idArg);
return addProperty(cx, self, id, nullptr, nullptr, slot, attrs, 0);
}
Shape*
NativeObject::addDataProperty(ExclusiveContext* cx, HandlePropertyName name,
- uint32_t slot, unsigned attrs)
+ uint32_t slot, unsigned attrs)
{
MOZ_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER)));
RootedNativeObject self(cx, this);
RootedId id(cx, NameToId(name));
return addProperty(cx, self, id, nullptr, nullptr, slot, attrs, 0);
}
template <AllowGC allowGC>
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@@ -300,16 +300,22 @@ class ObjectElements
flags |= FROZEN;
}
void markNotFrozen() {
MOZ_ASSERT(isFrozen());
MOZ_ASSERT(!isCopyOnWrite());
flags &= ~FROZEN;
}
+ uint8_t elementAttributes() const {
+ if (isFrozen())
+ return JSPROP_ENUMERATE | JSPROP_PERMANENT | JSPROP_READONLY;
+ return JSPROP_ENUMERATE;
+ }
+
// This is enough slots to store an object of this class. See the static
// assertion below.
static const size_t VALUES_PER_HEADER = 2;
};
static_assert(ObjectElements::VALUES_PER_HEADER * sizeof(HeapSlot) == sizeof(ObjectElements),
"ObjectElements doesn't fit in the given number of slots");
@@ -859,34 +865,34 @@ class NativeObject : public ShapedObject
// MAX_FIXED_SLOTS is the biggest number of fixed slots our GC
// size classes will give an object.
static const uint32_t MAX_FIXED_SLOTS = 16;
protected:
inline bool updateSlotsForSpan(ExclusiveContext* cx, size_t oldSpan, size_t newSpan);
- public:
+ private:
+ void prepareElementRangeForOverwrite(size_t start, size_t end) {
+ MOZ_ASSERT(end <= getDenseInitializedLength());
+ MOZ_ASSERT(!denseElementsAreCopyOnWrite());
+ for (size_t i = start; i < end; i++)
+ elements_[i].HeapSlot::~HeapSlot();
+ }
+
/*
* Trigger the write barrier on a range of slots that will no longer be
* reachable.
*/
void prepareSlotRangeForOverwrite(size_t start, size_t end) {
for (size_t i = start; i < end; i++)
getSlotAddressUnchecked(i)->HeapSlot::~HeapSlot();
}
- void prepareElementRangeForOverwrite(size_t start, size_t end) {
- MOZ_ASSERT(end <= getDenseInitializedLength());
- MOZ_ASSERT(!denseElementsAreCopyOnWrite());
- MOZ_ASSERT(!denseElementsAreFrozen());
- for (size_t i = start; i < end; i++)
- elements_[i].HeapSlot::~HeapSlot();
- }
-
+ public:
static bool rollbackProperties(ExclusiveContext* cx, HandleNativeObject obj,
uint32_t slotSpan);
inline void setSlotWithType(ExclusiveContext* cx, Shape* shape,
const Value& value, bool overwriting = true);
inline const Value& getReservedSlot(uint32_t index) const {
MOZ_ASSERT(index < JSSLOT_FREE(getClass()));
@@ -1010,32 +1016,45 @@ class NativeObject : public ShapedObject
JS::shadow::Runtime* shadowRuntime = shadowRuntimeFromMainThread();
shadowRuntime->gcStoreBufferPtr()->putSlot(this, HeapSlot::Element,
start + i, count - i);
return;
}
}
}
- public:
- void setDenseInitializedLength(uint32_t length) {
+ // See the comment over setDenseElementUnchecked, this applies in the same way.
+ void setDenseInitializedLengthUnchecked(uint32_t length) {
MOZ_ASSERT(length <= getDenseCapacity());
MOZ_ASSERT(!denseElementsAreCopyOnWrite());
- MOZ_ASSERT(!denseElementsAreFrozen());
prepareElementRangeForOverwrite(length, getElementsHeader()->initializedLength);
getElementsHeader()->initializedLength = length;
}
+ // Use this function with care. This is done to allow sparsifying frozen
+ // objects, but should only be called in a few places, and should be
+ // audited carefully!
+ void setDenseElementUnchecked(uint32_t index, const Value& val) {
+ MOZ_ASSERT(index < getDenseInitializedLength());
+ MOZ_ASSERT(!denseElementsAreCopyOnWrite());
+ elements_[index].set(this, HeapSlot::Element, index, val);
+ }
+
+ public:
+ void setDenseInitializedLength(uint32_t length) {
+ MOZ_ASSERT(!denseElementsAreFrozen());
+ setDenseInitializedLengthUnchecked(length);
+ }
+
inline void ensureDenseInitializedLength(ExclusiveContext* cx,
uint32_t index, uint32_t extra);
+
void setDenseElement(uint32_t index, const Value& val) {
- MOZ_ASSERT(index < getDenseInitializedLength());
- MOZ_ASSERT(!denseElementsAreCopyOnWrite());
MOZ_ASSERT(!denseElementsAreFrozen());
- elements_[index].set(this, HeapSlot::Element, index, val);
+ setDenseElementUnchecked(index, val);
}
void initDenseElement(uint32_t index, const Value& val) {
MOZ_ASSERT(index < getDenseInitializedLength());
MOZ_ASSERT(!denseElementsAreCopyOnWrite());
MOZ_ASSERT(!denseElementsAreFrozen());
elements_[index].init(this, HeapSlot::Element, index, val);
}
--- a/js/src/vm/Shape-inl.h
+++ b/js/src/vm/Shape-inl.h
@@ -170,19 +170,17 @@ AutoRooterGetterSetter::AutoRooterGetter
static inline uint8_t
GetShapeAttributes(JSObject* obj, Shape* shape)
{
MOZ_ASSERT(obj->isNative());
if (IsImplicitDenseOrTypedArrayElement(shape)) {
if (obj->is<TypedArrayObject>())
return JSPROP_ENUMERATE | JSPROP_PERMANENT;
- if (obj->as<NativeObject>().getElementsHeader()->isFrozen())
- return JSPROP_ENUMERATE | JSPROP_PERMANENT | JSPROP_READONLY;
- return JSPROP_ENUMERATE;
+ return obj->as<NativeObject>().getElementsHeader()->elementAttributes();
}
return shape->attributes();
}
} /* namespace js */
#endif /* vm_Shape_inl_h */