Commit 0d582ada authored by Peter Marshall's avatar Peter Marshall Committed by Commit Bot

[builtins] Fix overly strict CHECK in TypedArray.Set.

The existing CHECK assumed that the source and destination could not
have the same buffer, but they actually can as long as the data
ranges do not overlap within the buffer. Change the check to look for
this more relaxed condition instead.

Moved the check outside of the memcpy case as well, given that it
should also apply for the slower, element-by-element copy as well.

Also use JSTypedArray::element_size() to get the element size instead
of the helper on the FixedTypedArrayBase. This lets us change that
helper back to private again.

Bug: chromium:717022

Change-Id: I2eca1df1e87444c5db397e0b7cf686cefe67d29c
Reviewed-on: https://chromium-review.googlesource.com/493147
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45035}
parent 74aa7ff3
......@@ -3140,25 +3140,27 @@ class TypedElementsAccessor
destination_elements->map()->instance_type();
bool same_type = source_type == destination_type;
bool same_size = FixedTypedArrayBase::ElementSize(source_type) ==
FixedTypedArrayBase::ElementSize(destination_type);
bool same_size = source->element_size() == destination->element_size();
bool both_are_simple = HasSimpleRepresentation(source_type) &&
HasSimpleRepresentation(destination_type);
// We assume the source and destination don't overlap, even though they
// can share the same buffer. This is always true for newly allocated
// TypedArrays.
uint8_t* source_data = static_cast<uint8_t*>(source_elements->DataPtr());
uint8_t* dest_data = static_cast<uint8_t*>(destination_elements->DataPtr());
size_t source_byte_length = NumberToSize(source->byte_offset());
size_t dest_byte_length = NumberToSize(destination->byte_offset());
CHECK(dest_data + dest_byte_length <= source_data ||
source_data + source_byte_length <= dest_data);
// We can simply copy the backing store if the types are the same, or if
// we are converting e.g. Uint8 <-> Int8, as the binary representation
// will be the same. This is not the case for floats or clamped Uint8,
// which have special conversion operations.
if (same_type || (same_size && both_are_simple)) {
// We assume the source and destination don't overlap. This is always true
// for newly allocated TypedArrays.
CHECK_NE(source->buffer(), destination->buffer());
int element_size = FixedTypedArrayBase::ElementSize(source_type);
uint8_t* source_data = static_cast<uint8_t*>(source_elements->DataPtr());
uint8_t* destination_data =
static_cast<uint8_t*>(destination_elements->DataPtr());
std::memcpy(destination_data, source_data, length * element_size);
size_t element_size = source->element_size();
std::memcpy(dest_data, source_data, length * element_size);
} else {
// We use scalar accessors below to avoid boxing/unboxing, so there are
// no allocations.
......
......@@ -3480,7 +3480,6 @@ class FixedTypedArrayBase: public FixedArrayBase {
static inline int TypedArraySize(InstanceType type, int length);
inline int TypedArraySize(InstanceType type);
static inline int ElementSize(InstanceType type);
// Use with care: returns raw pointer into heap.
inline void* DataPtr();
......@@ -3488,6 +3487,8 @@ class FixedTypedArrayBase: public FixedArrayBase {
inline int DataSize();
private:
static inline int ElementSize(InstanceType type);
inline int DataSize(InstanceType type);
DISALLOW_IMPLICIT_CONSTRUCTORS(FixedTypedArrayBase);
......
......@@ -589,6 +589,21 @@ function TestTypedArraySet() {
assertThrows(function() { a.set(0, 1); }, TypeError);
assertEquals(1, a.set.length);
// Shared buffer that does not overlap.
var buf = new ArrayBuffer(32);
var a101 = new Int8Array(buf, 0, 16);
var b101 = new Uint8Array(buf, 16);
b101[0] = 42;
a101.set(b101);
assertArrayPrefix([42], a101);
buf = new ArrayBuffer(32);
var a101 = new Int8Array(buf, 0, 16);
var b101 = new Uint8Array(buf, 16);
a101[0] = 42;
b101.set(a101);
assertArrayPrefix([42], b101);
}
TestTypedArraySet();
......
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