Commit dd580e8f authored by Jakob Gruber's avatar Jakob Gruber Committed by Commit Bot

[regexp] Fix sticky callable replace with OOB lastIndex

When given a sticky regexp s.t. lastIndex > subject.length, the
following should happen:

1. exec returns null (= no match)
2. lastIndex is reset to 0.

This is usually done by the RegExp.p.exec builtin; but in some cases
we take different paths and try to re-implement the parts of exec that
we need.

One of these cases was in %StringReplaceNonGlobalRegExpWithFunction.
Here, we set lastIndex to 0 but then incorrectly called into
RegExpImpl::Exec. REI::Exec started matching with lastIndex == 0,
which is just plain wrong. With this CL we now correctly omit the
REI::Exec call and return null.

Bug: chromium:937681, v8:5361
Change-Id: I6bb1114a6b92ed3c6e63ec7f6ec2df4b95a19b4c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1514679Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60169}
parent e7cc2512
......@@ -1283,8 +1283,8 @@ V8_WARN_UNUSED_RESULT MaybeHandle<String> RegExpReplace(
Handle<Object> match_indices_obj(ReadOnlyRoots(isolate).null_value(),
isolate);
// A lastIndex exceeding the string length always always returns null
// (signalling failure) in RegExpBuiltinExec, thus we can skip the call.
// A lastIndex exceeding the string length always returns null (signalling
// failure) in RegExpBuiltinExec, thus we can skip the call.
if (last_index <= static_cast<uint32_t>(string->length())) {
ASSIGN_RETURN_ON_EXCEPTION(isolate, match_indices_obj,
RegExpImpl::Exec(isolate, regexp, string,
......@@ -1302,8 +1302,9 @@ V8_WARN_UNUSED_RESULT MaybeHandle<String> RegExpReplace(
const int start_index = match_indices->Capture(0);
const int end_index = match_indices->Capture(1);
if (sticky)
if (sticky) {
regexp->set_last_index(Smi::FromInt(end_index), SKIP_WRITE_BARRIER);
}
IncrementalStringBuilder builder(isolate);
builder.AppendString(factory->NewSubString(string, 0, start_index));
......@@ -1402,14 +1403,19 @@ RUNTIME_FUNCTION(Runtime_StringReplaceNonGlobalRegExpWithFunction) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, last_index_obj, Object::ToLength(isolate, last_index_obj));
last_index = PositiveNumberToUint32(*last_index_obj);
if (last_index > static_cast<uint32_t>(subject->length())) last_index = 0;
}
Handle<Object> match_indices_obj;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, match_indices_obj,
RegExpImpl::Exec(isolate, regexp, subject, last_index, last_match_info));
Handle<Object> match_indices_obj(ReadOnlyRoots(isolate).null_value(),
isolate);
// A lastIndex exceeding the string length always returns null (signalling
// failure) in RegExpBuiltinExec, thus we can skip the call.
if (last_index <= static_cast<uint32_t>(subject->length())) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, match_indices_obj,
RegExpImpl::Exec(isolate, regexp, subject, last_index,
last_match_info));
}
if (match_indices_obj->IsNull(isolate)) {
if (sticky) regexp->set_last_index(Smi::kZero, SKIP_WRITE_BARRIER);
......@@ -1422,8 +1428,9 @@ RUNTIME_FUNCTION(Runtime_StringReplaceNonGlobalRegExpWithFunction) {
const int index = match_indices->Capture(0);
const int end_of_match = match_indices->Capture(1);
if (sticky)
if (sticky) {
regexp->set_last_index(Smi::FromInt(end_of_match), SKIP_WRITE_BARRIER);
}
IncrementalStringBuilder builder(isolate);
builder.AppendString(factory->NewSubString(subject, 0, index));
......
// 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.
const str = 'aaaa';
const re0 = /./y;
// Twice to go through both runtime and the builtin.
re0.lastIndex = 9;
assertEquals(str, re0[Symbol.replace](str, () => 42));
re0.lastIndex = 9;
assertEquals(str, re0[Symbol.replace](str, () => 42));
re0.lastIndex = 9;
assertEquals(str, re0[Symbol.replace](str, "42"));
re0.lastIndex = 9;
assertEquals(str, re0[Symbol.replace](str, "42"));
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment