Bug 1651645 part 5 - Assert initDenseElement/setDenseElement aren't called with the hole MagicValue. r=evilpie
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 14 Jul 2020 07:26:24 +0000
changeset 540353 9a6e4bca8f6ae1fbc3b7556bac1f63fa45d6c97a
parent 540352 c7a06239c86036335898d1ccf7aa477634c8b619
child 540354 726d3e7fd0428cf47be8bd3cb7931551f8db3c77
push id121651
push userjdemooij@mozilla.com
push dateTue, 14 Jul 2020 08:39:33 +0000
treeherderautoland@f112125cab4d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1651645
milestone80.0a1
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
Bug 1651645 part 5 - Assert initDenseElement/setDenseElement aren't called with the hole MagicValue. r=evilpie Differential Revision: https://phabricator.services.mozilla.com/D82962
js/src/builtin/Array.cpp
js/src/vm/NativeObject-inl.h
js/src/vm/NativeObject.h
--- a/js/src/builtin/Array.cpp
+++ b/js/src/builtin/Array.cpp
@@ -1639,30 +1639,37 @@ static DenseElementResult ArrayReverseDe
     }
   }
 
   if (!MaybeInIteration(obj, cx) && !cx->zone()->needsIncrementalBarrier()) {
     obj->reverseDenseElementsNoPreBarrier(length);
     return DenseElementResult::Success;
   }
 
+  auto setElementMaybeHole = [](JSContext* cx, HandleNativeObject obj,
+                                uint32_t index, const Value& val) {
+    if (MOZ_LIKELY(!val.isMagic(JS_ELEMENTS_HOLE))) {
+      obj->setDenseElement(index, val);
+      return true;
+    }
+
+    obj->setDenseElementHole(cx, index);
+    return SuppressDeletedProperty(cx, obj, INT_TO_JSID(index));
+  };
+
   RootedValue origlo(cx), orighi(cx);
 
   uint32_t lo = 0, hi = length - 1;
   for (; lo < hi; lo++, hi--) {
     origlo = obj->getDenseElement(lo);
     orighi = obj->getDenseElement(hi);
-    obj->setDenseElement(lo, orighi);
-    if (orighi.isMagic(JS_ELEMENTS_HOLE) &&
-        !SuppressDeletedProperty(cx, obj, INT_TO_JSID(lo))) {
+    if (!setElementMaybeHole(cx, obj, lo, orighi)) {
       return DenseElementResult::Failure;
     }
-    obj->setDenseElement(hi, origlo);
-    if (origlo.isMagic(JS_ELEMENTS_HOLE) &&
-        !SuppressDeletedProperty(cx, obj, INT_TO_JSID(hi))) {
+    if (!setElementMaybeHole(cx, obj, hi, origlo)) {
       return DenseElementResult::Failure;
     }
   }
 
   return DenseElementResult::Success;
 }
 
 // ES2017 draft rev 1b0184bc17fc09a8ddcf4aeec9b6d9fcac4eafce
--- a/js/src/vm/NativeObject-inl.h
+++ b/js/src/vm/NativeObject-inl.h
@@ -63,31 +63,35 @@ inline void NativeObject::setShouldConve
 
 inline void NativeObject::clearShouldConvertDoubleElements() {
   MOZ_ASSERT(is<ArrayObject>() && !hasEmptyElements());
   getElementsHeader()->clearShouldConvertDoubleElements();
 }
 
 inline void NativeObject::addDenseElementType(JSContext* cx, uint32_t index,
                                               const Value& val) {
+  MOZ_ASSERT(!val.isMagic(JS_ELEMENTS_HOLE));
+
   if (!IsTypeInferenceEnabled()) {
     return;
   }
 
   // Avoid a slow AddTypePropertyId call if the type is the same as the type
   // of the previous element.
   TypeSet::Type thisType = TypeSet::GetValueType(val);
   if (index == 0 || elements_[index - 1].isMagic() ||
       TypeSet::GetValueType(elements_[index - 1]) != thisType) {
     AddTypePropertyId(cx, this, JSID_VOID, thisType);
   }
 }
 
 inline void NativeObject::setDenseElementWithType(JSContext* cx, uint32_t index,
                                                   const Value& val) {
+  MOZ_ASSERT(!val.isMagic(JS_ELEMENTS_HOLE));
+
   addDenseElementType(cx, index, val);
   if (val.isInt32() && shouldConvertDoubleElements()) {
     setDenseElement(index, DoubleValue(val.toInt32()));
   } else {
     setDenseElement(index, val);
   }
 }
 
@@ -98,17 +102,17 @@ inline void NativeObject::initDenseEleme
   MOZ_ASSERT(!val.isMagic(JS_ELEMENTS_HOLE));
 
   addDenseElementType(cx, index, val);
   initDenseElement(index, val);
 }
 
 inline void NativeObject::setDenseElementHole(JSContext* cx, uint32_t index) {
   markDenseElementsNotPacked(cx);
-  setDenseElement(index, MagicValue(JS_ELEMENTS_HOLE));
+  setDenseElementUnchecked(index, MagicValue(JS_ELEMENTS_HOLE));
 }
 
 inline void NativeObject::removeDenseElementForSparseIndex(JSContext* cx,
                                                            uint32_t index) {
   MOZ_ASSERT(containsPure(INT_TO_JSID(index)));
   if (IsTypeInferenceEnabled()) {
     MarkObjectGroupFlags(cx, this, OBJECT_FLAG_SPARSE_INDEXES);
   }
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@@ -1241,32 +1241,42 @@ class NativeObject : public JSObject {
       shrinkCapacityToInitializedLength(cx);
     }
   }
 
   inline void ensureDenseInitializedLength(JSContext* 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());
-    checkStoredValue(val);
-    elements_[index].set(this, HeapSlot::Element, unshiftedIndex(index), val);
+    // Note: Streams code can call this for the internal ListObject type with
+    // MagicValue(JS_WRITABLESTREAM_CLOSE_RECORD).
+    MOZ_ASSERT_IF(val.isMagic(), val.whyMagic() != JS_ELEMENTS_HOLE);
+    setDenseElementUnchecked(index, val);
   }
 
   void initDenseElement(uint32_t index, const Value& val) {
+    MOZ_ASSERT(!val.isMagic(JS_ELEMENTS_HOLE));
     MOZ_ASSERT(index < getDenseInitializedLength());
     MOZ_ASSERT(!denseElementsAreCopyOnWrite());
     MOZ_ASSERT(isExtensible());
     checkStoredValue(val);
     elements_[index].init(this, HeapSlot::Element, unshiftedIndex(index), val);
   }
 
  private:
+  // Note: 'Unchecked' here means we don't assert |val| isn't the hole
+  // MagicValue.
+  void setDenseElementUnchecked(uint32_t index, const Value& val) {
+    MOZ_ASSERT(index < getDenseInitializedLength());
+    MOZ_ASSERT(!denseElementsAreCopyOnWrite());
+    MOZ_ASSERT(!denseElementsAreFrozen());
+    checkStoredValue(val);
+    elements_[index].set(this, HeapSlot::Element, unshiftedIndex(index), val);
+  }
+
   inline void addDenseElementType(JSContext* cx, uint32_t index,
                                   const Value& val);
 
   // Packed information for this object's elements.
   inline bool writeToIndexWouldMarkNotPacked(uint32_t index);
   inline void markDenseElementsNotPacked(JSContext* cx);
 
  public: