[regexp] Ensure FastFlagGetter returns either 0 or 1

While not strictly necessary, this is consistent with how
SlowFlagGetter behaves. It adds an additional shift operation (which
we could fold into the smi untagging if needed).

Drive-by: Typify flag accessors.

Bug: chromium:899464
Change-Id: Ib154d626e522ed723e2c19b1ab7f68560ac414bc
Reviewed-on: https://chromium-review.googlesource.com/c/1304315Reviewed-by: Peter Wong <peter.wm.wong@gmail.com>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57063}

src/builtins/builtins-regexp-gen.cc

@@ -1526,73 +1526,66 @@ TF_BUILTIN(RegExpPrototypeSourceGetter, RegExpBuiltinsAssembler) {
 }
 
 // Fast-path implementation for flag checks on an unmodified JSRegExp instance.
-Node* RegExpBuiltinsAssembler::FastFlagGetter(Node* const regexp,
-                                              JSRegExp::Flag flag) {
-  TNode<Smi> const flags =
-      CAST(LoadObjectField(regexp, JSRegExp::kFlagsOffset));
-  TNode<Smi> const mask = SmiConstant(flag);
-  return SmiToInt32(SmiAnd(flags, mask));
+TNode<Int32T> RegExpBuiltinsAssembler::FastFlagGetter(TNode<JSRegExp> regexp,
+                                                      JSRegExp::Flag flag) {
+  TNode<Smi> flags = CAST(LoadObjectField(regexp, JSRegExp::kFlagsOffset));
+  TNode<Smi> mask = SmiConstant(flag);
+  return SmiToInt32(SmiShr(SmiAnd(flags, mask), JSRegExp::FlagShiftBits(flag)));
 }
 
 // Load through the GetProperty stub.
-Node* RegExpBuiltinsAssembler::SlowFlagGetter(Node* const context,
-                                              Node* const regexp,
-                                              JSRegExp::Flag flag) {
-  Factory* factory = isolate()->factory();
-
+TNode<Int32T> RegExpBuiltinsAssembler::SlowFlagGetter(TNode<Context> context,
+                                                      TNode<Object> regexp,
+                                                      JSRegExp::Flag flag) {
   Label out(this);
-  VARIABLE(var_result, MachineRepresentation::kWord32);
+  TVARIABLE(Int32T, var_result);
 
   Handle<String> name;
   switch (flag) {
     case JSRegExp::kGlobal:
-      name = factory->global_string();
+      name = isolate()->factory()->global_string();
       break;
     case JSRegExp::kIgnoreCase:
-      name = factory->ignoreCase_string();
+      name = isolate()->factory()->ignoreCase_string();
       break;
     case JSRegExp::kMultiline:
-      name = factory->multiline_string();
+      name = isolate()->factory()->multiline_string();
       break;
     case JSRegExp::kDotAll:
       UNREACHABLE();  // Never called for dotAll.
       break;
     case JSRegExp::kSticky:
-      name = factory->sticky_string();
+      name = isolate()->factory()->sticky_string();
       break;
     case JSRegExp::kUnicode:
-      name = factory->unicode_string();
+      name = isolate()->factory()->unicode_string();
       break;
     default:
       UNREACHABLE();
   }
 
-  Node* const value = GetProperty(context, regexp, name);
+  TNode<Object> value = GetProperty(context, regexp, name);
 
   Label if_true(this), if_false(this);
   BranchIfToBooleanIsTrue(value, &if_true, &if_false);
 
   BIND(&if_true);
-  {
-    var_result.Bind(Int32Constant(1));
-    Goto(&out);
-  }
+  var_result = Int32Constant(1);
+  Goto(&out);
 
   BIND(&if_false);
-  {
-    var_result.Bind(Int32Constant(0));
-    Goto(&out);
-  }
+  var_result = Int32Constant(0);
+  Goto(&out);
 
   BIND(&out);
   return var_result.value();
 }
 
-Node* RegExpBuiltinsAssembler::FlagGetter(Node* const context,
-                                          Node* const regexp,
-                                          JSRegExp::Flag flag,
-                                          bool is_fastpath) {
-  return is_fastpath ? FastFlagGetter(regexp, flag)
+TNode<Int32T> RegExpBuiltinsAssembler::FlagGetter(TNode<Context> context,
+                                                  TNode<Object> regexp,
+                                                  JSRegExp::Flag flag,
+                                                  bool is_fastpath) {
+  return is_fastpath ? FastFlagGetter(CAST(regexp), flag)
                      : SlowFlagGetter(context, regexp, flag);
 }
 
@@ -1610,7 +1603,7 @@ void RegExpBuiltinsAssembler::FlagGetter(Node* context, Node* receiver,
   BIND(&if_isunmodifiedjsregexp);
   {
     // Refer to JSRegExp's flag property on the fast-path.
-    Node* const is_flag_set = FastFlagGetter(receiver, flag);
+    Node* const is_flag_set = FastFlagGetter(CAST(receiver), flag);
     Return(SelectBooleanConstant(is_flag_set));
   }
 
@@ -1874,7 +1867,7 @@ void RegExpBuiltinsAssembler::RegExpPrototypeMatchBody(Node* const context,
   if (is_fastpath) CSA_ASSERT(this, IsFastRegExp(context, regexp));
 
   Node* const is_global =
-      FlagGetter(context, regexp, JSRegExp::kGlobal, is_fastpath);
+      FlagGetter(CAST(context), CAST(regexp), JSRegExp::kGlobal, is_fastpath);
 
   Label if_isglobal(this), if_isnotglobal(this);
   Branch(is_global, &if_isglobal, &if_isnotglobal);
@@ -1890,8 +1883,8 @@ void RegExpBuiltinsAssembler::RegExpPrototypeMatchBody(Node* const context,
 
   BIND(&if_isglobal);
   {
-    Node* const is_unicode =
-        FlagGetter(context, regexp, JSRegExp::kUnicode, is_fastpath);
+    Node* const is_unicode = FlagGetter(CAST(context), CAST(regexp),
+                                        JSRegExp::kUnicode, is_fastpath);
 
     StoreLastIndex(context, regexp, SmiZero(), is_fastpath);
 
@@ -2054,12 +2047,10 @@ TNode<Object> RegExpBuiltinsAssembler::MatchAllIterator(
     CSA_ASSERT(this, IsFastRegExp(context, var_matcher.value()));
 
     // d. Let global be ? ToBoolean(? Get(matcher, "global")).
-    var_global = UncheckedCast<Int32T>(
-        FastFlagGetter(var_matcher.value(), JSRegExp::kGlobal));
+    var_global = FastFlagGetter(CAST(var_matcher.value()), JSRegExp::kGlobal);
 
     // e. Let fullUnicode be ? ToBoolean(? Get(matcher, "unicode").
-    var_unicode = UncheckedCast<Int32T>(
-        FastFlagGetter(var_matcher.value(), JSRegExp::kUnicode));
+    var_unicode = FastFlagGetter(CAST(var_matcher.value()), JSRegExp::kUnicode);
 
     // f. Let lastIndex be ? ToLength(? Get(R, "lastIndex")).
     // g. Perform ? Set(matcher, "lastIndex", lastIndex, true).
@@ -2357,7 +2348,8 @@ void RegExpBuiltinsAssembler::RegExpPrototypeSplitBody(Node* const context,
                                                        TNode<String> string,
                                                        TNode<Smi> const limit) {
   CSA_ASSERT(this, IsFastRegExp(context, regexp));
-  CSA_ASSERT(this, Word32BinaryNot(FastFlagGetter(regexp, JSRegExp::kSticky)));
+  CSA_ASSERT(this,
+             Word32BinaryNot(FastFlagGetter(CAST(regexp), JSRegExp::kSticky)));
 
   TNode<IntPtrT> const int_limit = SmiUntag(limit);
 
@@ -2479,7 +2471,7 @@ void RegExpBuiltinsAssembler::RegExpPrototypeSplitBody(Node* const context,
       GotoIfNot(SmiEqual(match_to, next_search_from), &next);
       GotoIfNot(SmiEqual(match_to, last_matched_until), &next);
 
-      Node* const is_unicode = FastFlagGetter(regexp, JSRegExp::kUnicode);
+      Node* const is_unicode = FastFlagGetter(CAST(regexp), JSRegExp::kUnicode);
       Node* const new_next_search_from =
           AdvanceStringIndex(string, next_search_from, is_unicode, true);
       var_next_search_from = CAST(new_next_search_from);
@@ -2902,10 +2894,10 @@ Node* RegExpBuiltinsAssembler::ReplaceSimpleStringFastPath(
       if_nofurthermatches(this);
 
   // Is {regexp} global?
-  Node* const is_global = FastFlagGetter(regexp, JSRegExp::kGlobal);
+  Node* const is_global = FastFlagGetter(CAST(regexp), JSRegExp::kGlobal);
   GotoIfNot(is_global, &loop);
 
-  var_is_unicode.Bind(FastFlagGetter(regexp, JSRegExp::kUnicode));
+  var_is_unicode.Bind(FastFlagGetter(CAST(regexp), JSRegExp::kUnicode));
   FastStoreLastIndex(regexp, SmiZero());
   Goto(&loop);
 

src/builtins/builtins-regexp-gen.h

@@ -107,11 +107,12 @@ class RegExpBuiltinsAssembler : public CodeStubAssembler {
 
   Node* FlagsGetter(Node* const context, Node* const regexp, bool is_fastpath);
 
-  Node* FastFlagGetter(Node* const regexp, JSRegExp::Flag flag);
-  Node* SlowFlagGetter(Node* const context, Node* const regexp,
-                       JSRegExp::Flag flag);
-  Node* FlagGetter(Node* const context, Node* const regexp, JSRegExp::Flag flag,
-                   bool is_fastpath);
+  TNode<Int32T> FastFlagGetter(TNode<JSRegExp> regexp, JSRegExp::Flag flag);
+  TNode<Int32T> SlowFlagGetter(TNode<Context> context, TNode<Object> regexp,
+                               JSRegExp::Flag flag);
+  TNode<Int32T> FlagGetter(TNode<Context> context, TNode<Object> regexp,
+                           JSRegExp::Flag flag, bool is_fastpath);
+
   void FlagGetter(Node* context, Node* receiver, JSRegExp::Flag flag,
                   int counter, const char* method_name);
 

src/objects/js-regexp.h

@@ -50,6 +50,32 @@ class JSRegExp : public JSObject {
 
   static constexpr int FlagCount() { return 6; }
 
+  static int FlagShiftBits(Flag flag) {
+    switch (flag) {
+      case kGlobal:
+        STATIC_ASSERT(kGlobal == (1 << 0));
+        return 0;
+      case kIgnoreCase:
+        STATIC_ASSERT(kIgnoreCase == (1 << 1));
+        return 1;
+      case kMultiline:
+        STATIC_ASSERT(kMultiline == (1 << 2));
+        return 2;
+      case kSticky:
+        STATIC_ASSERT(kSticky == (1 << 3));
+        return 3;
+      case kUnicode:
+        STATIC_ASSERT(kUnicode == (1 << 4));
+        return 4;
+      case kDotAll:
+        STATIC_ASSERT(kDotAll == (1 << 5));
+        return 5;
+      default:
+        STATIC_ASSERT(FlagCount() == 6);
+        UNREACHABLE();
+    }
+  }
+
   DECL_ACCESSORS(data, Object)
   DECL_ACCESSORS(flags, Object)
   DECL_ACCESSORS(last_index, Object)

test/mjsunit/regress/regress-crbug-899464.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: --harmony-string-matchall
+
+''.matchAll(/./u);