Skip to content

Commit

Permalink
Merged: Revert "[typedarray] Make JSTypedArray::length authoritative."
Browse files Browse the repository at this point in the history
Revision: 18100666628e46ee00222df6c2be9df811599657

BUG=v8:4153,v8:7881
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
TBR=jarin@chromium.org, bmeurer@chromium.org
R=szuend@chromium.org

Change-Id: I7bca25822b77b2e9eed698ba6717a023a891484b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1594437
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/branch-heads/7.5@{#18}
Cr-Branched-From: 35b9bf5-refs/heads/7.5.288@{#1}
Cr-Branched-From: 912b391-refs/heads/master@{#60911}
  • Loading branch information
psmarshall authored and Commit Bot committed May 3, 2019
1 parent 018ca54 commit 6773e16
Show file tree
Hide file tree
Showing 28 changed files with 114 additions and 138 deletions.
2 changes: 1 addition & 1 deletion src/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7626,7 +7626,7 @@ size_t v8::ArrayBufferView::ByteLength() {

size_t v8::TypedArray::Length() {
i::Handle<i::JSTypedArray> obj = Utils::OpenHandle(this);
return obj->WasDetached() ? 0 : obj->length();
return obj->WasDetached() ? 0 : obj->length_value();
}

static_assert(v8::TypedArray::kMaxLength == i::Smi::kMaxValue,
Expand Down
7 changes: 4 additions & 3 deletions src/builtins/base.tq
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,9 @@ extern class JSArrayBufferView extends JSObject {
}

extern class JSTypedArray extends JSArrayBufferView {
AttachOffHeapBuffer(buffer: JSArrayBuffer, map: Map, byteOffset: uintptr):
void {
AttachOffHeapBuffer(
buffer: JSArrayBuffer, map: Map, length: PositiveSmi,
byteOffset: uintptr): void {
const basePointer: Smi = 0;

// The max byteOffset is 8 * MaxSmi on the particular platform. 32 bit
Expand All @@ -473,7 +474,7 @@ extern class JSTypedArray extends JSArrayBufferView {
this.buffer = buffer;
this.elements = new FixedTypedArrayBase{
map,
length: 0,
length,
base_pointer: basePointer,
external_pointer: externalPointer
};
Expand Down
3 changes: 2 additions & 1 deletion src/builtins/builtins-sharedarraybuffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ V8_WARN_UNUSED_RESULT Maybe<size_t> ValidateAtomicAccess(

size_t access_index;
if (!TryNumberToSize(*access_index_obj, &access_index) ||
typed_array->WasDetached() || access_index >= typed_array->length()) {
typed_array->WasDetached() ||
access_index >= typed_array->length_value()) {
isolate->Throw(*isolate->factory()->NewRangeError(
MessageTemplate::kInvalidAtomicAccessIndex));
return Nothing<size_t>();
Expand Down
10 changes: 5 additions & 5 deletions src/builtins/builtins-typed-array.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ BUILTIN(TypedArrayPrototypeCopyWithin) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, array, JSTypedArray::Validate(isolate, args.receiver(), method));

int64_t len = array->length();
int64_t len = array->length_value();
int64_t to = 0;
int64_t from = 0;
int64_t final = len;
Expand Down Expand Up @@ -124,7 +124,7 @@ BUILTIN(TypedArrayPrototypeFill) {
Object::ToNumber(isolate, obj_value));
}

int64_t len = array->length();
int64_t len = array->length_value();
int64_t start = 0;
int64_t end = len;

Expand Down Expand Up @@ -171,7 +171,7 @@ BUILTIN(TypedArrayPrototypeIncludes) {

if (args.length() < 2) return ReadOnlyRoots(isolate).false_value();

int64_t len = array->length();
int64_t len = array->length_value();
if (len == 0) return ReadOnlyRoots(isolate).false_value();

int64_t index = 0;
Expand Down Expand Up @@ -203,7 +203,7 @@ BUILTIN(TypedArrayPrototypeIndexOf) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, array, JSTypedArray::Validate(isolate, args.receiver(), method));

int64_t len = array->length();
int64_t len = array->length_value();
if (len == 0) return Smi::FromInt(-1);

int64_t index = 0;
Expand Down Expand Up @@ -234,7 +234,7 @@ BUILTIN(TypedArrayPrototypeLastIndexOf) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, array, JSTypedArray::Validate(isolate, args.receiver(), method));

int64_t len = array->length();
int64_t len = array->length_value();
if (len == 0) return Smi::FromInt(-1);

int64_t index = len - 1;
Expand Down
6 changes: 4 additions & 2 deletions src/builtins/typed-array-createtypedarray.tq
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ namespace typed_array_createtypedarray {
label AttachOffHeapBuffer(bufferObj: Object) {
const buffer = Cast<JSArrayBuffer>(bufferObj) otherwise unreachable;
const byteOffset: uintptr = 0;
typedArray.AttachOffHeapBuffer(buffer, elementsInfo.map, byteOffset);
typedArray.AttachOffHeapBuffer(
buffer, elementsInfo.map, length, byteOffset);
}

const byteOffset: uintptr = 0;
Expand Down Expand Up @@ -236,7 +237,8 @@ namespace typed_array_createtypedarray {
}

SetupTypedArray(typedArray, newLength, offset, newByteLength);
typedArray.AttachOffHeapBuffer(buffer, elementsInfo.map, offset);
typedArray.AttachOffHeapBuffer(
buffer, elementsInfo.map, newLength, offset);
}
label IfInvalidAlignment(problemString: String) deferred {
ThrowInvalidTypedArrayAlignment(typedArray.map, problemString);
Expand Down
6 changes: 6 additions & 0 deletions src/code-stub-assembler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10533,6 +10533,12 @@ void CodeStubAssembler::EmitBigTypedArrayElementStore(
TVARIABLE(UintPtrT, var_high);
BigIntToRawBytes(bigint_value, &var_low, &var_high);

// Assert that offset < elements.length. Given that it's an offset for a raw
// pointer we correct it by the usual kHeapObjectTag offset.
CSA_ASSERT(
this, IsOffsetInBounds(offset, LoadAndUntagFixedArrayBaseLength(elements),
kHeapObjectTag, BIGINT64_ELEMENTS));

MachineRepresentation rep = WordT::kMachineRepresentation;
#if defined(V8_TARGET_BIG_ENDIAN)
if (!Is64()) {
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/js-heap-broker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ class JSTypedArrayData : public JSObjectData {
Handle<JSTypedArray> object);

bool is_on_heap() const { return is_on_heap_; }
size_t length() const { return length_; }
size_t length_value() const { return length_value_; }
void* elements_external_pointer() const { return elements_external_pointer_; }

void Serialize(JSHeapBroker* broker);
Expand All @@ -364,7 +364,7 @@ class JSTypedArrayData : public JSObjectData {

private:
bool const is_on_heap_;
size_t const length_;
size_t const length_value_;
void* const elements_external_pointer_;

bool serialized_ = false;
Expand All @@ -375,7 +375,7 @@ JSTypedArrayData::JSTypedArrayData(JSHeapBroker* broker, ObjectData** storage,
Handle<JSTypedArray> object)
: JSObjectData(broker, storage, object),
is_on_heap_(object->is_on_heap()),
length_(object->length()),
length_value_(object->length_value()),
elements_external_pointer_(
FixedTypedArrayBase::cast(object->elements())->external_pointer()) {}

Expand Down Expand Up @@ -2601,7 +2601,7 @@ BIMODAL_ACCESSOR(JSFunction, SharedFunctionInfo, shared)
BIMODAL_ACCESSOR(JSFunction, FeedbackVector, feedback_vector)

BIMODAL_ACCESSOR_C(JSTypedArray, bool, is_on_heap)
BIMODAL_ACCESSOR_C(JSTypedArray, size_t, length)
BIMODAL_ACCESSOR_C(JSTypedArray, size_t, length_value)
BIMODAL_ACCESSOR(JSTypedArray, HeapObject, buffer)

BIMODAL_ACCESSOR_B(Map, bit_field2, elements_kind, Map::ElementsKindBits)
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/js-heap-broker.h
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ class JSTypedArrayRef : public JSObjectRef {
Handle<JSTypedArray> object() const;

bool is_on_heap() const;
size_t length() const;
size_t length_value() const;
void* elements_external_pointer() const;

void Serialize();
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/js-native-context-specialization.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2586,7 +2586,8 @@ JSNativeContextSpecialization::BuildElementAccess(
GetTypedArrayConstant(broker(), receiver);
if (typed_array.has_value()) {
buffer = jsgraph()->Constant(typed_array->buffer());
length = jsgraph()->Constant(static_cast<double>(typed_array->length()));
length =
jsgraph()->Constant(static_cast<double>(typed_array->length_value()));

// Load the (known) base and external pointer for the {receiver}. The
// {external_pointer} might be invalid if the {buffer} was detached, so
Expand Down
5 changes: 2 additions & 3 deletions src/debug/debug-property-iterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,8 @@ void DebugPropertyIterator::FillKeysForCurrentPrototypeAndStage() {
bool has_exotic_indices = receiver->IsJSTypedArray();
if (stage_ == kExoticIndices) {
if (!has_exotic_indices) return;
// TODO(bmeurer, v8:4153): Change this to size_t later.
exotic_length_ =
static_cast<uint32_t>(Handle<JSTypedArray>::cast(receiver)->length());
exotic_length_ = static_cast<uint32_t>(
Handle<JSTypedArray>::cast(receiver)->length_value());
return;
}
bool skip_indices = has_exotic_indices;
Expand Down
66 changes: 30 additions & 36 deletions src/elements.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3080,10 +3080,8 @@ class TypedElementsAccessor

static uint32_t GetCapacityImpl(JSObject holder,
FixedArrayBase backing_store) {
JSTypedArray typed_array = JSTypedArray::cast(holder);
if (WasDetached(typed_array)) return 0;
// TODO(bmeurer, v8:4153): We need to support arbitrary size_t here.
return static_cast<uint32_t>(typed_array->length());
if (WasDetached(holder)) return 0;
return backing_store->length();
}

static uint32_t NumberOfElementsImpl(JSObject receiver,
Expand Down Expand Up @@ -3135,7 +3133,7 @@ class TypedElementsAccessor
// Ensure indexes are within array bounds
CHECK_LE(0, start);
CHECK_LE(start, end);
CHECK_LE(end, array->length());
CHECK_LE(end, array->length_value());

DisallowHeapAllocation no_gc;
BackingStore elements = BackingStore::cast(receiver->elements());
Expand All @@ -3155,24 +3153,23 @@ class TypedElementsAccessor
Handle<Object> value,
uint32_t start_from, uint32_t length) {
DisallowHeapAllocation no_gc;
JSTypedArray typed_array = JSTypedArray::cast(*receiver);

// TODO(caitp): return Just(false) here when implementing strict throwing on
// detached views.
if (WasDetached(typed_array)) {
if (WasDetached(*receiver)) {
return Just(value->IsUndefined(isolate) && length > start_from);
}

BackingStore elements = BackingStore::cast(typed_array->elements());
if (value->IsUndefined(isolate) && length > typed_array->length()) {
BackingStore elements = BackingStore::cast(receiver->elements());
if (value->IsUndefined(isolate) &&
length > static_cast<uint32_t>(elements->length())) {
return Just(true);
}
ctype typed_search_value;
// Prototype has no elements, and not searching for the hole --- limit
// search to backing store length.
if (typed_array->length() < length) {
// TODO(bmeurer, v8:4153): Don't cast to uint32_t here.
length = static_cast<uint32_t>(typed_array->length());
if (static_cast<uint32_t>(elements->length()) < length) {
length = elements->length();
}

if (Kind == BIGINT64_ELEMENTS || Kind == BIGUINT64_ELEMENTS) {
Expand Down Expand Up @@ -3218,11 +3215,10 @@ class TypedElementsAccessor
Handle<Object> value,
uint32_t start_from, uint32_t length) {
DisallowHeapAllocation no_gc;
JSTypedArray typed_array = JSTypedArray::cast(*receiver);

if (WasDetached(typed_array)) return Just<int64_t>(-1);
if (WasDetached(*receiver)) return Just<int64_t>(-1);

BackingStore elements = BackingStore::cast(typed_array->elements());
BackingStore elements = BackingStore::cast(receiver->elements());
ctype typed_search_value;

if (Kind == BIGINT64_ELEMENTS || Kind == BIGUINT64_ELEMENTS) {
Expand Down Expand Up @@ -3254,9 +3250,8 @@ class TypedElementsAccessor

// Prototype has no elements, and not searching for the hole --- limit
// search to backing store length.
if (typed_array->length() < length) {
// TODO(bmeurer, v8:4153): Don't cast to uint32_t here.
length = static_cast<uint32_t>(typed_array->length());
if (static_cast<uint32_t>(elements->length()) < length) {
length = elements->length();
}

for (uint32_t k = start_from; k < length; ++k) {
Expand All @@ -3270,11 +3265,9 @@ class TypedElementsAccessor
Handle<Object> value,
uint32_t start_from) {
DisallowHeapAllocation no_gc;
JSTypedArray typed_array = JSTypedArray::cast(*receiver);

DCHECK(!WasDetached(typed_array));
DCHECK(!WasDetached(*receiver));

BackingStore elements = BackingStore::cast(typed_array->elements());
BackingStore elements = BackingStore::cast(receiver->elements());
ctype typed_search_value;

if (Kind == BIGINT64_ELEMENTS || Kind == BIGUINT64_ELEMENTS) {
Expand Down Expand Up @@ -3304,7 +3297,8 @@ class TypedElementsAccessor
}
}

DCHECK_LT(start_from, typed_array->length());
DCHECK_LT(start_from, elements->length());

uint32_t k = start_from;
do {
ctype element_k = elements->get_scalar(k);
Expand All @@ -3315,13 +3309,11 @@ class TypedElementsAccessor

static void ReverseImpl(JSObject receiver) {
DisallowHeapAllocation no_gc;
JSTypedArray typed_array = JSTypedArray::cast(receiver);
DCHECK(!WasDetached(receiver));

DCHECK(!WasDetached(typed_array));

BackingStore elements = BackingStore::cast(typed_array->elements());
BackingStore elements = BackingStore::cast(receiver->elements());

size_t len = typed_array->length();
uint32_t len = elements->length();
if (len == 0) return;

ctype* data = static_cast<ctype*>(elements->DataPtr());
Expand Down Expand Up @@ -3357,10 +3349,10 @@ class TypedElementsAccessor
CHECK(!source->WasDetached());
CHECK(!destination->WasDetached());
DCHECK_LE(start, end);
DCHECK_LE(end, source->length());
DCHECK_LE(end, source->length_value());

size_t count = end - start;
DCHECK_LE(count, destination->length());
DCHECK_LE(count, destination->length_value());

FixedTypedArrayBase src_elements =
FixedTypedArrayBase::cast(source->elements());
Expand Down Expand Up @@ -3432,9 +3424,10 @@ class TypedElementsAccessor
BackingStore destination_elements =
BackingStore::cast(destination->elements());

DCHECK_LE(offset, destination->length());
DCHECK_LE(length, destination->length() - offset);
DCHECK_LE(length, source->length());
DCHECK_LE(offset, destination->length_value());
DCHECK_LE(length, destination->length_value() - offset);
DCHECK(source->length()->IsSmi());
DCHECK_LE(length, source->length_value());

InstanceType source_type = source_elements->map()->instance_type();
InstanceType destination_type =
Expand Down Expand Up @@ -3524,7 +3517,7 @@ class TypedElementsAccessor
length <= current_length);
USE(current_length);

size_t dest_length = destination->length();
size_t dest_length = destination->length_value();
DCHECK(length + offset <= dest_length);
USE(dest_length);

Expand Down Expand Up @@ -3636,7 +3629,7 @@ class TypedElementsAccessor
Isolate* isolate = destination->GetIsolate();
Handle<JSTypedArray> destination_ta =
Handle<JSTypedArray>::cast(destination);
DCHECK_LE(offset + length, destination_ta->length());
DCHECK_LE(offset + length, destination_ta->length_value());
CHECK(!destination_ta->WasDetached());

if (length == 0) return *isolate->factory()->undefined_value();
Expand Down Expand Up @@ -3664,7 +3657,8 @@ class TypedElementsAccessor
}
// If we have to copy more elements than we have in the source, we need to
// do special handling and conversion; that happens in the slow case.
if (!source_ta->WasDetached() && length + offset <= source_ta->length()) {
if (!source_ta->WasDetached() &&
length + offset <= source_ta->length_value()) {
CopyElementsFromTypedArray(*source_ta, *destination_ta, length, offset);
return *isolate->factory()->undefined_value();
}
Expand Down
Loading

0 comments on commit 6773e16

Please sign in to comment.