[CSA builtins] Fast case array iteration does unnecessary prototype walks

In ArrayBuiltinsAssembler::VisitAllFastElementsOneKind(), we enumerate
an arrays elements, carefully checking for the "hole" when required.
This code is only called for arrays whose prototype is the initial array
prototype. And the path is only available when the initial array
prototype is free of elements. Since that's the case, we only need to
verify that the initial array prototype remains free of elements during
an iteration with javascript callbacks. We don't need a body of code
that can walk the prototype chain looking for elements visible through
the "hole" value. In practice, this code was never run.

Change-Id: Iba5e275c559d495aa1cf6a4f29d66e2ce475c981
Reviewed-on: https://chromium-review.googlesource.com/1015023
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52660}

src/builtins/builtins-array-gen.cc

@@ -679,6 +679,9 @@ Node* ArrayBuiltinsAssembler::FindProcessor(Node* k_value, Node* k) {
       Label* array_changed, ParameterMode mode, ForEachDirection direction,
       MissingPropertyMode missing_property_mode, TNode<Smi> length) {
     Comment("begin VisitAllFastElementsOneKind");
+    // We only use this kind of processing if the no-elements protector is
+    // in place at the start. We'll continue checking during array iteration.
+    CSA_ASSERT(this, Word32BinaryNot(IsNoElementsProtectorCellInvalid()));
     VARIABLE(original_map, MachineRepresentation::kTagged);
     original_map.Bind(LoadMap(o()));
     VariableList list({&original_map, &a_, &k_, &to_}, zone());
@@ -730,10 +733,9 @@ Node* ArrayBuiltinsAssembler::FindProcessor(Node* k_value, Node* k) {
 
           BIND(&hole_element);
           if (missing_property_mode == MissingPropertyMode::kSkip) {
-            // Check if o's prototype change unexpectedly has elements after
-            // the callback in the case of a hole.
-            BranchIfPrototypesHaveNoElements(o_map, &one_element_done,
-                                             array_changed);
+            // The NoElementsProtectorCell could go invalid during callbacks.
+            Branch(IsNoElementsProtectorCellInvalid(), array_changed,
+                   &one_element_done);
           } else {
             value.Bind(UndefinedConstant());
             Goto(&process_element);

src/code-stub-assembler.cc

@@ -976,7 +976,7 @@ void CodeStubAssembler::BranchIfFastJSArray(Node* object, Node* context,
   Node* elements_kind = LoadMapElementsKind(map);
   GotoIfNot(IsFastElementsKind(elements_kind), if_false);
 
-  // Check prototype chain if receiver does not have packed elements
+  // Verify that our prototype is the initial array prototype.
   GotoIfNot(IsPrototypeInitialArrayPrototype(context, map), if_false);
 
   Branch(IsNoElementsProtectorCellInvalid(), if_false, if_true);

test/mjsunit/proto-elements-add-during-foreach.js

@@ -2,9 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var a = [0,1,2,,,,7];
-var proto = {}
-a.__proto__ = proto;
-var visits = 0;
-Array.prototype.forEach.call(a, (v,i,o) => { ++visits; proto[4] = 4; });
-assertEquals(5, visits);
+(()=>{
+  var a = [0,1,2,,,,7];
+  var proto = {}
+  a.__proto__ = proto;
+  var visits = 0;
+  Array.prototype.forEach.call(a, (v,i,o) => { ++visits; proto[4] = 4; });
+  assertEquals(5, visits);
+})();
+
+// We have a fast path for arrays with the initial array prototype.
+// If elements are inserted into the initial array prototype as we traverse
+// a holey array, we should gracefully exit the fast path.
+(()=>{
+  let a = [1, 2, 3,,,, 7];
+  function poison(v, i) {
+    if (i === 2) {
+      [].__proto__[4] = 3;
+    }
+    return v*v;
+  }
+  a.forEach(poison);
+})();