[regexp] Fix spec ordering issue in @@split

This fixes a spec bug in which the order of calls to 1) the flag getter
and 2) ToUint32(limit) was incorrect if ToUint32 pushes the regexp
instance onto the slow path. We are now more restrictive and completely
avoid ToUint32 on the fast path.

Bug: chromium:801171
Change-Id: I21d15fe566754d2bc05853f895636bb882fbf599
Reviewed-on: https://chromium-review.googlesource.com/863644Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50533}

src/builtins/builtins-regexp-gen.cc

@@ -2543,7 +2543,7 @@ TF_BUILTIN(RegExpSplit, RegExpBuiltinsAssembler) {
   // to verify the constructor property and jump to the slow path if it has
   // been changed.
 
-  // Convert {maybe_limit} to a uint32, capping at the maximal smi value.
+  // Verify {maybe_limit}.
 
   VARIABLE(var_limit, MachineRepresentation::kTagged, maybe_limit);
   Label if_limitissmimax(this), runtime(this, Label::kDeferred);
@@ -2552,21 +2552,12 @@ TF_BUILTIN(RegExpSplit, RegExpBuiltinsAssembler) {
     Label next(this);
 
     GotoIf(IsUndefined(maybe_limit), &if_limitissmimax);
-    GotoIf(TaggedIsPositiveSmi(maybe_limit), &next);
+    Branch(TaggedIsPositiveSmi(maybe_limit), &next, &runtime);
 
-    var_limit.Bind(ToUint32(context, maybe_limit));
-    {
-      // ToUint32(limit) could potentially change the shape of the RegExp
-      // object. Recheck that we are still on the fast path and bail to runtime
-      // otherwise.
-      {
-        Label next(this);
-        BranchIfFastRegExp(context, regexp, &next, &runtime);
-        BIND(&next);
-      }
-
-      Branch(TaggedIsPositiveSmi(var_limit.value()), &next, &if_limitissmimax);
-    }
+    // We need to be extra-strict and require the given limit to be either
+    // undefined or a positive smi. We can't call ToUint32(maybe_limit) since
+    // that might move us onto the slow path, resulting in ordering spec
+    // violations (see https://crbug.com/801171).
 
     BIND(&if_limitissmimax);
     {
@@ -2590,13 +2581,8 @@ TF_BUILTIN(RegExpSplit, RegExpBuiltinsAssembler) {
   RegExpPrototypeSplitBody(context, regexp, string, var_limit.value());
 
   BIND(&runtime);
-  {
-    // The runtime call passes in limit to ensure the second ToUint32(limit)
-    // call is not observable.
-    CSA_ASSERT(this, IsNumber(var_limit.value()));
-    Return(CallRuntime(Runtime::kRegExpSplit, context, regexp, string,
-                       var_limit.value()));
-  }
+  Return(CallRuntime(Runtime::kRegExpSplit, context, regexp, string,
+                     var_limit.value()));
 }
 
 // ES#sec-regexp.prototype-@@split

test/mjsunit/regress/regress-801171.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.
+
+let called_custom_unicode_getter = false;
+const re = /./;
+
+function f() {
+  re.__defineGetter__("unicode", function() {
+    called_custom_unicode_getter = true;
+  });
+  return 2;
+}
+
+assertEquals(["","",], re[Symbol.split]("abc", { valueOf: f }));
+
+// The spec mandates retrieving the regexp instance's flags before
+// ToUint(limit), i.e. the unicode getter must still be unmodified when
+// flags are retrieved.
+assertFalse(called_custom_unicode_getter);