Bug 1660599: Only StoreDenseElementHole with handleAdd=true guarantees writable array length. r=jandem

We can only assert that the array length is writable for the
`handleTrue = true` case. This also requires to reintroduce the writable array
length check to the Ion IC code.

Added tests to cover multiple scenarios where the StoreDenseElementHole IC is
used.

Differential Revision: https://phabricator.services.mozilla.com/D88031

js/src/jit-test/tests/cacheir/add-dense-element-non-extensible.js

@@ -0,0 +1,160 @@
+// Add dense elements to packed and non-packed arrays. Cover both mono- and
+// polymorphic call sites. Change array to non-extensible during execution.
+
+function testAddDenseEmpty() {
+  var array = [];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 0; i < 10; ++i) {
+    if (i === 5) {
+      Object.preventExtensions(array);
+    }
+    store(array, i);
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDenseEmpty();
+
+function testAddDensePacked() {
+  var array = [0, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 2; i < 10; ++i) {
+    if (i === 5) {
+      Object.preventExtensions(array);
+    }
+    store(array, i);
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDensePacked();
+
+function testAddDenseNonPacked() {
+  var array = [/* hole */, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 2; i < 10; ++i) {
+    if (i === 5) {
+      Object.preventExtensions(array);
+    }
+    store(array, i);
+  }
+
+  assertEq(array.length, 5);
+  assertEq(0 in array, false);
+  for (var i = 1; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDenseNonPacked();
+
+function testAddDenseEmptyPoly() {
+  var array = [];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 0; i < 10; ++i) {
+    if (i === 5) {
+      Object.preventExtensions(array);
+    }
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDenseEmptyPoly();
+
+function testAddDensePackedPoly() {
+  var array = [0, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 2; i < 10; ++i) {
+    if (i === 5) {
+      Object.preventExtensions(array);
+    }
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDensePackedPoly();
+
+function testAddDenseNonPackedPoly() {
+  var array = [/* hole */, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 2; i < 10; ++i) {
+    if (i === 5) {
+      Object.preventExtensions(array);
+    }
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 5);
+  assertEq(0 in array, false);
+  for (var i = 1; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDenseNonPackedPoly();

js/src/jit-test/tests/cacheir/add-dense-element-non-writable-length.js

@@ -0,0 +1,160 @@
+// Add dense elements to packed and non-packed arrays. Cover both mono- and
+// polymorphic call sites. Change array length to non-writable during execution.
+
+function testAddDenseEmpty() {
+  var array = [];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 0; i < 10; ++i) {
+    if (i === 5) {
+      Object.defineProperty(array, "length", {writable: false});
+    }
+    store(array, i);
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDenseEmpty();
+
+function testAddDensePacked() {
+  var array = [0, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 2; i < 10; ++i) {
+    if (i === 5) {
+      Object.defineProperty(array, "length", {writable: false});
+    }
+    store(array, i);
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDensePacked();
+
+function testAddDenseNonPacked() {
+  var array = [/* hole */, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 2; i < 10; ++i) {
+    if (i === 5) {
+      Object.defineProperty(array, "length", {writable: false});
+    }
+    store(array, i);
+  }
+
+  assertEq(array.length, 5);
+  assertEq(0 in array, false);
+  for (var i = 1; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDenseNonPacked();
+
+function testAddDenseEmptyPoly() {
+  var array = [];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 0; i < 10; ++i) {
+    if (i === 5) {
+      Object.defineProperty(array, "length", {writable: false});
+    }
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDenseEmptyPoly();
+
+function testAddDensePackedPoly() {
+  var array = [0, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 2; i < 10; ++i) {
+    if (i === 5) {
+      Object.defineProperty(array, "length", {writable: false});
+    }
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDensePackedPoly();
+
+function testAddDenseNonPackedPoly() {
+  var array = [/* hole */, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 2; i < 10; ++i) {
+    if (i === 5) {
+      Object.defineProperty(array, "length", {writable: false});
+    }
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 5);
+  assertEq(0 in array, false);
+  for (var i = 1; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testAddDenseNonPackedPoly();

js/src/jit-test/tests/cacheir/add-dense-element.js

@@ -0,0 +1,124 @@
+// Add dense elements to packed and non-packed arrays. Cover both mono- and
+// polymorphic call sites.
+
+function testAddDenseEmpty() {
+  var array = [];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 0; i < 10; ++i) {
+    store(array, i);
+  }
+
+  assertEq(array.length, 10);
+  for (var i = 0; i < 10; ++i) {
+    assertEq(array[i], i);
+  }
+}
+testAddDenseEmpty();
+
+function testAddDensePacked() {
+  var array = [0, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 2; i < 10; ++i) {
+    store(array, i);
+  }
+
+  assertEq(array.length, 10);
+  for (var i = 0; i < 10; ++i) {
+    assertEq(array[i], i);
+  }
+}
+testAddDensePacked();
+
+function testAddDenseNonPacked() {
+  var array = [/* hole */, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 2; i < 10; ++i) {
+    store(array, i);
+  }
+
+  assertEq(array.length, 10);
+  assertEq(0 in array, false);
+  for (var i = 1; i < 10; ++i) {
+    assertEq(array[i], i);
+  }
+}
+testAddDenseNonPacked();
+
+function testAddDenseEmptyPoly() {
+  var array = [];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 0; i < 10; ++i) {
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 10);
+  for (var i = 0; i < 10; ++i) {
+    assertEq(array[i], i);
+  }
+}
+testAddDenseEmptyPoly();
+
+function testAddDensePackedPoly() {
+  var array = [0, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 2; i < 10; ++i) {
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 10);
+  for (var i = 0; i < 10; ++i) {
+    assertEq(array[i], i);
+  }
+}
+testAddDensePackedPoly();
+
+function testAddDenseNonPackedPoly() {
+  var array = [/* hole */, 1];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 2; i < 10; ++i) {
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 10);
+  assertEq(0 in array, false);
+  for (var i = 1; i < 10; ++i) {
+    assertEq(array[i], i);
+  }
+}
+testAddDenseNonPackedPoly();

js/src/jit-test/tests/cacheir/store-dense-element-hole-non-extensible.js

@@ -0,0 +1,55 @@
+// Store an element into a previous hole value and later add more elements
+// exceeding the initialised length. Cover both mono- and polymorphic call
+// sites. Change array length to non-extensible during execution.
+
+function testStoreDenseHole() {
+  var array = [/* hole */, /* hole */, /* hole */, /* hole */, ];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 0; i < 10; ++i) {
+    if (i === 5) {
+      Object.preventExtensions(array);
+    }
+    store(array, i);
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testStoreDenseHole();
+
+function testStoreDenseHolePoly() {
+  var array = [/* hole */, /* hole */, /* hole */, /* hole */, ];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 0; i < 10; ++i) {
+    if (i === 5) {
+      Object.preventExtensions(array);
+    }
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testStoreDenseHolePoly();

js/src/jit-test/tests/cacheir/store-dense-element-hole-non-writable-length-at-start.js

@@ -0,0 +1,55 @@
+// Store an element into a previous hole value and later add more elements
+// exceeding the initialised length. Cover both mono- and polymorphic call
+// sites. The array has a non-writable length at the start.
+
+function testStoreDenseHole() {
+  var array = [/* hole */, /* hole */, /* hole */, /* hole */, ];
+  Object.defineProperty(array, "length", {
+      writable: false
+  });
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 0; i < 10; ++i) {
+    store(array, i);
+  }
+
+  assertEq(array.length, 4);
+  for (var i = 0; i < 4; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 4; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testStoreDenseHole();
+
+function testStoreDenseHolePoly() {
+  var array = [/* hole */, /* hole */, /* hole */, /* hole */, ];
+  Object.defineProperty(array, "length", {
+    writable: false
+  });
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 0; i < 10; ++i) {
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 4);
+  for (var i = 0; i < 4; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 4; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testStoreDenseHolePoly();

js/src/jit-test/tests/cacheir/store-dense-element-hole-non-writable-length.js

@@ -0,0 +1,55 @@
+// Store an element into a previous hole value and later add more elements
+// exceeding the initialised length. Cover both mono- and polymorphic call
+// sites. Change array length to non-writable during execution.
+
+function testStoreDenseHole() {
+  var array = [/* hole */, /* hole */, /* hole */, /* hole */, ];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 0; i < 10; ++i) {
+    if (i === 5) {
+      Object.defineProperty(array, "length", {writable: false});
+    }
+    store(array, i);
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testStoreDenseHole();
+
+function testStoreDenseHolePoly() {
+  var array = [/* hole */, /* hole */, /* hole */, /* hole */, ];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 0; i < 10; ++i) {
+    if (i === 5) {
+      Object.defineProperty(array, "length", {writable: false});
+    }
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 5);
+  for (var i = 0; i < 5; ++i) {
+    assertEq(array[i], i);
+  }
+  for (var i = 5; i < 10; ++i) {
+    assertEq(i in array, false);
+  }
+}
+testStoreDenseHolePoly();

js/src/jit-test/tests/cacheir/store-dense-element-hole.js

@@ -0,0 +1,43 @@
+// Store an element into a previous hole value and later add more elements
+// exceeding the initialised length. Cover both mono- and polymorphic call
+// sites.
+
+function testStoreDenseHole() {
+  var array = [/* hole */, /* hole */, /* hole */, /* hole */, ];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  for (var i = 0; i < 10; ++i) {
+    store(array, i);
+  }
+
+  assertEq(array.length, 10);
+  for (var i = 0; i < 10; ++i) {
+    assertEq(array[i], i);
+  }
+}
+testStoreDenseHole();
+
+function testStoreDenseHolePoly() {
+  var array = [/* hole */, /* hole */, /* hole */, /* hole */, ];
+
+  function store(ar, index) {
+    ar[index] = index;
+  }
+
+  var objects = [array, {}];
+
+  for (var i = 0; i < 10; ++i) {
+    for (var j = 0; j < objects.length; ++j) {
+      store(objects[j], i);
+    }
+  }
+
+  assertEq(array.length, 10);
+  for (var i = 0; i < 10; ++i) {
+    assertEq(array[i], i);
+  }
+}
+testStoreDenseHolePoly();

js/src/jit/BaselineCacheIRCompiler.cpp

@@ -1164,19 +1164,30 @@ bool BaselineCacheIRCompiler::emitStoreDenseElement(ObjOperandId objId,
   return true;
 }
 
-static void EmitAssertExtensibleAndWritableArrayLength(MacroAssembler& masm,
+static void EmitAssertExtensibleElements(MacroAssembler& masm,
                                          Register elementsReg) {
 #ifdef DEBUG
-  // Preceding shape guards ensure the object is extensible and the array length
-  // is writable.
+  // Preceding shape guards ensure the object elements are extensible.
   Address elementsFlags(elementsReg, ObjectElements::offsetOfFlags());
   Label ok;
   masm.branchTest32(Assembler::Zero, elementsFlags,
-                    Imm32(ObjectElements::Flags::NOT_EXTENSIBLE |
-                          ObjectElements::Flags::NONWRITABLE_ARRAY_LENGTH),
+                    Imm32(ObjectElements::Flags::NOT_EXTENSIBLE),
                     &ok);
-  masm.assumeUnreachable(
-      "Unexpected non-extensible object or non-writable array length");
+  masm.assumeUnreachable("Unexpected non-extensible elements");
+  masm.bind(&ok);
+#endif
+}
+
+static void EmitAssertWritableArrayLengthElements(MacroAssembler& masm,
+                                                  Register elementsReg) {
+#ifdef DEBUG
+  // Preceding shape guards ensure the array length is writable.
+  Address elementsFlags(elementsReg, ObjectElements::offsetOfFlags());
+  Label ok;
+  masm.branchTest32(Assembler::Zero, elementsFlags,
+                    Imm32(ObjectElements::Flags::NONWRITABLE_ARRAY_LENGTH),
+                    &ok);
+  masm.assumeUnreachable("Unexpected non-writable array length elements");
   masm.bind(&ok);
 #endif
 }
@@ -1203,7 +1214,10 @@ bool BaselineCacheIRCompiler::emitStoreDenseElementHole(ObjOperandId objId,
   // Load obj->elements in scratch.
   masm.loadPtr(Address(obj, NativeObject::offsetOfElements()), scratch);
 
-  EmitAssertExtensibleAndWritableArrayLength(masm, scratch);
+  EmitAssertExtensibleElements(masm, scratch);
+  if (handleAdd) {
+    EmitAssertWritableArrayLengthElements(masm, scratch);
+  }
 
   BaseObjectElementIndex element(scratch, index);
   Address initLength(scratch, ObjectElements::offsetOfInitializedLength());
@@ -1348,7 +1362,8 @@ bool BaselineCacheIRCompiler::emitArrayPush(ObjOperandId objId,
   // Load obj->elements in scratch.
   masm.loadPtr(Address(obj, NativeObject::offsetOfElements()), scratch);
 
-  EmitAssertExtensibleAndWritableArrayLength(masm, scratch);
+  EmitAssertExtensibleElements(masm, scratch);
+  EmitAssertWritableArrayLengthElements(masm, scratch);
 
   Address elementsInitLength(scratch,
                              ObjectElements::offsetOfInitializedLength());

js/src/jit/IonCacheIRCompiler.cpp

@@ -1687,19 +1687,30 @@ static void EmitAssertNoCopyOnWriteElements(MacroAssembler& masm,
 #endif
 }
 
-static void EmitAssertExtensibleAndWritableArrayLength(MacroAssembler& masm,
+static void EmitAssertExtensibleElements(MacroAssembler& masm,
                                          Register elementsReg) {
 #ifdef DEBUG
-  // Preceding shape guards ensure the object is extensible and the array length
-  // is writable.
+  // Preceding shape guards ensure the object elements are extensible.
   Address elementsFlags(elementsReg, ObjectElements::offsetOfFlags());
   Label ok;
   masm.branchTest32(Assembler::Zero, elementsFlags,
-                    Imm32(ObjectElements::Flags::NOT_EXTENSIBLE |
-                          ObjectElements::Flags::NONWRITABLE_ARRAY_LENGTH),
+                    Imm32(ObjectElements::Flags::NOT_EXTENSIBLE),
                     &ok);
-  masm.assumeUnreachable(
-      "Unexpected non-extensible object or non-writable array length");
+  masm.assumeUnreachable("Unexpected non-extensible elements");
+  masm.bind(&ok);
+#endif
+}
+
+static void EmitAssertWritableArrayLengthElements(MacroAssembler& masm,
+                                                  Register elementsReg) {
+#ifdef DEBUG
+  // Preceding shape guards ensure the array length is writable.
+  Address elementsFlags(elementsReg, ObjectElements::offsetOfFlags());
+  Label ok;
+  masm.branchTest32(Assembler::Zero, elementsFlags,
+                    Imm32(ObjectElements::Flags::NONWRITABLE_ARRAY_LENGTH),
+                    &ok);
+  masm.assumeUnreachable("Unexpected non-writable array length elements");
   masm.bind(&ok);
 #endif
 }
@@ -1774,7 +1785,10 @@ bool IonCacheIRCompiler::emitStoreDenseElementHole(ObjOperandId objId,
 
   EmitAssertNoCopyOnWriteElements(masm, scratch1);
 
-  EmitAssertExtensibleAndWritableArrayLength(masm, scratch1);
+  EmitAssertExtensibleElements(masm, scratch1);
+  if (handleAdd) {
+    EmitAssertWritableArrayLengthElements(masm, scratch1);
+  }
 
   Address initLength(scratch1, ObjectElements::offsetOfInitializedLength());
   BaseObjectElementIndex element(scratch1, index);
@@ -1793,7 +1807,15 @@ bool IonCacheIRCompiler::emitStoreDenseElementHole(ObjOperandId objId,
   masm.spectreBoundsCheck32(index, capacity, spectreScratch, &allocElement);
   masm.jump(&capacityOk);
 
+  // Check for non-writable array length. We only have to do this if
+  // index >= capacity and handleAdd is false.
   masm.bind(&allocElement);
+  if (!handleAdd) {
+    Address elementsFlags(scratch1, ObjectElements::offsetOfFlags());
+    masm.branchTest32(Assembler::NonZero, elementsFlags,
+                      Imm32(ObjectElements::NONWRITABLE_ARRAY_LENGTH),
+                      failure->label());
+  }
 
   LiveRegisterSet save(GeneralRegisterSet::Volatile(), liveVolatileFloatRegs());
   save.takeUnchecked(scratch1);