Commit 4e12baa6 authored by peterwmwong's avatar peterwmwong Committed by Commit Bot

Reland "[builtins] Fix Array.p.join length overflow and invalid string length handling"

This is a reland of ec969ea3

Temporarily removes high memory usage test.

Original change's description:
> [builtins] Fix Array.p.join length overflow and invalid string length handling
>
> - Fixes and simplify allocating the temporary fixed array for ToString-ed elements.
>   - When the array size is greater than representable by an intptr, it overflowed into a negative value causing a non-negative assert to fail.
>   - Simplify fallback behavior by always allocating a conservatively sized temporary fixed array. Previously, if the array had dictionary elements, the temporary fixed array was sized based on %GetNumberDictionaryNumberOfElements() and then resized when entering the fallback.
>
> - Fixes related invalid string length handling. When the running total of the resulting string length overflowed or exceeded String::kMaxLength, a RangeError is thrown. Previously, this thrown RangeError bypassed JoinStackPop and left the receiver on the stack.
>
> Bug: chromium:897404
> Change-Id: I157b71ef04ab06125a5b1c3454e5ed3713bdb591
> Reviewed-on: https://chromium-review.googlesource.com/c/1293070
> Commit-Queue: Peter Wong <peter.wm.wong@gmail.com>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56907}

Bug: chromium:897404
Change-Id: I4995893f6f9724b26c231d05619ad65dbccc7223
Reviewed-on: https://chromium-review.googlesource.com/c/1297675Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Commit-Queue: Peter Wong <peter.wm.wong@gmail.com>
Cr-Commit-Position: refs/heads/master@{#56946}
parent ce00ea80
...@@ -10,15 +10,9 @@ module array { ...@@ -10,15 +10,9 @@ module array {
extern macro CallJSArrayArrayJoinConcatToSequentialString( extern macro CallJSArrayArrayJoinConcatToSequentialString(
FixedArray, intptr, String, String): String; FixedArray, intptr, String, String): String;
extern macro CallLoadJoinElement(implicit context: Context)( extern macro CallArrayJoin(
LoadJoinElementFn, JSReceiver, Number): Object Context, constexpr bool, JSReceiver, String, Number, Object, Object):
labels IfException(Object); String
extern macro CallConvertToLocaleString(implicit context: Context)(
Object, Object, Object): String
labels IfException(Object);
extern macro CallToString(implicit context: Context)(Object): String
labels IfException(Object); labels IfException(Object);
builtin LoadJoinElement<T: type>( builtin LoadJoinElement<T: type>(
...@@ -148,11 +142,11 @@ module array { ...@@ -148,11 +142,11 @@ module array {
isOneByte: bool; isOneByte: bool;
} }
macro BufferInit(estimatedNonHoleyElements: uintptr, sep: String): Buffer { macro BufferInit(len: uintptr, sep: String): Buffer {
const bufferSize: intptr = Signed(estimatedNonHoleyElements + 1); const cappedBufferSize: intptr = len > kFixedArrayMaxLength ?
const cappedBufferSize: intptr = bufferSize > kFixedArrayMaxLength ?
FromConstexpr<intptr>(kFixedArrayMaxLength) : FromConstexpr<intptr>(kFixedArrayMaxLength) :
bufferSize; Signed(len);
assert(cappedBufferSize > 0);
const fixedArray: FixedArray = AllocateZeroedFixedArray(cappedBufferSize); const fixedArray: FixedArray = AllocateZeroedFixedArray(cappedBufferSize);
const isOneByte: bool = HasOnlyOneByteChars(sep.instanceType); const isOneByte: bool = HasOnlyOneByteChars(sep.instanceType);
return Buffer{fixedArray, 0, 0, isOneByte}; return Buffer{fixedArray, 0, 0, isOneByte};
...@@ -220,76 +214,62 @@ module array { ...@@ -220,76 +214,62 @@ module array {
macro ArrayJoinImpl( macro ArrayJoinImpl(
context: Context, receiver: JSReceiver, sep: String, lengthNumber: Number, context: Context, receiver: JSReceiver, sep: String, lengthNumber: Number,
useToLocaleString: constexpr bool, locales: Object, options: Object, useToLocaleString: constexpr bool, locales: Object, options: Object,
estimatedNonHoleyElements: uintptr, initialLoadJoinElement: LoadJoinElementFn): String {
initialLoadJoinElement: LoadJoinElementFn): String
labels IfException(Object) {
const initialMap: Map = receiver.map; const initialMap: Map = receiver.map;
const len: uintptr = Convert<uintptr>(lengthNumber); const len: uintptr = Convert<uintptr>(lengthNumber);
const separatorLength: intptr = sep.length; const separatorLength: intptr = sep.length;
let nofSeparators: intptr = 0; let nofSeparators: intptr = 0;
let loadJoinElements: LoadJoinElementFn = initialLoadJoinElement; let loadJoinElements: LoadJoinElementFn = initialLoadJoinElement;
let buffer: Buffer = BufferInit(estimatedNonHoleyElements, sep); let buffer: Buffer = BufferInit(len, sep);
if (estimatedNonHoleyElements != 0) { // 6. Let k be 0.
// 6. Let k be 0. let k: uintptr = 0;
let k: uintptr = 0;
// 7. Repeat, while k < len
// 7. Repeat, while k < len while (k < len) {
while (k < len) { if (k > 0) {
if (k > 0) { // a. If k > 0, let R be the string-concatenation of R and sep.
// a. If k > 0, let R be the string-concatenation of R and sep. nofSeparators = nofSeparators + 1;
nofSeparators = nofSeparators + 1;
// Verify the current LoadJoinElement specialization can safely be
// Verify the current LoadJoinElement specialization can safely be // used. Otherwise, fall back to generic element access (see
// used. Otherwise, fall back to generic element access (see // LoadJoinElement<GenericElementsAccessor>).
// LoadJoinElement<GenericElementsAccessor>). if (loadJoinElements != LoadJoinElement<GenericElementsAccessor>&&
if (loadJoinElements != LoadJoinElement<GenericElementsAccessor>&& CannotUseSameArrayAccessor(initialMap, lengthNumber, receiver))
CannotUseSameArrayAccessor(initialMap, lengthNumber, receiver)) deferred {
deferred { loadJoinElements = LoadJoinElement<GenericElementsAccessor>;
loadJoinElements = LoadJoinElement<GenericElementsAccessor>; }
}
// Join the current buffer into a single string and add it to a
// new buffer that the fall back will continue with.
const temp: String = BufferJoin(buffer, sep);
buffer = BufferInit((len - k + 1), sep);
buffer = BufferAdd(buffer, temp, 0, separatorLength);
}
}
// b. Let element be ? Get(O, ! ToString(k)). // b. Let element be ? Get(O, ! ToString(k)).
const element: Object = CallLoadJoinElement( const element: Object =
loadJoinElements, receiver, Convert<Number>(k++)) loadJoinElements(context, receiver, Convert<Number>(k++));
otherwise IfException;
// c. If element is undefined or null, let next be the empty String;
// c. If element is undefined or null, let next be the empty String; // otherwise, let next be ? ToString(element).
// otherwise, let next be ? ToString(element). let next: String;
let next: String; if constexpr (useToLocaleString) {
if constexpr (useToLocaleString) { next = ConvertToLocaleString(context, element, locales, options);
next = CallConvertToLocaleString(element, locales, options) if (next == kEmptyString) continue;
otherwise IfException; } else {
if (next == kEmptyString) continue; typeswitch (element) {
} else { case (str: String): {
typeswitch (element) { if (str == kEmptyString) continue;
case (str: String): { next = str;
if (str == kEmptyString) continue; }
next = str; case (num: Number): {
} next = NumberToString(num);
case (num: Number): { }
next = NumberToString(num); case (obj: HeapObject): {
} if (IsNullOrUndefined(obj)) continue;
case (obj: HeapObject): { next = ToString(context, obj);
if (IsNullOrUndefined(obj)) continue;
next = CallToString(obj) otherwise IfException;
}
} }
} }
// d. Set R to the string-concatenation of R and next.
buffer = BufferAdd(buffer, next, nofSeparators, separatorLength);
nofSeparators = 0;
} }
} else {
nofSeparators = Signed(len - 1); // d. Set R to the string-concatenation of R and next.
buffer = BufferAdd(buffer, next, nofSeparators, separatorLength);
nofSeparators = 0;
} }
// Add any separators at the end. // Add any separators at the end.
...@@ -301,16 +281,9 @@ module array { ...@@ -301,16 +281,9 @@ module array {
macro ArrayJoin(implicit context: Context)( macro ArrayJoin(implicit context: Context)(
useToLocaleString: constexpr bool, receiver: JSReceiver, sep: String, useToLocaleString: constexpr bool, receiver: JSReceiver, sep: String,
lenNumber: Number, locales: Object, options: Object): Object lenNumber: Number, locales: Object, options: Object): Object {
labels IfException(Object) {
const map: Map = receiver.map; const map: Map = receiver.map;
const kind: ElementsKind = map.elements_kind; const kind: ElementsKind = map.elements_kind;
const len: uintptr = Convert<uintptr>(lenNumber);
// Estimated number of elements that are not holes. This is conservatively
// defaulted to `len`. When the receiver has dictionary elements, a better
// estimate can be determined through GetNumberDictionaryNumberOfElements.
let estimatedNonHoleyElements: uintptr = len;
let loadJoinElements: LoadJoinElementFn; let loadJoinElements: LoadJoinElementFn;
try { try {
...@@ -323,14 +296,26 @@ module array { ...@@ -323,14 +296,26 @@ module array {
loadJoinElements = LoadJoinElement<FastSmiOrObjectElements>; loadJoinElements = LoadJoinElement<FastSmiOrObjectElements>;
} else if (IsElementsKindLessThanOrEqual(kind, HOLEY_DOUBLE_ELEMENTS)) { } else if (IsElementsKindLessThanOrEqual(kind, HOLEY_DOUBLE_ELEMENTS)) {
loadJoinElements = LoadJoinElement<FastDoubleElements>; loadJoinElements = LoadJoinElement<FastDoubleElements>;
} else if (kind == DICTIONARY_ELEMENTS) { } else if (kind == DICTIONARY_ELEMENTS)
const dict: NumberDictionary = deferred {
UnsafeCast<NumberDictionary>(array.elements); const dict: NumberDictionary =
estimatedNonHoleyElements = UnsafeCast<NumberDictionary>(array.elements);
Unsigned(Convert<intptr>(GetNumberDictionaryNumberOfElements(dict))) const nofElements: Smi = GetNumberDictionaryNumberOfElements(dict);
<< 1; if (nofElements == 0) {
loadJoinElements = LoadJoinElement<DictionaryElements>; if (sep == kEmptyString) return kEmptyString;
} else { try {
const nofSeparators: Smi =
Cast<Smi>(lenNumber - 1) otherwise IfNotSmi;
return StringRepeat(context, sep, nofSeparators);
}
label IfNotSmi {
ThrowRangeError(context, kInvalidStringLength);
}
} else {
loadJoinElements = LoadJoinElement<DictionaryElements>;
}
}
else {
goto IfSlowPath; goto IfSlowPath;
} }
} }
...@@ -339,8 +324,19 @@ module array { ...@@ -339,8 +324,19 @@ module array {
} }
return ArrayJoinImpl( return ArrayJoinImpl(
context, receiver, sep, lenNumber, useToLocaleString, locales, options, context, receiver, sep, lenNumber, useToLocaleString, locales, options,
estimatedNonHoleyElements, loadJoinElements) loadJoinElements);
otherwise IfException; }
builtin ArrayJoinWithToLocaleString(
context: Context, receiver: JSReceiver, sep: String, len: Number,
locales: Object, options: Object): Object {
return ArrayJoin(true, receiver, sep, len, locales, options);
}
builtin ArrayJoinWithoutToLocaleString(
context: Context, receiver: JSReceiver, sep: String, len: Number,
locales: Object, options: Object): Object {
return ArrayJoin(false, receiver, sep, len, locales, options);
} }
// The Join Stack detects cyclical calls to Array Join builtins // The Join Stack detects cyclical calls to Array Join builtins
...@@ -472,8 +468,8 @@ module array { ...@@ -472,8 +468,8 @@ module array {
JoinStackPushInline(o) otherwise IfReturnEmpty; JoinStackPushInline(o) otherwise IfReturnEmpty;
const result: Object = const result: Object = CallArrayJoin(
ArrayJoin(useToLocaleString, o, sep, len, locales, options) context, useToLocaleString, o, sep, len, locales, options)
otherwise IfException; otherwise IfException;
JoinStackPopInline(o); JoinStackPopInline(o);
......
...@@ -1183,3 +1183,4 @@ extern macro TryIntPtrAdd(intptr, intptr): intptr ...@@ -1183,3 +1183,4 @@ extern macro TryIntPtrAdd(intptr, intptr): intptr
labels IfOverflow; labels IfOverflow;
extern builtin ObjectToString(Context, Object): Object; extern builtin ObjectToString(Context, Object): Object;
extern builtin StringRepeat(Context, String, Number): String;
...@@ -91,43 +91,16 @@ class ArrayBuiltinsAssembler : public BaseBuiltinsFromDSLAssembler { ...@@ -91,43 +91,16 @@ class ArrayBuiltinsAssembler : public BaseBuiltinsFromDSLAssembler {
// Temporary Torque support for Array.prototype.join(). // Temporary Torque support for Array.prototype.join().
// TODO(pwong): Remove this when Torque supports exception handlers. // TODO(pwong): Remove this when Torque supports exception handlers.
TNode<Object> CallLoadJoinElement(TNode<Context> context, TNode<String> CallArrayJoin(TNode<Context> context, bool use_to_locale_string,
TNode<Code> loadJoinElement, TNode<JSReceiver> receiver, TNode<String> sep,
TNode<JSReceiver> receiver, TNode<Number> k, TNode<Number> len, TNode<Object> locales,
Label* if_exception, TNode<Object> options, Label* if_exception,
TVariable<Object>* var_exception) { TVariable<Object>* var_exception) {
// Calling a specialization of LoadJoinElement (see array-join.tq), requires Builtins::Name builtin = use_to_locale_string
// a descriptor. We arbitrarily use one of specialization's descriptor, as ? Builtins::kArrayJoinWithToLocaleString
// all specializations share the same interface. : Builtins::kArrayJoinWithoutToLocaleString;
TNode<Object> result = CallStub( TNode<Object> result =
Builtins::CallableFor(isolate(), CallBuiltin(builtin, context, receiver, sep, len, locales, options);
Builtins::kLoadJoinElement20ATDictionaryElements)
.descriptor(),
loadJoinElement, context, receiver, k);
GotoIfException(result, if_exception, var_exception);
return result;
}
// Temporary Torque support for Array.prototype.join().
// TODO(pwong): Remove this when Torque supports exception handlers.
TNode<String> CallConvertToLocaleString(TNode<Context> context,
TNode<Object> element,
TNode<Object> locales,
TNode<Object> options,
Label* if_exception,
TVariable<Object>* var_exception) {
TNode<Object> result = CallBuiltin(Builtins::kConvertToLocaleString,
context, element, locales, options);
GotoIfException(result, if_exception, var_exception);
return CAST(result);
}
// Temporary Torque support for Array.prototype.join().
// TODO(pwong): Remove this when Torque supports exception handlers.
TNode<String> CallToString(TNode<Context> context, TNode<Object> obj,
Label* if_exception,
TVariable<Object>* var_exception) {
TNode<Object> result = CallBuiltin(Builtins::kToString, context, obj);
GotoIfException(result, if_exception, var_exception); GotoIfException(result, if_exception, var_exception);
return CAST(result); return CAST(result);
} }
......
...@@ -1257,7 +1257,6 @@ TF_BUILTIN(StringRepeat, StringBuiltinsAssembler) { ...@@ -1257,7 +1257,6 @@ TF_BUILTIN(StringRepeat, StringBuiltinsAssembler) {
CSA_ASSERT(this, IsString(string)); CSA_ASSERT(this, IsString(string));
CSA_ASSERT(this, Word32BinaryNot(IsEmptyString(string))); CSA_ASSERT(this, Word32BinaryNot(IsEmptyString(string)));
CSA_ASSERT(this, TaggedIsPositiveSmi(count)); CSA_ASSERT(this, TaggedIsPositiveSmi(count));
CSA_ASSERT(this, SmiLessThanOrEqual(count, SmiConstant(String::kMaxLength)));
// The string is repeated with the following algorithm: // The string is repeated with the following algorithm:
// let n = count; // let n = count;
......
...@@ -254,7 +254,11 @@ void CSAGenerator::EmitInstruction( ...@@ -254,7 +254,11 @@ void CSAGenerator::EmitInstruction(
out_ << ", &" << var_names[i][j]; out_ << ", &" << var_names[i][j];
} }
} }
out_ << ");\n"; if (return_type->IsStructType()) {
out_ << ").Flatten();\n";
} else {
out_ << ");\n";
}
if (instruction.return_continuation) { if (instruction.return_continuation) {
out_ << " Goto(&" << BlockName(*instruction.return_continuation); out_ << " Goto(&" << BlockName(*instruction.return_continuation);
for (const std::string& value : *stack) { for (const std::string& value : *stack) {
......
// 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: --allow-natives-syntax
(function DictionaryStringRepeatFastPath() {
const a = new Array(%StringMaxLength());
assertTrue(%HasDictionaryElements(a));
const sep = '12';
assertThrows(() => a.join(sep), RangeError);
// Verifies cycle detection still works properly after thrown error.
assertThrows(() => a.join(sep), RangeError);
a.length = 3;
a[0] = 'a';
a[1] = 'b';
a[2] = 'c';
assertSame('a,b,c', a.join());
})();
(function SeparatorOverflow() {
const a = ['a',,,,,'b'];
const sep = ','.repeat(%StringMaxLength());
assertThrows(() => a.join(sep), RangeError);
// Verifies cycle detection still works properly after thrown error.
assertThrows(() => a.join(sep), RangeError);
assertSame('a,,,,,b', a.join());
})();
(function ElementOverflow() {
const el = ','.repeat(%StringMaxLength());
const a = [el, el, el, el, el];
assertThrows(() => a.join(), RangeError);
// Verifies cycle detection still works properly after thrown error.
assertThrows(() => a.join(), RangeError);
a[0] = 'a';
a[1] = 'b';
a[2] = 'c';
a[3] = 'd';
a[4] = 'e';
assertSame('a,b,c,d,e', a.join());
})();
(function ElementSeparatorOverflow() {
const el = ','.repeat(%StringMaxLength());
const a = [el, el, el, el];
assertThrows(() => a.join(el), RangeError);
// Verifies cycle detection still works properly after thrown error.
assertThrows(() => a.join(el), RangeError);
a[0] = 'a';
a[1] = 'b';
a[2] = 'c';
a[3] = 'd';
assertSame('a,b,c,d', a.join());
})();
...@@ -570,6 +570,10 @@ test(function() { ...@@ -570,6 +570,10 @@ test(function() {
"a".repeat(1 << 30); "a".repeat(1 << 30);
}, "Invalid string length", RangeError); }, "Invalid string length", RangeError);
test(function() {
new Array(1 << 30).join();
}, "Invalid string length", RangeError);
// kNormalizationForm // kNormalizationForm
test(function() { test(function() {
"".normalize("ABC"); "".normalize("ABC");
......
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