Skip to content

V
V8
Commit 4530f0dc authored by littledan's avatar littledan Committed by Commit bot
Browse files
Options

Revert of [typedarrays] move %TypedArray%.prototype.copyWithin to C++... 

Revert of [typedarrays] move %TypedArray%.prototype.copyWithin to C++ (patchset #6 id:100001 of https://codereview.chromium.org/2671233002/ )

Reason for revert:
Due to security issue described in review thread.

Original issue's description:
> [typedarrays] move %TypedArray%.prototype.copyWithin to C++
>
> - Removes shared InnerArrayCopyWithin JS builtin from src/js/array.js
> - Implements %TypedArray%.prototype.copyWithin as a C++ builtin, which
> relies on std::memmove rather than accessing individual eleements.
> - Fixes the case where copyWithin is invoked on a TypedArray with a
> detached buffer.
> - Add tests to ensure that +/-Infinity (for all 3 parameters) is handled correctly by the
> algorithm
>
> The C++ version gets through the benchmark more than 25000 times as
> quickly as the JS implementation.
>
> BUG=v8:5925, v8:5929, v8:4648
> R=cbruni@chromium.org, adamk@chromium.org, littledan@chromium.org
>
> Review-Url: https://codereview.chromium.org/2671233002
> Cr-Commit-Position: refs/heads/master@{#42975}
> Committed: https://chromium.googlesource.com/v8/v8/+/0f1c626d556cbf84b0e572635eb803729f88cbb3

TBR=cbruni@chromium.org,adamk@chromium.org,bmeurer@chromium.org,cwhan.tunz@gmail.com,caitp@igalia.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=v8:5925, v8:5929, v8:4648

Review-Url: https://codereview.chromium.org/2693753002
Cr-Commit-Position: refs/heads/master@{#43132}
parent 32ed6291
Hide whitespace changes
Inline Side-by-side
Showing with 29 additions and 158 deletions
src/bootstrapper.cc
View file @ 4530f0dc
......@@ -2483,10 +2483,6 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
values->shared()->set_builtin_function_id(kTypedArrayValues);
JSObject::AddProperty(prototype, factory->iterator_symbol(), values,
DONT_ENUM);
// TODO(caitp): alphasort accessors/methods
SimpleInstallFunction(prototype, "copyWithin",
Builtins::kTypedArrayPrototypeCopyWithin, 2, false);
}
{ // -- T y p e d A r r a y s
......
src/builtins/builtins-typedarray.cc
View file @ 4530f0dc
......@@ -167,95 +167,5 @@ void Builtins::Generate_TypedArrayPrototypeKeys(
state, "%TypedArray%.prototype.keys()");
}
namespace {
MaybeHandle<JSTypedArray> ValiadateTypedArray(Isolate* isolate,
Handle<Object> receiver,
const char* method_name) {
if (V8_UNLIKELY(!receiver->IsJSTypedArray())) {
const MessageTemplate::Template message = MessageTemplate::kNotTypedArray;
THROW_NEW_ERROR(isolate, NewTypeError(message), JSTypedArray);
}
Handle<JSTypedArray> array = Handle<JSTypedArray>::cast(receiver);
if (V8_UNLIKELY(array->WasNeutered())) {
const MessageTemplate::Template message = MessageTemplate::kNotTypedArray;
Handle<String> operation =
isolate->factory()->NewStringFromAsciiChecked(method_name);
THROW_NEW_ERROR(isolate, NewTypeError(message, operation), JSTypedArray);
}
return array;
}
int64_t CapRelativeIndex(Handle<Object> num, int64_t minimum, int64_t maximum) {
int64_t relative;
if (V8_LIKELY(num->IsSmi())) {
relative = Smi::cast(*num)->value();
} else {
DCHECK(num->IsHeapNumber());
double fp = HeapNumber::cast(*num)->value();
if (V8_UNLIKELY(!std::isfinite(fp))) {
// +Infinity / -Infinity
DCHECK(!std::isnan(fp));
return fp < 0 ? minimum : maximum;
}
relative = static_cast<int64_t>(fp);
}
return relative < 0 ? std::max<int64_t>(relative + maximum, minimum)
: std::min<int64_t>(relative, maximum);
}
} // namespace
BUILTIN(TypedArrayPrototypeCopyWithin) {
HandleScope scope(isolate);
Handle<JSTypedArray> array;
const char* method = "%TypedArray%.prototype.copyWithin";
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, array, ValiadateTypedArray(isolate, args.receiver(), method));
int64_t len = array->length_value();
int64_t to = 0;
int64_t from = 0;
int64_t final = len;
if (V8_LIKELY(args.length() > 1)) {
Handle<Object> num;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, num, Object::ToInteger(isolate, args.at<Object>(1)));
to = CapRelativeIndex(num, 0, len);
if (args.length() > 2) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, num, Object::ToInteger(isolate, args.at<Object>(2)));
from = CapRelativeIndex(num, 0, len);
Handle<Object> end = args.atOrUndefined(isolate, 3);
if (!end->IsUndefined(isolate)) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, num,
Object::ToInteger(isolate, end));
final = CapRelativeIndex(num, 0, len);
}
}
}
int64_t count = std::min<int64_t>(final - from, len - to);
if (count <= 0) return *array;
Handle<FixedTypedArrayBase> elements(
FixedTypedArrayBase::cast(array->elements()));
size_t element_size = array->element_size();
to = to * element_size;
from = from * element_size;
count = count * element_size;
uint8_t* data = static_cast<uint8_t*>(elements->DataPtr());
std::memmove(data + to, data + from, count);
return *array;
}
} // namespace internal
} // namespace v8
src/builtins/builtins.h
View file @ 4530f0dc
......@@ -803,9 +803,7 @@ class Isolate;
/* ES6 #sec-%typedarray%.prototype.keys */ \
TFJ(TypedArrayPrototypeKeys, 0) \
/* ES6 #sec-%typedarray%.prototype.values */ \
TFJ(TypedArrayPrototypeValues, 0) \
/* ES6 #sec-%typedarray%.prototype.copywithin */ \
CPP(TypedArrayPrototypeCopyWithin)
TFJ(TypedArrayPrototypeValues, 0)
#define IGNORE_BUILTIN(...)
......
src/js/array.js
View file @ 4530f0dc
......@@ -1270,13 +1270,7 @@ function ArrayReduceRight(callback, current) {
}
// ES6 draft 03-17-15, section 22.1.3.3
function ArrayCopyWithin(target, start, end) {
CHECK_OBJECT_COERCIBLE(this, "Array.prototype.copyWithin");
var array = TO_OBJECT(this);
var length = TO_LENGTH(array.length);
function InnerArrayCopyWithin(target, start, end, array, length) {
target = TO_INTEGER(target);
var to;
if (target < 0) {
......@@ -1324,6 +1318,17 @@ function ArrayCopyWithin(target, start, end) {
}
// ES6 draft 03-17-15, section 22.1.3.3
function ArrayCopyWithin(target, start, end) {
CHECK_OBJECT_COERCIBLE(this, "Array.prototype.copyWithin");
var array = TO_OBJECT(this);
var length = TO_LENGTH(array.length);
return InnerArrayCopyWithin(target, start, end, array, length);
}
function InnerArrayFind(predicate, thisArg, array, length) {
if (!IS_CALLABLE(predicate)) {
throw %make_type_error(kCalledNonCallable, predicate);
......@@ -1620,6 +1625,7 @@ utils.Export(function(to) {
to.ArrayPush = ArrayPush;
to.ArrayToString = ArrayToString;
to.ArrayValues = IteratorFunctions.values,
to.InnerArrayCopyWithin = InnerArrayCopyWithin;
to.InnerArrayEvery = InnerArrayEvery;
to.InnerArrayFill = InnerArrayFill;
to.InnerArrayFilter = InnerArrayFilter;
......
src/js/typedarray.js
View file @ 4530f0dc
......@@ -20,6 +20,7 @@ var GlobalArray = global.Array;
var GlobalArrayBuffer = global.ArrayBuffer;
var GlobalArrayBufferPrototype = GlobalArrayBuffer.prototype;
var GlobalObject = global.Object;
var InnerArrayCopyWithin;
var InnerArrayEvery;
var InnerArrayFill;
var InnerArrayFilter;
......@@ -67,6 +68,7 @@ utils.Import(function(from) {
ArrayValues = from.ArrayValues;
GetIterator = from.GetIterator;
GetMethod = from.GetMethod;
InnerArrayCopyWithin = from.InnerArrayCopyWithin;
InnerArrayEvery = from.InnerArrayEvery;
InnerArrayFill = from.InnerArrayFill;
InnerArrayFilter = from.InnerArrayFilter;
......@@ -436,6 +438,17 @@ function TypedArrayGetToStringTag() {
}
function TypedArrayCopyWithin(target, start, end) {
if (!IS_TYPEDARRAY(this)) throw %make_type_error(kNotTypedArray);
var length = %_TypedArrayGetLength(this);
// TODO(littledan): Replace with a memcpy for better performance
return InnerArrayCopyWithin(target, start, end, this, length);
}
%FunctionSetLength(TypedArrayCopyWithin, 2);
// ES6 draft 05-05-15, section 22.2.3.7
function TypedArrayEvery(f, receiver) {
if (!IS_TYPEDARRAY(this)) throw %make_type_error(kNotTypedArray);
......@@ -845,6 +858,7 @@ utils.InstallGetter(GlobalTypedArray.prototype, toStringTagSymbol,
utils.InstallFunctions(GlobalTypedArray.prototype, DONT_ENUM, [
"subarray", TypedArraySubArray,
"set", TypedArraySet,
"copyWithin", TypedArrayCopyWithin,
"every", TypedArrayEvery,
"fill", TypedArrayFill,
"filter", TypedArrayFilter,
......
test/mjsunit/es6/typedarray-copywithin.js
View file @ 4530f0dc
......@@ -171,57 +171,3 @@ CheckEachTypedArray(function copyWithinNullEnd(constructor) {
assertArrayEquals([1, 2, 3, 4, 5],
new constructor([1, 2, 3, 4, 5]).copyWithin(0, 3, null));
});
CheckEachTypedArray(function copyWithinMinusInfinityTarget(constructor) {
var arr = new constructor([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
var expected = [6, 7, 8, 9, 10, 6, 7, 8, 9, 10];
assertArrayEquals(expected, arr.copyWithin(-Infinity, 5));
assertEquals(10, arr.length);
});
CheckEachTypedArray(function copyWithinPositiveInfinityTarget(constructor) {
var arr = new constructor([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
var expected = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
assertArrayEquals(expected, arr.copyWithin(+Infinity, 5));
assertEquals(10, arr.length);
});
CheckEachTypedArray(function copyWithinMinusInfinityStart(constructor) {
var arr = new constructor([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
var expected = [1, 2, 3, 4, 5, 1, 2, 3, 4, 5];
assertArrayEquals(expected, arr.copyWithin(5, -Infinity));
assertEquals(10, arr.length);
});
CheckEachTypedArray(function copyWithinPositiveInfinityStart(constructor) {
var arr = new constructor([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
var expected = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
assertArrayEquals(expected, arr.copyWithin(5, +Infinity));
assertEquals(10, arr.length);
});
CheckEachTypedArray(function copyWithinMinusInfinityEnd(constructor) {
var arr = new constructor([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
var expected = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
assertArrayEquals(expected, arr.copyWithin(5, 0, -Infinity));
assertEquals(10, arr.length);
});
CheckEachTypedArray(function copyWithinPositiveInfinityEnd(constructor) {
var arr = new constructor([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
var expected = [1, 2, 3, 4, 5, 1, 2, 3, 4, 5];
assertArrayEquals(expected, arr.copyWithin(5, 0, +Infinity));
assertEquals(10, arr.length);
});
test/test262/test262.status
View file @ 4530f0dc
......@@ -167,6 +167,7 @@
'built-ins/DataView/prototype/setUint8/detached-buffer-before-outofrange-byteoffset': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=4648
'built-ins/TypedArray/prototype/copyWithin/detached-buffer': [FAIL],
'built-ins/TypedArray/prototype/every/detached-buffer': [FAIL],
'built-ins/TypedArray/prototype/fill/detached-buffer': [FAIL],
'built-ins/TypedArray/prototype/filter/detached-buffer': [FAIL],
......
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