[builtins] Fix String#pad{Start,End} for a large maxLength argument.

If maxLength is larger than String::kMaxLength, we used to throw
immediately. However, we must first look at the filler argument, which
is observable. Moreover, if the filler is empty, we must return the
input unchanged.

Bug: v8:8078
Change-Id: Ic3d135f9e25da56df45b059144e45e19dda9c3d8
Reviewed-on: https://chromium-review.googlesource.com/1188313
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55414}

src/builtins/builtins-string-gen.cc

@@ -1640,7 +1640,8 @@ class StringPadAssembler : public StringBuiltinsAssembler {
     TVARIABLE(String, var_fill_string, StringConstant(" "));
     TVARIABLE(IntPtrT, var_fill_length, IntPtrConstant(1));
 
-    Label argc_2(this), dont_pad(this), invalid_string_length(this), pad(this);
+    Label check_fill(this), dont_pad(this), invalid_string_length(this),
+        pad(this);
 
     // If no max_length was provided, return the string.
     GotoIf(IntPtrEqual(argc, IntPtrConstant(0)), &dont_pad);
@@ -1649,41 +1650,41 @@ class StringPadAssembler : public StringBuiltinsAssembler {
         ToLength_Inline(context, arguments.AtIndex(0));
     CSA_ASSERT(this, IsNumberNormalized(max_length));
 
-    // Throw if max_length is not a smi or greater than the max string length.
+    // If max_length <= string_length, return the string.
+    GotoIfNot(TaggedIsSmi(max_length), &check_fill);
+    Branch(SmiLessThanOrEqual(CAST(max_length), string_length), &dont_pad,
+           &check_fill);
+
+    BIND(&check_fill);
+    {
+      GotoIf(IntPtrEqual(argc, IntPtrConstant(1)), &pad);
+      Node* const fill = arguments.AtIndex(1);
+      GotoIf(IsUndefined(fill), &pad);
+
+      var_fill_string = ToString_Inline(context, fill);
+      var_fill_length = LoadStringLengthAsWord(var_fill_string.value());
+      Branch(WordEqual(var_fill_length.value(), IntPtrConstant(0)), &dont_pad,
+             &pad);
+    }
+
+    BIND(&pad);
+    {
+      CSA_ASSERT(this,
+                 IntPtrGreaterThan(var_fill_length.value(), IntPtrConstant(0)));
+
+      // Throw if max_length is greater than String::kMaxLength.
       GotoIfNot(TaggedIsSmi(max_length), &invalid_string_length);
       TNode<Smi> smi_max_length = CAST(max_length);
       GotoIfNot(
           SmiLessThanOrEqual(smi_max_length, SmiConstant(String::kMaxLength)),
           &invalid_string_length);
 
-    // If the max_length is less than length of the string, return the string.
-    CSA_ASSERT(this, TaggedIsPositiveSmi(smi_max_length));
-    GotoIf(SmiLessThanOrEqual(smi_max_length, string_length), &dont_pad);
-
-    Branch(IntPtrEqual(argc, IntPtrConstant(1)), &pad, &argc_2);
-    BIND(&argc_2);
-    {
-      Node* const fill = arguments.AtIndex(1);
-      GotoIf(IsUndefined(fill), &pad);
-
-      var_fill_string = ToString_Inline(context, fill);
-      var_fill_length = LoadStringLengthAsWord(var_fill_string.value());
-
-      Branch(IntPtrGreaterThan(var_fill_length.value(), IntPtrConstant(0)),
-             &pad, &dont_pad);
-    }
-    BIND(&pad);
-    {
-      CSA_ASSERT(this,
-                 IntPtrGreaterThan(var_fill_length.value(), IntPtrConstant(0)));
-      CSA_ASSERT(this, SmiGreaterThan(smi_max_length, string_length));
-
       Callable stringadd_callable =
           CodeFactory::StringAdd(isolate(), STRING_ADD_CHECK_NONE, NOT_TENURED);
+      CSA_ASSERT(this, SmiGreaterThan(smi_max_length, string_length));
       TNode<Smi> const pad_length = SmiSub(smi_max_length, string_length);
 
       VARIABLE(var_pad, MachineRepresentation::kTagged);
-
       Label single_char_fill(this), multi_char_fill(this), return_result(this);
       Branch(IntPtrEqual(var_fill_length.value(), IntPtrConstant(1)),
              &single_char_fill, &multi_char_fill);

test/mjsunit/string-pad.js

@@ -0,0 +1,155 @@
+// 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.
+
+class MyError {};
+const throwing = {toString() {throw new MyError}};
+const empties = ['', {toString() {return ''}}];
+
+{
+  const s = '';
+
+  assertThrows(_ => s.padStart(Symbol(), throwing), TypeError);
+  assertEquals(s, s.padStart(NaN, throwing));
+  assertEquals(s, s.padStart(-Infinity, throwing));
+  assertEquals(s, s.padStart(-9, throwing));
+  assertEquals(s, s.padStart(-1, throwing));
+  assertEquals(s, s.padStart(-0, throwing));
+  assertEquals(s, s.padStart(0, throwing));
+  assertThrows(_ => s.padStart(3, throwing), MyError);
+  assertThrows(_ => s.padStart(9, throwing), MyError);
+  assertThrows(_ => s.padStart(2**31-1, throwing), MyError);
+  assertThrows(_ => s.padStart(2**31, throwing), MyError);
+  assertThrows(_ => s.padStart(2**32-1, throwing), MyError);
+  assertThrows(_ => s.padStart(2**32, throwing), MyError);
+  assertThrows(_ => s.padStart(2**53-1, throwing), MyError);
+  assertThrows(_ => s.padStart(2**53, throwing), MyError);
+  assertThrows(_ => s.padStart(Infinity, throwing), MyError);
+
+  assertThrows(_ => s.padEnd(Symbol(), throwing), TypeError);
+  assertEquals(s, s.padEnd(NaN, throwing));
+  assertEquals(s, s.padEnd(-Infinity, throwing));
+  assertEquals(s, s.padEnd(-9, throwing));
+  assertEquals(s, s.padEnd(-1, throwing));
+  assertEquals(s, s.padEnd(-0, throwing));
+  assertEquals(s, s.padEnd(0, throwing));
+  assertThrows(_ => s.padEnd(3, throwing), MyError);
+  assertThrows(_ => s.padEnd(9, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**31-1, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**31, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**32-1, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**32, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**53-1, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**53, throwing), MyError);
+  assertThrows(_ => s.padEnd(Infinity, throwing), MyError);
+
+  for (const empty of empties) {
+    assertThrows(_ => s.padStart(Symbol(), empty), TypeError);
+    assertEquals(s, s.padStart(NaN, empty));
+    assertEquals(s, s.padStart(-Infinity, empty));
+    assertEquals(s, s.padStart(-9, empty));
+    assertEquals(s, s.padStart(-1, empty));
+    assertEquals(s, s.padStart(-0, empty));
+    assertEquals(s, s.padStart(0, empty));
+    assertEquals(s, s.padStart(3, empty));
+    assertEquals(s, s.padStart(9, empty));
+    assertEquals(s, s.padStart(2**31-1, empty));
+    assertEquals(s, s.padStart(2**31, empty));
+    assertEquals(s, s.padStart(2**32-1, empty));
+    assertEquals(s, s.padStart(2**32, empty));
+    assertEquals(s, s.padStart(2**53-1, empty));
+    assertEquals(s, s.padStart(2**53, empty));
+    assertEquals(s, s.padStart(Infinity, empty));
+
+    assertThrows(_ => s.padEnd(Symbol(), empty), TypeError);
+    assertEquals(s, s.padEnd(NaN, empty));
+    assertEquals(s, s.padEnd(-Infinity, empty));
+    assertEquals(s, s.padEnd(-9, empty));
+    assertEquals(s, s.padEnd(-1, empty));
+    assertEquals(s, s.padEnd(-0, empty));
+    assertEquals(s, s.padEnd(0, empty));
+    assertEquals(s, s.padEnd(3, empty));
+    assertEquals(s, s.padEnd(9, empty));
+    assertEquals(s, s.padEnd(2**31-1, empty));
+    assertEquals(s, s.padEnd(2**31, empty));
+    assertEquals(s, s.padEnd(2**32-1, empty));
+    assertEquals(s, s.padEnd(2**32, empty));
+    assertEquals(s, s.padEnd(2**53-1, empty));
+    assertEquals(s, s.padEnd(2**53, empty));
+    assertEquals(s, s.padEnd(Infinity, empty));
+  }
+}
+
+{
+  const s = 'hello';
+
+  assertThrows(_ => s.padStart(Symbol(), throwing), TypeError);
+  assertEquals(s, s.padStart(NaN, throwing));
+  assertEquals(s, s.padStart(-Infinity, throwing));
+  assertEquals(s, s.padStart(-9, throwing));
+  assertEquals(s, s.padStart(-1, throwing));
+  assertEquals(s, s.padStart(-0, throwing));
+  assertEquals(s, s.padStart(0, throwing));
+  assertEquals(s, s.padStart(3, throwing));
+  assertThrows(_ => s.padStart(9, throwing), MyError);
+  assertThrows(_ => s.padStart(2**31-1, throwing), MyError);
+  assertThrows(_ => s.padStart(2**31, throwing), MyError);
+  assertThrows(_ => s.padStart(2**32-1, throwing), MyError);
+  assertThrows(_ => s.padStart(2**32, throwing), MyError);
+  assertThrows(_ => s.padStart(2**53-1, throwing), MyError);
+  assertThrows(_ => s.padStart(2**53, throwing), MyError);
+  assertThrows(_ => s.padStart(Infinity, throwing), MyError);
+
+  assertThrows(_ => s.padEnd(Symbol(), throwing), TypeError);
+  assertEquals(s, s.padEnd(NaN, throwing));
+  assertEquals(s, s.padEnd(-Infinity, throwing));
+  assertEquals(s, s.padEnd(-9, throwing));
+  assertEquals(s, s.padEnd(-1, throwing));
+  assertEquals(s, s.padEnd(-0, throwing));
+  assertEquals(s, s.padEnd(0, throwing));
+  assertEquals(s, s.padEnd(3, throwing));
+  assertThrows(_ => s.padEnd(9, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**31-1, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**31, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**32-1, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**32, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**53-1, throwing), MyError);
+  assertThrows(_ => s.padEnd(2**53, throwing), MyError);
+  assertThrows(_ => s.padEnd(Infinity, throwing), MyError);
+
+  for (const empty of empties) {
+    assertThrows(_ => s.padStart(Symbol(), empty), TypeError);
+    assertEquals(s, s.padStart(NaN, empty));
+    assertEquals(s, s.padStart(-Infinity, empty));
+    assertEquals(s, s.padStart(-9, empty));
+    assertEquals(s, s.padStart(-1, empty));
+    assertEquals(s, s.padStart(-0, empty));
+    assertEquals(s, s.padStart(0, empty));
+    assertEquals(s, s.padStart(3, empty));
+    assertEquals(s, s.padStart(9, empty));
+    assertEquals(s, s.padStart(2**31-1, empty));
+    assertEquals(s, s.padStart(2**31, empty));
+    assertEquals(s, s.padStart(2**32-1, empty));
+    assertEquals(s, s.padStart(2**32, empty));
+    assertEquals(s, s.padStart(2**53-1, empty));
+    assertEquals(s, s.padStart(2**53, empty));
+    assertEquals(s, s.padStart(Infinity, empty));
+
+    assertThrows(_ => s.padEnd(Symbol(), empty), TypeError);
+    assertEquals(s, s.padEnd(NaN, empty));
+    assertEquals(s, s.padEnd(-Infinity, empty));
+    assertEquals(s, s.padEnd(-9, empty));
+    assertEquals(s, s.padEnd(-1, empty));
+    assertEquals(s, s.padEnd(-0, empty));
+    assertEquals(s, s.padEnd(0, empty));
+    assertEquals(s, s.padEnd(3, empty));
+    assertEquals(s, s.padEnd(9, empty));
+    assertEquals(s, s.padEnd(2**31-1, empty));
+    assertEquals(s, s.padEnd(2**31, empty));
+    assertEquals(s, s.padEnd(2**32-1, empty));
+    assertEquals(s, s.padEnd(2**32, empty));
+    assertEquals(s, s.padEnd(2**53-1, empty));
+    assertEquals(s, s.padEnd(2**53, empty));
+    assertEquals(s, s.padEnd(Infinity, empty));
+  }
+}