Commit 3f8d6f60 authored by jgruber's avatar jgruber Committed by Commit Bot

[regexp] Properly handle large values in AdvanceStringIndex

There were two separate bugs here. First, a signed/unsigned mismatch
where we took the result of PositiveNumberToUint32 and treated it as a
signed int. Second, AdvanceStringIndex did not handle large input
values correctly.

Both are fixed by using uint64_t consistently.

Bug: chromium:799813, v8:7258
Change-Id: If2819f87986d0ca732bc24df290f6dc7614083e8
Reviewed-on: https://chromium-review.googlesource.com/854272
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50432}
parent b3749e92
...@@ -178,6 +178,21 @@ int64_t NumberToInt64(Object* number) { ...@@ -178,6 +178,21 @@ int64_t NumberToInt64(Object* number) {
return static_cast<int64_t>(d); return static_cast<int64_t>(d);
} }
uint64_t PositiveNumberToUint64(Object* number) {
if (number->IsSmi()) {
int value = Smi::ToInt(number);
if (value <= 0) return 0;
return value;
}
DCHECK(number->IsHeapNumber());
double value = number->Number();
// Catch all values smaller than 1 and use the double-negation trick for NANs.
if (!(value >= 1)) return 0;
uint64_t max = std::numeric_limits<uint64_t>::max();
if (value < max) return static_cast<uint64_t>(value);
return max;
}
bool TryNumberToSize(Object* number, size_t* result) { bool TryNumberToSize(Object* number, size_t* result) {
// Do not create handles in this function! Don't use SealHandleScope because // Do not create handles in this function! Don't use SealHandleScope because
// the function can be used concurrently. // the function can be used concurrently.
......
...@@ -170,6 +170,7 @@ inline uint32_t PositiveNumberToUint32(Object* number); ...@@ -170,6 +170,7 @@ inline uint32_t PositiveNumberToUint32(Object* number);
inline int32_t NumberToInt32(Object* number); inline int32_t NumberToInt32(Object* number);
inline uint32_t NumberToUint32(Object* number); inline uint32_t NumberToUint32(Object* number);
inline int64_t NumberToInt64(Object* number); inline int64_t NumberToInt64(Object* number);
inline uint64_t PositiveNumberToUint64(Object* number);
double StringToDouble(UnicodeCache* unicode_cache, Handle<String> string, double StringToDouble(UnicodeCache* unicode_cache, Handle<String> string,
int flags, double empty_string_val = 0.0); int flags, double empty_string_val = 0.0);
......
...@@ -43,15 +43,15 @@ V8_INLINE bool HasInitialRegExpMap(Isolate* isolate, Handle<JSReceiver> recv) { ...@@ -43,15 +43,15 @@ V8_INLINE bool HasInitialRegExpMap(Isolate* isolate, Handle<JSReceiver> recv) {
MaybeHandle<Object> RegExpUtils::SetLastIndex(Isolate* isolate, MaybeHandle<Object> RegExpUtils::SetLastIndex(Isolate* isolate,
Handle<JSReceiver> recv, Handle<JSReceiver> recv,
int value) { uint64_t value) {
Handle<Object> value_as_object =
isolate->factory()->NewNumberFromInt64(value);
if (HasInitialRegExpMap(isolate, recv)) { if (HasInitialRegExpMap(isolate, recv)) {
JSRegExp::cast(*recv)->set_last_index(Smi::FromInt(value), JSRegExp::cast(*recv)->set_last_index(*value_as_object, SKIP_WRITE_BARRIER);
SKIP_WRITE_BARRIER);
return recv; return recv;
} else { } else {
return Object::SetProperty(recv, isolate->factory()->lastIndex_string(), return Object::SetProperty(recv, isolate->factory()->lastIndex_string(),
handle(Smi::FromInt(value), isolate), value_as_object, LanguageMode::kStrict);
LanguageMode::kStrict);
} }
} }
...@@ -161,12 +161,16 @@ bool RegExpUtils::IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj) { ...@@ -161,12 +161,16 @@ bool RegExpUtils::IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj) {
return last_index->IsSmi() && Smi::ToInt(last_index) >= 0; return last_index->IsSmi() && Smi::ToInt(last_index) >= 0;
} }
int RegExpUtils::AdvanceStringIndex(Isolate* isolate, Handle<String> string, uint64_t RegExpUtils::AdvanceStringIndex(Isolate* isolate,
int index, bool unicode) { Handle<String> string, uint64_t index,
if (unicode && index < string->length()) { bool unicode) {
const uint16_t first = string->Get(index); DCHECK_LE(static_cast<double>(index), kMaxSafeInteger);
if (first >= 0xD800 && first <= 0xDBFF && string->length() > index + 1) { const uint64_t string_length = static_cast<uint64_t>(string->length());
const uint16_t second = string->Get(index + 1); if (unicode && index < string_length) {
const uint16_t first = string->Get(static_cast<uint32_t>(index));
if (first >= 0xD800 && first <= 0xDBFF && index + 1 < string_length) {
DCHECK_LT(index, std::numeric_limits<uint64_t>::max());
const uint16_t second = string->Get(static_cast<uint32_t>(index + 1));
if (second >= 0xDC00 && second <= 0xDFFF) { if (second >= 0xDC00 && second <= 0xDFFF) {
return index + 2; return index + 2;
} }
...@@ -187,8 +191,8 @@ MaybeHandle<Object> RegExpUtils::SetAdvancedStringIndex( ...@@ -187,8 +191,8 @@ MaybeHandle<Object> RegExpUtils::SetAdvancedStringIndex(
ASSIGN_RETURN_ON_EXCEPTION(isolate, last_index_obj, ASSIGN_RETURN_ON_EXCEPTION(isolate, last_index_obj,
Object::ToLength(isolate, last_index_obj), Object); Object::ToLength(isolate, last_index_obj), Object);
const int last_index = PositiveNumberToUint32(*last_index_obj); const uint64_t last_index = PositiveNumberToUint64(*last_index_obj);
const int new_last_index = const uint64_t new_last_index =
AdvanceStringIndex(isolate, string, last_index, unicode); AdvanceStringIndex(isolate, string, last_index, unicode);
return SetLastIndex(isolate, regexp, new_last_index); return SetLastIndex(isolate, regexp, new_last_index);
......
...@@ -22,7 +22,7 @@ class RegExpUtils : public AllStatic { ...@@ -22,7 +22,7 @@ class RegExpUtils : public AllStatic {
// Last index (RegExp.lastIndex) accessors. // Last index (RegExp.lastIndex) accessors.
static MUST_USE_RESULT MaybeHandle<Object> SetLastIndex( static MUST_USE_RESULT MaybeHandle<Object> SetLastIndex(
Isolate* isolate, Handle<JSReceiver> regexp, int value); Isolate* isolate, Handle<JSReceiver> regexp, uint64_t value);
static MUST_USE_RESULT MaybeHandle<Object> GetLastIndex( static MUST_USE_RESULT MaybeHandle<Object> GetLastIndex(
Isolate* isolate, Handle<JSReceiver> recv); Isolate* isolate, Handle<JSReceiver> recv);
...@@ -41,8 +41,8 @@ class RegExpUtils : public AllStatic { ...@@ -41,8 +41,8 @@ class RegExpUtils : public AllStatic {
// ES#sec-advancestringindex // ES#sec-advancestringindex
// AdvanceStringIndex ( S, index, unicode ) // AdvanceStringIndex ( S, index, unicode )
static int AdvanceStringIndex(Isolate* isolate, Handle<String> string, static uint64_t AdvanceStringIndex(Isolate* isolate, Handle<String> string,
int index, bool unicode); uint64_t index, bool unicode);
static MUST_USE_RESULT MaybeHandle<Object> SetAdvancedStringIndex( static MUST_USE_RESULT MaybeHandle<Object> SetAdvancedStringIndex(
Isolate* isolate, Handle<JSReceiver> regexp, Handle<String> string, Isolate* isolate, Handle<JSReceiver> regexp, Handle<String> string,
bool unicode); bool unicode);
......
...@@ -1660,8 +1660,8 @@ RUNTIME_FUNCTION(Runtime_RegExpSplit) { ...@@ -1660,8 +1660,8 @@ RUNTIME_FUNCTION(Runtime_RegExpSplit) {
factory->undefined_value())); factory->undefined_value()));
if (result->IsNull(isolate)) { if (result->IsNull(isolate)) {
string_index = RegExpUtils::AdvanceStringIndex(isolate, string, string_index = static_cast<uint32_t>(RegExpUtils::AdvanceStringIndex(
string_index, unicode); isolate, string, string_index, unicode));
continue; continue;
} }
...@@ -1675,8 +1675,8 @@ RUNTIME_FUNCTION(Runtime_RegExpSplit) { ...@@ -1675,8 +1675,8 @@ RUNTIME_FUNCTION(Runtime_RegExpSplit) {
const uint32_t end = const uint32_t end =
std::min(PositiveNumberToUint32(*last_index_obj), length); std::min(PositiveNumberToUint32(*last_index_obj), length);
if (end == prev_string_index) { if (end == prev_string_index) {
string_index = RegExpUtils::AdvanceStringIndex(isolate, string, string_index = static_cast<uint32_t>(RegExpUtils::AdvanceStringIndex(
string_index, unicode); isolate, string, string_index, unicode));
continue; continue;
} }
......
// 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.
function testAdvanceLastIndex(initial_last_index_value,
expected_final_last_index_value) {
let exec_call_count = 0;
let last_index_setter_call_count = 0;
let final_last_index_value;
var customRegexp = {
get global() { return true; },
get unicode() { return true; },
get lastIndex() {
return initial_last_index_value;
},
set lastIndex(v) {
last_index_setter_call_count++;
final_last_index_value = v;
},
exec() {
return (exec_call_count++ == 0) ? [""] : null;
}
};
RegExp.prototype[Symbol.replace].call(customRegexp);
assertEquals(2, exec_call_count);
assertEquals(2, last_index_setter_call_count);
assertEquals(expected_final_last_index_value, final_last_index_value);
}
testAdvanceLastIndex(-1, 1);
testAdvanceLastIndex( 0, 1);
testAdvanceLastIndex(2**31 - 2, 2**31 - 1);
testAdvanceLastIndex(2**31 - 1, 2**31 - 0);
testAdvanceLastIndex(2**32 - 3, 2**32 - 2);
testAdvanceLastIndex(2**32 - 2, 2**32 - 1);
testAdvanceLastIndex(2**32 - 1, 2**32 - 0);
testAdvanceLastIndex(2**53 - 2, 2**53 - 1);
testAdvanceLastIndex(2**53 - 1, 2**53 - 0);
testAdvanceLastIndex(2**53 - 0, 2**53 - 0);
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