[builtins] Port String.prototype.substr to Torque

... and refactor String.prototype.substring.

This is done to simplify cleaning up callers of ConvertToRelativeIndex()
in a follow-up CL.

This CL also introduces Smi-overflow friendly helper function
  ClampToIndexRange(index: JSAny, min: uintptr, max: uintptr): uintptr

which can be used in other String builtins as a better alternative to
  NumberMin(NumberMax(value, min), max)
pattern.

Bug: v8:8996, v8:4153
Change-Id: Ie1bb5ab305ebf851c033d109ffe9e6afb9418274
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1872392
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64463}

BUILD.gn

@@ -985,6 +985,7 @@ torque_files = [
   "src/builtins/string-slice.tq",
   "src/builtins/string-startswith.tq",
   "src/builtins/string-substring.tq",
+  "src/builtins/string-substr.tq",
   "src/builtins/torque-internal.tq",
   "src/builtins/typed-array-createtypedarray.tq",
   "src/builtins/typed-array-every.tq",

src/builtins/base.tq

@@ -2034,6 +2034,7 @@ extern operator '~' macro ConstexprWordNot(constexpr uintptr):
 extern operator '==' macro Float64Equal(float64, float64): bool;
 extern operator '!=' macro Float64NotEqual(float64, float64): bool;
 extern operator '>' macro Float64GreaterThan(float64, float64): bool;
+extern operator '>=' macro Float64GreaterThanOrEqual(float64, float64): bool;
 extern operator '<' macro Float64LessThan(float64, float64): bool;
 extern operator '<=' macro Float64LessThanOrEqual(float64, float64): bool;
 
@@ -2190,6 +2191,9 @@ extern operator '.instanceType' macro LoadInstanceType(HeapObject):
     InstanceType;
 
 extern operator '.length_intptr' macro LoadStringLengthAsWord(String): intptr;
+operator '.length_uintptr' macro LoadStringLengthAsUintPtr(s: String): uintptr {
+  return Unsigned(s.length_intptr);
+}
 extern operator '.length_uint32' macro LoadStringLengthAsWord32(String): uint32;
 extern operator '.length_smi' macro LoadStringLengthAsSmi(String): Smi;
 
@@ -3705,6 +3709,44 @@ macro ConvertToRelativeIndex(indexNumber: Number, lengthIntPtr: intptr):
   }
 }
 
+// After converting an index to a signed integer, clamps it to the provided
+// range [0, limit]:
+// return min(max(index, 0), limit)
+@export
+transitioning macro ClampToIndexRange(implicit context: Context)(
+    index: JSAny, limit: uintptr): uintptr {
+  const indexNumber: Number =
+      ToInteger_Inline(context, index, kTruncateMinusZero);
+  return ClampToIndexRange(indexNumber, limit);
+}
+
+// Clamps given signed indexNumber to the provided range [0, limit]:
+// return min(max(index, 0), limit)
+@export
+macro ClampToIndexRange(indexNumber: Number, limit: uintptr): uintptr {
+  typeswitch (indexNumber) {
+    case (indexSmi: Smi): {
+      if (indexSmi < 0) return 0;
+      const index: uintptr = Unsigned(Convert<intptr>(indexSmi));
+      if (index >= limit) return limit;
+      return index;
+    }
+    case (indexHeapNumber: HeapNumber): {
+      const indexDouble: float64 = Convert<float64>(indexHeapNumber);
+      // NaNs must already be handled by ClampToIndexRange() version
+      // above accepting JSAny indices.
+      assert(!Float64IsNaN(indexDouble));
+      if (indexDouble <= 0) return 0;
+
+      const maxIndexDouble: float64 = Convert<float64>(limit);
+      assert(maxIndexDouble <= kMaxSafeInteger);
+      if (indexDouble >= maxIndexDouble) return limit;
+
+      return ChangeFloat64ToUintPtr(indexDouble);
+    }
+  }
+}
+
 extern builtin ObjectToString(Context, JSAny): JSAny;
 extern builtin StringRepeat(Context, String, Number): String;
 

src/builtins/builtins-definitions.h

@@ -894,8 +894,6 @@ namespace internal {
   TFJ(StringPrototypeSearch, 1, kReceiver, kRegexp)                            \
   /* ES6 #sec-string.prototype.split */                                        \
   TFJ(StringPrototypeSplit, SharedFunctionInfo::kDontAdaptArgumentsSentinel)   \
-  /* ES6 #sec-string.prototype.substr */                                       \
-  TFJ(StringPrototypeSubstr, SharedFunctionInfo::kDontAdaptArgumentsSentinel)  \
   TFJ(StringPrototypeTrim, SharedFunctionInfo::kDontAdaptArgumentsSentinel)    \
   TFJ(StringPrototypeTrimEnd, SharedFunctionInfo::kDontAdaptArgumentsSentinel) \
   TFJ(StringPrototypeTrimStart,                                                \

src/builtins/builtins-string-gen.cc

@@ -1796,104 +1796,6 @@ TF_BUILTIN(StringPrototypeSplit, StringBuiltinsAssembler) {
   }
 }
 
-// ES6 #sec-string.prototype.substr
-TF_BUILTIN(StringPrototypeSubstr, StringBuiltinsAssembler) {
-  const int kStartArg = 0;
-  const int kLengthArg = 1;
-
-  TNode<IntPtrT> const argc =
-      ChangeInt32ToIntPtr(Parameter(Descriptor::kJSActualArgumentsCount));
-  CodeStubArguments args(this, argc);
-
-  TNode<Object> receiver = args.GetReceiver();
-  TNode<Object> start = args.GetOptionalArgumentValue(kStartArg);
-  TNode<Object> length = args.GetOptionalArgumentValue(kLengthArg);
-  TNode<Context> context = CAST(Parameter(Descriptor::kContext));
-
-  Label out(this);
-
-  TVARIABLE(IntPtrT, var_start);
-  TVARIABLE(Number, var_length);
-
-  TNode<IntPtrT> const zero = IntPtrConstant(0);
-
-  // Check that {receiver} is coercible to Object and convert it to a String.
-  TNode<String> const string =
-      ToThisString(context, receiver, "String.prototype.substr");
-
-  TNode<IntPtrT> const string_length = LoadStringLengthAsWord(string);
-
-  // Convert {start} to a relative index.
-  var_start = ConvertToRelativeIndex(context, start, string_length);
-
-  // Conversions and bounds-checks for {length}.
-  Label if_issmi(this), if_isheapnumber(this, Label::kDeferred);
-
-  // Default to {string_length} if {length} is undefined.
-  {
-    Label if_isundefined(this, Label::kDeferred), if_isnotundefined(this);
-    Branch(IsUndefined(length), &if_isundefined, &if_isnotundefined);
-
-    BIND(&if_isundefined);
-    var_length = SmiTag(string_length);
-    Goto(&if_issmi);
-
-    BIND(&if_isnotundefined);
-    var_length = ToInteger_Inline(context, length,
-                                  CodeStubAssembler::kTruncateMinusZero);
-  }
-
-  TVARIABLE(IntPtrT, var_result_length);
-
-  Branch(TaggedIsSmi(var_length.value()), &if_issmi, &if_isheapnumber);
-
-  // Set {length} to min(max({length}, 0), {string_length} - {start}
-  BIND(&if_issmi);
-  {
-    TNode<IntPtrT> const positive_length =
-        IntPtrMax(SmiUntag(CAST(var_length.value())), zero);
-    TNode<IntPtrT> const minimal_length =
-        IntPtrSub(string_length, var_start.value());
-    var_result_length = IntPtrMin(positive_length, minimal_length);
-
-    GotoIfNot(IntPtrLessThanOrEqual(var_result_length.value(), zero), &out);
-    args.PopAndReturn(EmptyStringConstant());
-  }
-
-  BIND(&if_isheapnumber);
-  {
-    // If {length} is a heap number, it is definitely out of bounds. There are
-    // two cases according to the spec: if it is negative, "" is returned; if
-    // it is positive, then length is set to {string_length} - {start}.
-
-    CSA_ASSERT(this, IsHeapNumber(CAST(var_length.value())));
-
-    Label if_isnegative(this), if_ispositive(this);
-    TNode<Float64T> const float_zero = Float64Constant(0.);
-    TNode<Float64T> const length_float =
-        LoadHeapNumberValue(CAST(var_length.value()));
-    Branch(Float64LessThan(length_float, float_zero), &if_isnegative,
-           &if_ispositive);
-
-    BIND(&if_isnegative);
-    args.PopAndReturn(EmptyStringConstant());
-
-    BIND(&if_ispositive);
-    {
-      var_result_length = IntPtrSub(string_length, var_start.value());
-      GotoIfNot(IntPtrLessThanOrEqual(var_result_length.value(), zero), &out);
-      args.PopAndReturn(EmptyStringConstant());
-    }
-  }
-
-  BIND(&out);
-  {
-    TNode<IntPtrT> const end =
-        IntPtrAdd(var_start.value(), var_result_length.value());
-    args.PopAndReturn(SubString(string, var_start.value(), end));
-  }
-}
-
 TF_BUILTIN(StringSubstring, StringBuiltinsAssembler) {
   TNode<String> string = CAST(Parameter(Descriptor::kString));
   TNode<IntPtrT> from = UncheckedCast<IntPtrT>(Parameter(Descriptor::kFrom));

src/builtins/builtins-string-gen.h

@@ -37,8 +37,14 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
 
   // Return a new string object which holds a substring containing the range
   // [from,to[ of string.
+  // TODO(v8:9880): Fix implementation to use UintPtrT arguments and drop
+  // IntPtrT version once all callers use UintPtrT version.
   TNode<String> SubString(TNode<String> string, TNode<IntPtrT> from,
                           TNode<IntPtrT> to);
+  TNode<String> SubString(TNode<String> string, TNode<UintPtrT> from,
+                          TNode<UintPtrT> to) {
+    return SubString(string, Signed(from), Signed(to));
+  }
 
   // Copies |character_count| elements from |from_string| to |to_string|
   // starting at the |from_index|'th character. |from_string| and |to_string|

src/builtins/string-substr.tq

@@ -0,0 +1,45 @@
+// Copyright 2019 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.
+
+namespace string_substr {
+
+  extern macro StringBuiltinsAssembler::SubString(String, intptr, intptr):
+      String;
+
+  // String.prototype.substr ( start, length )
+  // ES6 #sec-string.prototype.substr
+  transitioning javascript builtin StringPrototypeSubstr(
+      js-implicit context: Context, receiver: JSAny)(...arguments): String {
+    const methodName: constexpr string = 'String.prototype.substr';
+    // 1. Let O be ? RequireObjectCoercible(this value).
+    // 2. Let S be ? ToString(O).
+    const string: String = ToThisString(receiver, methodName);
+
+    // 5. Let size be the number of code units in S.
+    const size: intptr = string.length_intptr;
+
+    // 3. Let intStart be ? ToInteger(start).
+    // 6. If intStart < 0, set intStart to max(size + intStart, 0).
+    const start = arguments[0];
+    const initStart: intptr =
+        start != Undefined ? ConvertToRelativeIndex(start, size) : 0;
+
+    // 4. If length is undefined,
+    //   let end be +∞; otherwise let end be ? ToInteger(length).
+    // 7. Let resultLength be min(max(end, 0), size - intStart).
+    const length = arguments[1];
+    const lengthLimit = size - initStart;
+    assert(lengthLimit >= 0);
+    const resultLength: intptr = length != Undefined ?
+        Signed(ClampToIndexRange(length, Unsigned(lengthLimit))) :
+        lengthLimit;
+
+    // 8. If resultLength ≤ 0, return the empty String "".
+    if (resultLength == 0) return EmptyStringConstant();
+
+    // 9. Return the String value containing resultLength consecutive code units
+    // from S beginning with the code unit at index intStart.
+    return SubString(string, initStart, initStart + resultLength);
+  }
+}

src/builtins/string-substring.tq

@@ -4,48 +4,30 @@
 
 namespace string_substring {
 
-  extern macro StringBuiltinsAssembler::SubString(String, intptr, intptr):
+  extern macro StringBuiltinsAssembler::SubString(String, uintptr, uintptr):
       String;
 
-  transitioning macro ToSmiBetweenZeroAnd(implicit context: Context)(
-      value: JSAny, limit: Smi): Smi {
-    const valueInt: Number =
-        ToInteger_Inline(context, value, kTruncateMinusZero);
-    typeswitch (valueInt) {
-      case (valueSmi: Smi): {
-        if (SmiAbove(valueSmi, limit)) {
-          return valueSmi < 0 ? 0 : limit;
-        }
-        return valueSmi;
-      }
-      // {value} is a heap number - in this case, it is definitely out of
-      // bounds.
-      case (hn: HeapNumber): {
-        const valueFloat: float64 = LoadHeapNumberValue(hn);
-        return valueFloat < 0. ? 0 : limit;
-      }
-    }
-  }
-
   // ES6 #sec-string.prototype.substring
   transitioning javascript builtin StringPrototypeSubstring(
       js-implicit context: Context, receiver: JSAny)(...arguments): String {
     // Check that {receiver} is coercible to Object and convert it to a String.
     const string: String = ToThisString(receiver, 'String.prototype.substring');
-    const length = string.length_smi;
+    const length: uintptr = string.length_uintptr;
 
     // Conversion and bounds-checks for {start}.
-    let start: Smi = ToSmiBetweenZeroAnd(arguments[0], length);
+    const arg0 = arguments[0];
+    let start: uintptr =
+        arg0 != Undefined ? ClampToIndexRange(arg0, length) : 0;
 
     // Conversion and bounds-checks for {end}.
-    let end: Smi = arguments[1] == Undefined ?
-        length :
-        ToSmiBetweenZeroAnd(arguments[1], length);
+    const arg1 = arguments[1];
+    let end: uintptr =
+        arg1 != Undefined ? ClampToIndexRange(arg1, length) : length;
     if (end < start) {
-      const tmp: Smi = end;
+      const tmp: uintptr = end;
       end = start;
       start = tmp;
     }
-    return SubString(string, SmiUntag(start), SmiUntag(end));
+    return SubString(string, start, end);
   }
 }