[builtins] Fix Collection constructor when entries have custom iteration.

- Introduce new helper IsFastJSArrayWithNoCustomIteration.
  - Consolidates all entry array checks...
    - Is a fast array (defers to BranchIfFastJSArray)
    - No possibility that the Array's iteration protocol has been tampered with
- Introduce new BoolT constant helpers Int32TrueConstant and Int32FalseConstant.

Bug: chromium:804176, chromium:804188
Change-Id: I6b08396484682dc680b431ea564a7a28eeab8108
Reviewed-on: https://chromium-review.googlesource.com/883065
Commit-Queue: Peter Wong <peter.wm.wong@gmail.com>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50867}

src/bootstrapper.cc

@@ -5314,6 +5314,11 @@ bool Genesis::ConfigureGlobalObjects(
   native_context()->set_js_map_map(js_map_fun->initial_map());
   native_context()->set_js_set_map(js_set_fun->initial_map());
 
+  Handle<JSFunction> js_array_constructor(native_context()->array_function());
+  Handle<JSObject> js_array_prototype(
+      JSObject::cast(js_array_constructor->instance_prototype()));
+  native_context()->set_initial_array_prototype_map(js_array_prototype->map());
+
   return true;
 }
 

src/builtins/builtins-collections-gen.cc

@@ -42,8 +42,7 @@ class BaseCollectionsAssembler : public CodeStubAssembler {
   void AddConstructorEntries(Variant variant, TNode<Context> context,
                              TNode<Context> native_context,
                              TNode<Object> collection,
-                             TNode<Object> initial_entries,
-                             TNode<BoolT> is_fast_jsarray);
+                             TNode<Object> initial_entries);
 
   // Fast path for adding constructor entries.  Assumes the entries are a fast
   // JS array (see CodeStubAssembler::BranchIfFastJSArray()).
@@ -160,11 +159,12 @@ void BaseCollectionsAssembler::AddConstructorEntry(
 
 void BaseCollectionsAssembler::AddConstructorEntries(
     Variant variant, TNode<Context> context, TNode<Context> native_context,
-    TNode<Object> collection, TNode<Object> initial_entries,
-    TNode<BoolT> is_fast_jsarray) {
+    TNode<Object> collection, TNode<Object> initial_entries) {
+  TVARIABLE(BoolT, use_fast_loop,
+            IsFastJSArrayWithNoCustomIteration(initial_entries, context,
+                                               native_context));
   TNode<IntPtrT> at_least_space_for =
-      EstimatedInitialSize(initial_entries, is_fast_jsarray);
-  TVARIABLE(BoolT, use_fast_loop, is_fast_jsarray);
+      EstimatedInitialSize(initial_entries, use_fast_loop);
   Label allocate_table(this, &use_fast_loop), exit(this), fast_loop(this),
       slow_loop(this, Label::kDeferred);
   Goto(&allocate_table);
@@ -173,22 +173,25 @@ void BaseCollectionsAssembler::AddConstructorEntries(
     TNode<Object> table = AllocateTable(variant, context, at_least_space_for);
     StoreObjectField(collection, GetTableOffset(variant), table);
     GotoIf(IsNullOrUndefined(initial_entries), &exit);
+    GotoIfNot(
+        HasInitialCollectionPrototype(variant, native_context, collection),
+        &slow_loop);
     Branch(use_fast_loop, &fast_loop, &slow_loop);
   }
   BIND(&fast_loop);
   {
+    TNode<JSArray> initial_entries_jsarray =
+        UncheckedCast<JSArray>(initial_entries);
 #if DEBUG
-    CSA_ASSERT(this, IsFastJSArray(initial_entries, context));
-    TNode<Map> original_initial_entries_map = LoadMap(CAST(initial_entries));
+    CSA_ASSERT(this, IsFastJSArrayWithNoCustomIteration(
+                         initial_entries_jsarray, context, native_context));
+    TNode<Map> original_initial_entries_map = LoadMap(initial_entries_jsarray);
 #endif
-    GotoIfNot(
-        HasInitialCollectionPrototype(variant, native_context, collection),
-        &slow_loop);
 
     Label if_may_have_side_effects(this, Label::kDeferred);
-    AddConstructorEntriesFromFastJSArray(
-        variant, context, native_context, collection,
-        UncheckedCast<JSArray>(initial_entries), &if_may_have_side_effects);
+    AddConstructorEntriesFromFastJSArray(variant, context, native_context,
+                                         collection, initial_entries_jsarray,
+                                         &if_may_have_side_effects);
     Goto(&exit);
 
     if (variant == kMap || variant == kWeakMap) {
@@ -197,9 +200,9 @@ void BaseCollectionsAssembler::AddConstructorEntries(
       CSA_ASSERT(this, HasInitialCollectionPrototype(variant, native_context,
                                                      collection));
       CSA_ASSERT(this, WordEqual(original_initial_entries_map,
-                                 LoadMap(CAST(initial_entries))));
+                                 LoadMap(initial_entries_jsarray)));
 #endif
-      use_fast_loop = ReinterpretCast<BoolT>(Int32Constant(0));
+      use_fast_loop = Int32FalseConstant();
       Goto(&allocate_table);
     }
   }
@@ -222,8 +225,8 @@ void BaseCollectionsAssembler::AddConstructorEntriesFromFastJSArray(
   CSA_ASSERT(
       this,
       WordEqual(GetAddFunction(variant, native_context, collection), add_func));
-  CSA_ASSERT(this, IsFastJSArray(fast_jsarray, context));
-  CSA_ASSERT(this, IsFastElementsKind(elements_kind));
+  CSA_ASSERT(this, IsFastJSArrayWithNoCustomIteration(fast_jsarray, context,
+                                                      native_context));
   TNode<IntPtrT> length = SmiUntag(LoadFastJSArrayLength(fast_jsarray));
   CSA_ASSERT(this, IntPtrGreaterThanOrEqual(length, IntPtrConstant(0)));
   CSA_ASSERT(
@@ -363,15 +366,10 @@ void BaseCollectionsAssembler::GenerateConstructor(
   GotoIf(IsUndefined(new_target), &if_undefined);
 
   TNode<Context> native_context = LoadNativeContext(context);
-  TNode<BoolT> array_iterator_unchanged =
-      HasInitialArrayIteratorPrototypeMap(native_context);
-  TNode<BoolT> is_fast_jsarray = UncheckedCast<BoolT>(
-      Word32And(IsFastJSArray(iterable, context), array_iterator_unchanged));
   TNode<Object> collection = AllocateJSCollection(
       context, GetConstructor(variant, native_context), new_target);
 
-  AddConstructorEntries(variant, context, native_context, collection, iterable,
-                        is_fast_jsarray);
+  AddConstructorEntries(variant, context, native_context, collection, iterable);
   Return(collection);
 
   BIND(&if_undefined);

src/code-stub-assembler.cc

@@ -897,12 +897,40 @@ TNode<BoolT> CodeStubAssembler::IsFastJSArray(SloppyTNode<Object> object,
   TVARIABLE(BoolT, var_result);
   BIND(&if_true);
   {
-    var_result = ReinterpretCast<BoolT>(Int32Constant(1));
+    var_result = Int32TrueConstant();
     Goto(&exit);
   }
   BIND(&if_false);
   {
-    var_result = ReinterpretCast<BoolT>(Int32Constant(0));
+    var_result = Int32FalseConstant();
+    Goto(&exit);
+  }
+  BIND(&exit);
+  return var_result;
+}
+
+TNode<BoolT> CodeStubAssembler::IsFastJSArrayWithNoCustomIteration(
+    TNode<Object> object, TNode<Context> context,
+    TNode<Context> native_context) {
+  Label if_false(this, Label::kDeferred), if_fast(this), exit(this);
+  GotoIfForceSlowPath(&if_false);
+  TVARIABLE(BoolT, var_result, Int32TrueConstant());
+  BranchIfFastJSArray(object, context, &if_fast, &if_false);
+  BIND(&if_fast);
+  {
+    // Check if the Array.prototype[@@iterator] may have changed.
+    GotoIfNot(InitialArrayPrototypeHasInitialArrayPrototypeMap(native_context),
+              &if_false);
+    // Check if array[@@iterator] may have changed.
+    GotoIfNot(HasInitialFastElementsKindMap(native_context, CAST(object)),
+              &if_false);
+    // Check if the array iterator has changed.
+    Branch(HasInitialArrayIteratorPrototypeMap(native_context), &exit,
+           &if_false);
+  }
+  BIND(&if_false);
+  {
+    var_result = Int32FalseConstant();
     Goto(&exit);
   }
   BIND(&exit);
@@ -1304,6 +1332,7 @@ Node* CodeStubAssembler::HasInstanceType(Node* object,
 
 TNode<BoolT> CodeStubAssembler::HasInitialArrayIteratorPrototypeMap(
     TNode<Context> native_context) {
+  CSA_ASSERT(this, IsNativeContext(native_context));
   TNode<Map> arr_it_proto_map = LoadMap(CAST(LoadContextElement(
       native_context, Context::INITIAL_ARRAY_ITERATOR_PROTOTYPE_INDEX)));
   TNode<Map> initial_map = CAST(LoadContextElement(
@@ -1311,6 +1340,27 @@ TNode<BoolT> CodeStubAssembler::HasInitialArrayIteratorPrototypeMap(
   return WordEqual(arr_it_proto_map, initial_map);
 }
 
+TNode<BoolT>
+CodeStubAssembler::InitialArrayPrototypeHasInitialArrayPrototypeMap(
+    TNode<Context> native_context) {
+  CSA_ASSERT(this, IsNativeContext(native_context));
+  TNode<Map> proto_map = LoadMap(CAST(LoadContextElement(
+      native_context, Context::INITIAL_ARRAY_PROTOTYPE_INDEX)));
+  TNode<Map> initial_map = CAST(LoadContextElement(
+      native_context, Context::INITIAL_ARRAY_PROTOTYPE_MAP_INDEX));
+  return WordEqual(proto_map, initial_map);
+}
+
+TNode<BoolT> CodeStubAssembler::HasInitialFastElementsKindMap(
+    TNode<Context> native_context, TNode<JSArray> jsarray) {
+  CSA_ASSERT(this, IsNativeContext(native_context));
+  TNode<Map> map = LoadMap(jsarray);
+  TNode<Int32T> elements_kind = LoadMapElementsKind(map);
+  TNode<Map> initial_jsarray_element_map =
+      LoadJSArrayElementsMap(elements_kind, native_context);
+  return WordEqual(initial_jsarray_element_map, map);
+}
+
 Node* CodeStubAssembler::DoesntHaveInstanceType(Node* object,
                                                 InstanceType instance_type) {
   return Word32NotEqual(LoadInstanceType(object), Int32Constant(instance_type));

src/code-stub-assembler.h

@@ -502,9 +502,15 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   TNode<Int32T> LoadInstanceType(SloppyTNode<HeapObject> object);
   // Compare the instance the type of the object against the provided one.
   Node* HasInstanceType(Node* object, InstanceType type);
-  // Determines whether the Array Iterator's prototype has changed.
+  // Determines whether Array Iterator's prototype has changed.
   TNode<BoolT> HasInitialArrayIteratorPrototypeMap(
       TNode<Context> native_context);
+  // Determines whether Array's prototype has changed.
+  TNode<BoolT> InitialArrayPrototypeHasInitialArrayPrototypeMap(
+      TNode<Context> native_context);
+  // Determines whether an array's elements map has changed.
+  TNode<BoolT> HasInitialFastElementsKindMap(TNode<Context> native_context,
+                                             TNode<JSArray> jsarray);
   Node* DoesntHaveInstanceType(Node* object, InstanceType type);
   Node* TaggedDoesntHaveInstanceType(Node* any_tagged, InstanceType type);
   // Load the properties backing store of a JSObject.
@@ -1085,6 +1091,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* IsExternalStringInstanceType(Node* instance_type);
   TNode<BoolT> IsFastJSArray(SloppyTNode<Object> object,
                              SloppyTNode<Context> context);
+  TNode<BoolT> IsFastJSArrayWithNoCustomIteration(
+      TNode<Object> object, TNode<Context> context,
+      TNode<Context> native_context);
   Node* IsFeedbackVector(Node* object);
   Node* IsFixedArray(Node* object);
   Node* IsFixedArraySubclass(Node* object);

src/compiler/code-assembler.h

@@ -651,6 +651,12 @@ class V8_EXPORT_PRIVATE CodeAssembler {
   TNode<ExternalReference> ExternalConstant(ExternalReference address);
   TNode<Float64T> Float64Constant(double value);
   TNode<HeapNumber> NaNConstant();
+  TNode<BoolT> Int32TrueConstant() {
+    return ReinterpretCast<BoolT>(Int32Constant(1));
+  }
+  TNode<BoolT> Int32FalseConstant() {
+    return ReinterpretCast<BoolT>(Int32Constant(0));
+  }
 
   bool ToInt32Constant(Node* node, int32_t& out_value);
   bool ToInt64Constant(Node* node, int64_t& out_value);

src/contexts.h

@@ -262,6 +262,7 @@ enum ContextLookupFlags {
   V(INITIAL_ARRAY_ITERATOR_PROTOTYPE_MAP_INDEX, Map,                           \
     initial_array_iterator_prototype_map)                                      \
   V(INITIAL_ARRAY_PROTOTYPE_INDEX, JSObject, initial_array_prototype)          \
+  V(INITIAL_ARRAY_PROTOTYPE_MAP_INDEX, Map, initial_array_prototype_map)       \
   V(INITIAL_ERROR_PROTOTYPE_INDEX, JSObject, initial_error_prototype)          \
   V(INITIAL_GENERATOR_PROTOTYPE_INDEX, JSObject, initial_generator_prototype)  \
   V(INITIAL_ASYNC_GENERATOR_PROTOTYPE_INDEX, JSObject,                         \

test/mjsunit/es6/collections-constructor-custom-iterator.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --opt
+
+function TestSetWithCustomIterator(ctor) {
+  const k1 = {};
+  const k2 = {};
+  const entries = [k1];
+  let callCount = 0;
+  entries[Symbol.iterator] = () => ({
+    next: () =>
+      callCount++ === 0
+        ? { value: k2, done: false }
+        : { done: true }
+  });
+  const set = new ctor(entries);
+  assertFalse(set.has(k1));
+  assertTrue(set.has(k2));
+  assertEquals(2, callCount);
+}
+TestSetWithCustomIterator(Set);
+TestSetWithCustomIterator(Set);
+TestSetWithCustomIterator(Set);
+%OptimizeFunctionOnNextCall(TestSetWithCustomIterator);
+TestSetWithCustomIterator(Set);
+assertOptimized(TestSetWithCustomIterator);
+
+TestSetWithCustomIterator(WeakSet);
+TestSetWithCustomIterator(WeakSet);
+TestSetWithCustomIterator(WeakSet);
+%OptimizeFunctionOnNextCall(TestSetWithCustomIterator);
+TestSetWithCustomIterator(WeakSet);
+assertOptimized(TestSetWithCustomIterator);
+
+function TestMapWithCustomIterator(ctor) {
+  const k1 = {};
+  const k2 = {};
+  const entries = [[k1, 1]];
+  let callCount = 0;
+  entries[Symbol.iterator] = () => ({
+    next: () =>
+      callCount++ === 0
+        ? { value: [k2, 2], done: false }
+        : { done: true }
+  });
+  const map = new ctor(entries);
+  assertFalse(map.has(k1));
+  assertEquals(2, map.get(k2));
+  assertEquals(2, callCount);
+}
+TestMapWithCustomIterator(Map);
+TestMapWithCustomIterator(Map);
+TestMapWithCustomIterator(Map);
+%OptimizeFunctionOnNextCall(TestMapWithCustomIterator);
+TestMapWithCustomIterator(Map);
+assertOptimized(TestMapWithCustomIterator);
+
+TestMapWithCustomIterator(WeakMap);
+TestMapWithCustomIterator(WeakMap);
+TestMapWithCustomIterator(WeakMap);
+%OptimizeFunctionOnNextCall(TestMapWithCustomIterator);
+TestMapWithCustomIterator(WeakMap);
+assertOptimized(TestMapWithCustomIterator);

test/mjsunit/es6/collections-constructor-with-modified-array-prototype.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --opt
+
+function TestSetWithCustomIterator(ctor) {
+  const k1 = {};
+  const k2 = {};
+  let callCount = 0;
+  Array.prototype[Symbol.iterator] = () => ({
+    next: () =>
+      callCount++ === 0
+        ? { value: k2, done: false }
+        : { done: true }
+  });
+  const entries = [k1];
+  const set = new ctor(entries);
+  assertFalse(set.has(k1));
+  assertTrue(set.has(k2));
+  assertEquals(2, callCount);
+}
+TestSetWithCustomIterator(Set);
+TestSetWithCustomIterator(Set);
+TestSetWithCustomIterator(Set);
+%OptimizeFunctionOnNextCall(TestSetWithCustomIterator);
+TestSetWithCustomIterator(Set);
+assertOptimized(TestSetWithCustomIterator);
+
+TestSetWithCustomIterator(WeakSet);
+TestSetWithCustomIterator(WeakSet);
+TestSetWithCustomIterator(WeakSet);
+%OptimizeFunctionOnNextCall(TestSetWithCustomIterator);
+TestSetWithCustomIterator(WeakSet);
+assertOptimized(TestSetWithCustomIterator);
+
+function TestMapWithCustomIterator(ctor) {
+  const k1 = {};
+  const k2 = {};
+  let callCount = 0;
+  Array.prototype[Symbol.iterator] = () => ({
+    next: () =>
+      callCount++ === 0
+        ? { value: [k2, 2], done: false }
+        : { done: true }
+  });
+  const entries = [[k1, 1]];
+  const map = new ctor(entries);
+  assertFalse(map.has(k1));
+  assertEquals(2, map.get(k2));
+  assertEquals(2, callCount);
+}
+TestMapWithCustomIterator(Map);
+TestMapWithCustomIterator(Map);
+TestMapWithCustomIterator(Map);
+%OptimizeFunctionOnNextCall(TestMapWithCustomIterator);
+TestMapWithCustomIterator(Map);
+assertOptimized(TestMapWithCustomIterator);
+
+TestMapWithCustomIterator(WeakMap);
+TestMapWithCustomIterator(WeakMap);
+TestMapWithCustomIterator(WeakMap);
+%OptimizeFunctionOnNextCall(TestMapWithCustomIterator);
+TestMapWithCustomIterator(WeakMap);
+assertOptimized(TestMapWithCustomIterator);

test/mjsunit/regress/regress-804176.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+const set_entries = [{}];
+set_entries[Symbol.iterator] = function() {};
+assertThrows(() => new Set(set_entries), TypeError);
+assertThrows(() => new WeakSet(set_entries), TypeError);
+
+const map_entries = [[{}, 1]];
+map_entries[Symbol.iterator] = function() {};
+assertThrows(() => new Set(map_entries), TypeError);
+assertThrows(() => new WeakSet(map_entries), TypeError);

test/mjsunit/regress/regress-804188.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+Object.defineProperty(Array.prototype, Symbol.iterator, {
+  value: function* () {}
+});
+const arrayIteratorProto = Object.getPrototypeOf([][Symbol.iterator]());
+arrayIteratorProto.next = function() {};
+
+assertThrows(() => new Map([[{}, 1], [{}, 2]]), TypeError);
+assertThrows(() => new WeakMap([[{}, 1], [{}, 2]]), TypeError);
+assertThrows(() => new Set([{}]), TypeError);
+assertThrows(() => new WeakSet([{}]), TypeError);