Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(264)

Unified Diff: src/builtins/builtins-regexp.cc

Issue 2808033002: [regexp] Properly handle HeapNumbers in AdvanceStringIndex (Closed)
Patch Set: Rebase Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/builtins/builtins-regexp.h ('k') | test/mjsunit/regress/regress-6209.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/builtins/builtins-regexp.cc
diff --git a/src/builtins/builtins-regexp.cc b/src/builtins/builtins-regexp.cc
index 93cb55899b449f95cecd87cde850d823a1551bbc..a71b2d4bf989f9c91ee6d9acc32d4de02ad6f6f4 100644
--- a/src/builtins/builtins-regexp.cc
+++ b/src/builtins/builtins-regexp.cc
@@ -1322,13 +1322,36 @@ TF_BUILTIN(RegExpPrototypeTest, RegExpBuiltinsAssembler) {
Node* RegExpBuiltinsAssembler::AdvanceStringIndex(Node* const string,
Node* const index,
- Node* const is_unicode) {
+ Node* const is_unicode,
+ bool is_fastpath) {
+ CSA_ASSERT(this, IsHeapNumberMap(LoadReceiverMap(index)));
+ if (is_fastpath) CSA_ASSERT(this, TaggedIsPositiveSmi(index));
+
// Default to last_index + 1.
- Node* const index_plus_one = SmiAdd(index, SmiConstant(1));
+ Node* const index_plus_one = NumberInc(index);
Variable var_result(this, MachineRepresentation::kTagged, index_plus_one);
+ // Advancing the index has some subtle issues involving the distinction
+ // between Smis and HeapNumbers. There's three cases:
+ // * {index} is a Smi, {index_plus_one} is a Smi. The standard case.
+ // * {index} is a Smi, {index_plus_one} overflows into a HeapNumber.
+ // In this case we can return the result early, because
+ // {index_plus_one} > {string}.length.
+ // * {index} is a HeapNumber, {index_plus_one} is a HeapNumber. This can only
+ // occur when {index} is outside the Smi range since we normalize
+ // explicitly. Again we can return early.
+ if (is_fastpath) {
+ // Must be in Smi range on the fast path. We control the value of {index}
+ // on all call-sites and can never exceed the length of the string.
+ STATIC_ASSERT(String::kMaxLength + 2 < Smi::kMaxValue);
+ CSA_ASSERT(this, TaggedIsPositiveSmi(index_plus_one));
+ }
+
Label if_isunicode(this), out(this);
- Branch(is_unicode, &if_isunicode, &out);
+ GotoIfNot(is_unicode, &out);
+
+ // Keep this unconditional (even on the fast path) just to be safe.
+ Branch(TaggedIsPositiveSmi(index_plus_one), &if_isunicode, &out);
Bind(&if_isunicode);
{
@@ -1346,7 +1369,7 @@ Node* RegExpBuiltinsAssembler::AdvanceStringIndex(Node* const string,
&out);
// At a surrogate pair, return index + 2.
- Node* const index_plus_two = SmiAdd(index, SmiConstant(2));
+ Node* const index_plus_two = NumberInc(index_plus_one);
var_result.Bind(index_plus_two);
Goto(&out);
@@ -1635,7 +1658,7 @@ void RegExpBuiltinsAssembler::RegExpPrototypeMatchBody(Node* const context,
last_index = CallStub(tolength_callable, context, last_index);
Node* const new_last_index =
- AdvanceStringIndex(string, last_index, is_unicode);
+ AdvanceStringIndex(string, last_index, is_unicode, is_fastpath);
StoreLastIndex(context, regexp, new_last_index, is_fastpath);
@@ -1939,7 +1962,7 @@ void RegExpBuiltinsAssembler::RegExpPrototypeSplitBody(Node* const context,
Node* const is_unicode = FastFlagGetter(regexp, JSRegExp::kUnicode);
Node* const new_next_search_from =
- AdvanceStringIndex(string, next_search_from, is_unicode);
+ AdvanceStringIndex(string, next_search_from, is_unicode, true);
var_next_search_from.Bind(new_next_search_from);
Goto(&loop);
« no previous file with comments | « src/builtins/builtins-regexp.h ('k') | test/mjsunit/regress/regress-6209.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698