Use slow path in IterableToList for big input strings.

AllocateJSArray always allocates in new space, so we bailout of the fast
path for strings if the new array does not fit in new space.

Bug found by ClusterFuzz. Regression test added.

This also switches to the BranchIf pattern to avoid materialize a bool.

Bug: chromium:895860, v8:7980
Change-Id: Ic7c41268c394ac2796b7694252390ab50fd74838
Reviewed-on: https://chromium-review.googlesource.com/c/1286337Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Hai Dang <dhai@google.com>
Cr-Commit-Position: refs/heads/master@{#56759}

src/builtins/builtins-iterator-gen.cc

@@ -289,12 +289,12 @@ TF_BUILTIN(IterableToListWithSymbolLookup, IteratorBuiltinsAssembler) {
 
   BIND(&check_string);
   {
+    Label string_fast_call(this);
     StringBuiltinsAssembler string_assembler(state());
-    GotoIfNot(string_assembler.IsStringPrimitiveWithNoCustomIteration(iterable,
-                                                                      context),
-              &check_map);
+    string_assembler.BranchIfStringPrimitiveWithNoCustomIteration(
+        iterable, context, &string_fast_call, &check_map);
 
-    // Fast path for strings.
+    BIND(&string_fast_call);
     TailCallBuiltin(Builtins::kStringToList, context, iterable);
   }
 

src/builtins/builtins-string-gen.cc

@@ -2493,41 +2493,38 @@ TF_BUILTIN(StringIteratorPrototypeNext, StringBuiltinsAssembler) {
   }
 }
 
-TNode<BoolT> StringBuiltinsAssembler::IsStringPrimitiveWithNoCustomIteration(
-    TNode<Object> object, TNode<Context> context) {
-  Label if_false(this, Label::kDeferred), exit(this);
-  TVARIABLE(BoolT, var_result);
-
-  GotoIf(TaggedIsSmi(object), &if_false);
-  GotoIfNot(IsString(CAST(object)), &if_false);
+void StringBuiltinsAssembler::BranchIfStringPrimitiveWithNoCustomIteration(
+    TNode<Object> object, TNode<Context> context, Label* if_true,
+    Label* if_false) {
+  GotoIf(TaggedIsSmi(object), if_false);
+  GotoIfNot(IsString(CAST(object)), if_false);
+
+  // Bailout if the new array doesn't fit in new space.
+  const TNode<IntPtrT> length = LoadStringLengthAsWord(CAST(object));
+  // Since we don't have allocation site, base size does not include
+  // AllocationMemento::kSize.
+  GotoIfFixedArraySizeDoesntFitInNewSpace(
+      length, if_false, JSArray::kSize + FixedArray::kHeaderSize,
+      INTPTR_PARAMETERS);
 
   // Check that the String iterator hasn't been modified in a way that would
   // affect iteration.
   Node* protector_cell = LoadRoot(RootIndex::kStringIteratorProtector);
   DCHECK(isolate()->heap()->string_iterator_protector()->IsPropertyCell());
-  var_result =
-      WordEqual(LoadObjectField(protector_cell, PropertyCell::kValueOffset),
-                SmiConstant(Isolate::kProtectorValid));
-  Goto(&exit);
-
-  BIND(&if_false);
-  {
-    var_result = Int32FalseConstant();
-    Goto(&exit);
-  }
-
-  BIND(&exit);
-  return var_result.value();
+  Branch(WordEqual(LoadObjectField(protector_cell, PropertyCell::kValueOffset),
+                   SmiConstant(Isolate::kProtectorValid)),
+         if_true, if_false);
 }
 
+// This function assumes StringPrimitiveWithNoCustomIteration is true.
 TNode<JSArray> StringBuiltinsAssembler::StringToList(TNode<Context> context,
                                                      TNode<String> string) {
-  CSA_ASSERT(this, IsStringPrimitiveWithNoCustomIteration(string, context));
   const ElementsKind kind = PACKED_ELEMENTS;
   const TNode<IntPtrT> length = LoadStringLengthAsWord(string);
 
   Node* const array_map =
       LoadJSArrayElementsMap(kind, LoadNativeContext(context));
+  // Allocate the array to new space, assuming that the new array will fit in.
   Node* const array = AllocateJSArray(kind, array_map, length, SmiTag(length));
   Node* const elements = LoadElements(array);
 

src/builtins/builtins-string-gen.h

@@ -23,8 +23,10 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
                         Node* rhs, Node* rhs_instance_type,
                         TNode<IntPtrT> length, Label* if_equal,
                         Label* if_not_equal, Label* if_indirect);
-  TNode<BoolT> IsStringPrimitiveWithNoCustomIteration(TNode<Object> object,
-                                                      TNode<Context> context);
+  void BranchIfStringPrimitiveWithNoCustomIteration(TNode<Object> object,
+                                                    TNode<Context> context,
+                                                    Label* if_true,
+                                                    Label* if_false);
 
  protected:
   TNode<JSArray> StringToList(TNode<Context> context, TNode<String> string);

test/mjsunit/es6/regress/regress-cr895860.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.
+
+(function() {
+  var s = "f";
+
+  // 2^18 length, enough to ensure an array (of pointers) bigger than 500KB.
+  for (var i = 0; i < 18; i++) {
+    s += s;
+  }
+
+  var ss = [...s];
+})();