[array] Move SafeRemoveArrayHoles to runtime

This CL implements the functionality of SafeRemoveArrayHoles (JS),
which is used as a pre-processing step for sorting, in a runtime
function.

SafeRemoveArrayHoles is a generic fallback, when an existing runtime
function fails to remove holes/move undefineds to the end of an array.

This CL extends the existing runtime function to also support JSProxy
objects, and objects where indices have accessors.

R=cbruni@chromium.org, jgruber@chromium.org

Bug: v8:7382
Change-Id: I4881539cf2171caba08ff6e3e50320291f49839c
Reviewed-on: https://chromium-review.googlesource.com/1041950
Commit-Queue: Simon Zünd <szuend@google.com>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53060}

src/js/array.js

@@ -857,63 +857,6 @@ function InnerArraySort(array, length, comparefn) {
     }
   };
 
-  function SafeRemoveArrayHoles(obj) {
-    // Copy defined elements from the end to fill in all holes and undefineds
-    // in the beginning of the array.  Write undefineds and holes at the end
-    // after loop is finished.
-    var first_undefined = 0;
-    var last_defined = length - 1;
-    var num_holes = 0;
-    while (first_undefined < last_defined) {
-      // Find first undefined element.
-      while (first_undefined < last_defined &&
-             !IS_UNDEFINED(obj[first_undefined])) {
-        first_undefined++;
-      }
-      // Maintain the invariant num_holes = the number of holes in the original
-      // array with indices <= first_undefined or > last_defined.
-      if (!HAS_OWN_PROPERTY(obj, first_undefined)) {
-        num_holes++;
-      }
-
-      // Find last defined element.
-      while (first_undefined < last_defined &&
-             IS_UNDEFINED(obj[last_defined])) {
-        if (!HAS_OWN_PROPERTY(obj, last_defined)) {
-          num_holes++;
-        }
-        last_defined--;
-      }
-      if (first_undefined < last_defined) {
-        // Fill in hole or undefined.
-        obj[first_undefined] = obj[last_defined];
-        obj[last_defined] = UNDEFINED;
-      }
-    }
-    // If there were any undefineds in the entire array, first_undefined
-    // points to one past the last defined element.  Make this true if
-    // there were no undefineds, as well, so that first_undefined == number
-    // of defined elements.
-    if (!IS_UNDEFINED(obj[first_undefined])) first_undefined++;
-    // Fill in the undefineds and the holes.  There may be a hole where
-    // an undefined should be and vice versa.
-    var i;
-    for (i = first_undefined; i < length - num_holes; i++) {
-      obj[i] = UNDEFINED;
-    }
-    for (i = length - num_holes; i < length; i++) {
-      // For compatibility with Webkit, do not expose elements in the prototype.
-      if (i in %object_get_prototype_of(obj)) {
-        obj[i] = UNDEFINED;
-      } else {
-        delete obj[i];
-      }
-    }
-
-    // Return the number of defined elements.
-    return first_undefined;
-  };
-
   if (length < 2) return array;
 
   var is_array = IS_ARRAY(array);
@@ -930,16 +873,10 @@ function InnerArraySort(array, length, comparefn) {
     max_prototype_element = CopyFromPrototype(array, length);
   }
 
-  // %RemoveArrayHoles returns -1 if fast removal is not supported.
+  // %RemoveArrayHoles moves all non-undefined elements to the front of the
+  // array and moves the undefineds after that. Holes are removed.
   var num_non_undefined = %RemoveArrayHoles(array, length);
 
-  if (num_non_undefined == -1) {
-    // There were indexed accessors in the array.
-    // Move array holes and undefineds to the end using a Javascript function
-    // that is safe in the presence of accessors.
-    num_non_undefined = SafeRemoveArrayHoles(array);
-  }
-
   QuickSort(array, 0, num_non_undefined);
 
   if (!is_array && (num_non_undefined + 1 < max_prototype_element)) {

test/mjsunit/array-sort.js

@@ -585,6 +585,60 @@ assertThrows(() => {
   Array.prototype.sort.call(undefined);
 }, TypeError);
 
+// This test ensures that RemoveArrayHoles does not shadow indices in the
+// prototype chain. There are multiple code paths, we force both and check that
+// they have the same behavior.
+function TestPrototypeHoles() {
+  function test(forceGenericFallback) {
+    let proto2 = {
+      7: 27,
+    };
+
+    let proto1 = {
+      __proto__: proto2,
+      8: 18,
+      9: 19,
+    };
+
+    let xs = {
+      __proto__: proto1,
+      length: 10,
+      7: 7,
+      8: 8,
+      9: 9,
+    };
+
+    if (forceGenericFallback) {
+      Object.defineProperty(xs, "6", {
+        get: () => this.foo,
+        set: (val) => this.foo = val
+      });
+    }
+    xs[6] = 6;
+
+    Array.prototype.sort.call(xs, (a, b) => a - b);
+
+    assertEquals(10, xs.length);
+    assertEquals(6, xs[0]);
+    assertEquals(7, xs[1]);
+    assertEquals(8, xs[2]);
+    assertEquals(9, xs[3]);
+
+    // Index 7,8,9 will get the prototype values.
+    assertFalse(xs.hasOwnProperty(7));
+    assertEquals(27, xs[7]);
+
+    assertFalse(xs.hasOwnProperty(8));
+    assertEquals(18, xs[8]);
+
+    assertFalse(xs.hasOwnProperty(9));
+    assertEquals(19, xs[9]);
+  }
+
+  test(true);
+  test(false);
+}
+TestPrototypeHoles();
 
 // The following Tests make sure that there is no crash when the element kind
 // or the array length changes. Since comparison functions like this are not

test/mjsunit/regress/regress-crbug-743154.js

@@ -17,6 +17,8 @@ assertEquals(0, v[1073741824]);
 Array.prototype.sort.call(v);
 
 assertEquals(['0', '1073741824', 'length'], Object.keys(v));
+assertTrue(v.hasOwnProperty(0));
 assertEquals(1.5, v[0]);
+assertFalse(v.hasOwnProperty(1));
 assertEquals(1.5, v[1]);
 assertEquals(0, v[1073741824]);