Skip to content

Commit

Permalink
[regexp] Properly handle HeapNumbers in AdvanceStringIndex
Browse files Browse the repository at this point in the history
This fixes behavior for HeapNumber {index} arguments passed to
AdvanceStringIndex.

Previously, we'd blindly treat {index} as a Smi. Passing a HeapNumber instead
would result in a Smi addition on the tagged HeapNumber pointer.

Backmerge of commit ed5496f.

BUG=chromium:709015
NOPRESUBMIT=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2808033002
Cr-Commit-Position: refs/branch-heads/5.8@{crosswalk-project#55}
Cr-Branched-From: eda659c-refs/heads/5.8.283@{crosswalk-project#1}
Cr-Branched-From: 4310cd0-refs/heads/master@{#43429}
  • Loading branch information
schuay authored and Commit bot committed Apr 10, 2017
1 parent b88c108 commit bd8da83
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 7 deletions.
35 changes: 29 additions & 6 deletions src/builtins/builtins-regexp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
{
Expand All @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/builtins/builtins-regexp.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class RegExpBuiltinsAssembler : public CodeStubAssembler {
Node* RegExpExec(Node* context, Node* regexp, Node* string);

Node* AdvanceStringIndex(Node* const string, Node* const index,
Node* const is_unicode);
Node* const is_unicode, bool is_fastpath);

void RegExpPrototypeMatchBody(Node* const context, Node* const regexp,
Node* const string, const bool is_fastpath);
Expand Down
25 changes: 25 additions & 0 deletions test/mjsunit/regress/regress-6209.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2017 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: --allow-natives-syntax

function testAdvanceStringIndex(lastIndex, expectedLastIndex) {
let exec_count = 0;
let last_last_index = -1;

let fake_re = {
exec: () => { return (exec_count++ == 0) ? [""] : null },
get lastIndex() { return lastIndex; },
set lastIndex(value) { last_last_index = value },
get global() { return true; },
get flags() { return "g"; }
};

assertEquals([""], RegExp.prototype[Symbol.match].call(fake_re, "abc"));
assertEquals(expectedLastIndex, last_last_index);
}

testAdvanceStringIndex(new Number(42), 43); // Value wrapper.
testAdvanceStringIndex(%AllocateHeapNumber(), 1); // HeapNumber.
testAdvanceStringIndex(4294967295, 4294967296); // HeapNumber.

0 comments on commit bd8da83

Please sign in to comment.