From 3b64764b1d7ea62a5ddda301cdcb5547186754c5 Mon Sep 17 00:00:00 2001 From: Georg Neis Date: Tue, 2 Oct 2018 15:39:48 +0200 Subject: [PATCH] Make JSTypedArray::length() and length_value() ignore neutering. Return the actual length even when the buffer is neutered (we used to return 0). This avoids confusion and makes the behavior consistent with byte_offset() and byte_length(). Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: I998f12fa4a428f8555f62e1535247f571ab053f2 Reviewed-on: https://chromium-review.googlesource.com/c/1256768 Reviewed-by: Benedikt Meurer Commit-Queue: Georg Neis Cr-Commit-Position: refs/heads/master@{#56433} --- src/api.cc | 2 +- src/builtins/builtins-sharedarraybuffer.cc | 1 + src/compiler/js-native-context-specialization.cc | 4 ++-- src/elements.cc | 11 ++++++++--- src/objects/js-array-buffer-inl.h | 4 +--- src/objects/js-array-buffer.cc | 4 ++-- src/runtime/runtime-futex.cc | 1 + src/runtime/runtime-wasm.cc | 2 ++ 8 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/api.cc b/src/api.cc index 171c7fc0ef..3f62a23d43 100644 --- a/src/api.cc +++ b/src/api.cc @@ -7639,7 +7639,7 @@ size_t v8::ArrayBufferView::ByteLength() { size_t v8::TypedArray::Length() { i::Handle obj = Utils::OpenHandle(this); - return obj->length_value(); + return obj->WasNeutered() ? 0 : obj->length_value(); } static_assert(v8::TypedArray::kMaxLength == i::Smi::kMaxValue, diff --git a/src/builtins/builtins-sharedarraybuffer.cc b/src/builtins/builtins-sharedarraybuffer.cc index c5239bc444..859d634cc9 100644 --- a/src/builtins/builtins-sharedarraybuffer.cc +++ b/src/builtins/builtins-sharedarraybuffer.cc @@ -74,6 +74,7 @@ V8_WARN_UNUSED_RESULT Maybe ValidateAtomicAccess( size_t access_index; if (!TryNumberToSize(*access_index_obj, &access_index) || + typed_array->WasNeutered() || access_index >= typed_array->length_value()) { isolate->Throw(*isolate->factory()->NewRangeError( MessageTemplate::kInvalidAtomicAccessIndex)); diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc index bd06e81447..af209576a6 100644 --- a/src/compiler/js-native-context-specialization.cc +++ b/src/compiler/js-native-context-specialization.cc @@ -2361,7 +2361,6 @@ JSNativeContextSpecialization::BuildElementAccess( length = jsgraph()->Constant(static_cast(typed_array->length_value())); - // Check if the {receiver}s buffer was neutered. buffer = jsgraph()->HeapConstant(typed_array->GetBuffer()); // Load the (known) base and external pointer for the {receiver}. The @@ -2415,7 +2414,8 @@ JSNativeContextSpecialization::BuildElementAccess( dependencies()->DependOnProtector(PropertyCellRef( js_heap_broker(), factory()->array_buffer_neutering_protector())); } else { - // Bail out if the {buffer} was neutered. + // Deopt if the {buffer} was neutered. + // Note: A neutered buffer leads to megamorphic feedback. Node* buffer_bit_field = effect = graph()->NewNode( simplified()->LoadField(AccessBuilder::ForJSArrayBufferBitField()), buffer, effect, control); diff --git a/src/elements.cc b/src/elements.cc index e4b48b122f..5fad30711d 100644 --- a/src/elements.cc +++ b/src/elements.cc @@ -3178,8 +3178,8 @@ class TypedElementsAccessor size_t start, size_t end) { DisallowHeapAllocation no_gc; DCHECK_EQ(destination->GetElementsKind(), AccessorClass::kind()); - DCHECK(!source->WasNeutered()); - DCHECK(!destination->WasNeutered()); + CHECK(!source->WasNeutered()); + CHECK(!destination->WasNeutered()); DCHECK_LE(start, end); DCHECK_LE(end, source->length_value()); @@ -3249,6 +3249,9 @@ class TypedElementsAccessor // side-effects, as the source elements will always be a number. DisallowHeapAllocation no_gc; + CHECK(!source->WasNeutered()); + CHECK(!destination->WasNeutered()); + FixedTypedArrayBase* source_elements = FixedTypedArrayBase::cast(source->elements()); BackingStore* destination_elements = @@ -3339,6 +3342,8 @@ class TypedElementsAccessor DisallowHeapAllocation no_gc; DisallowJavascriptExecution no_js(isolate); + CHECK(!destination->WasNeutered()); + size_t current_length; DCHECK(source->length()->IsNumber() && TryNumberToSize(source->length(), ¤t_length) && @@ -3459,6 +3464,7 @@ class TypedElementsAccessor Handle destination_ta = Handle::cast(destination); DCHECK_LE(offset + length, destination_ta->length_value()); + CHECK(!destination_ta->WasNeutered()); if (length == 0) return *isolate->factory()->undefined_value(); @@ -3486,7 +3492,6 @@ 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 (length + offset <= source_ta->length_value()) { - DCHECK(length == 0 || !source_ta->WasNeutered()); CopyElementsFromTypedArray(*source_ta, *destination_ta, length, offset); return *isolate->factory()->undefined_value(); } diff --git a/src/objects/js-array-buffer-inl.h b/src/objects/js-array-buffer-inl.h index 42c01352f5..7bc01a8b8a 100644 --- a/src/objects/js-array-buffer-inl.h +++ b/src/objects/js-array-buffer-inl.h @@ -130,13 +130,11 @@ bool JSArrayBufferView::WasNeutered() const { } Object* JSTypedArray::length() const { - if (WasNeutered()) return Smi::kZero; return Object::cast(READ_FIELD(this, kLengthOffset)); } size_t JSTypedArray::length_value() const { - if (WasNeutered()) return 0; - double val = Object::cast(READ_FIELD(this, kLengthOffset))->Number(); + double val = length()->Number(); DCHECK_LE(val, kMaxSafeInteger); // 2^53-1 DCHECK_GE(val, -kMaxSafeInteger); // -2^53+1 DCHECK_LE(val, std::numeric_limits::max()); diff --git a/src/objects/js-array-buffer.cc b/src/objects/js-array-buffer.cc index 9badaca190..36950f9de6 100644 --- a/src/objects/js-array-buffer.cc +++ b/src/objects/js-array-buffer.cc @@ -221,9 +221,9 @@ Maybe JSTypedArray::DefineOwnProperty(Isolate* isolate, NewTypeError(MessageTemplate::kInvalidTypedArrayIndex)); } // 3b iv. Let length be O.[[ArrayLength]]. - uint32_t length = o->length()->Number(); + size_t length = o->length_value(); // 3b v. If numericIndex ≥ length, return false. - if (index >= length) { + if (o->WasNeutered() || index >= length) { RETURN_FAILURE(isolate, should_throw, NewTypeError(MessageTemplate::kInvalidTypedArrayIndex)); } diff --git a/src/runtime/runtime-futex.cc b/src/runtime/runtime-futex.cc index 33af11494a..c891b6582c 100644 --- a/src/runtime/runtime-futex.cc +++ b/src/runtime/runtime-futex.cc @@ -23,6 +23,7 @@ RUNTIME_FUNCTION(Runtime_AtomicsNumWaitersForTesting) { DCHECK_EQ(2, args.length()); CONVERT_ARG_HANDLE_CHECKED(JSTypedArray, sta, 0); CONVERT_SIZE_ARG_CHECKED(index, 1); + CHECK(!sta->WasNeutered()); CHECK(sta->GetBuffer()->is_shared()); CHECK_LT(index, NumberToSize(sta->length())); CHECK_EQ(sta->type(), kExternalInt32Array); diff --git a/src/runtime/runtime-wasm.cc b/src/runtime/runtime-wasm.cc index b9cb600f93..f852df0d85 100644 --- a/src/runtime/runtime-wasm.cc +++ b/src/runtime/runtime-wasm.cc @@ -163,6 +163,7 @@ RUNTIME_FUNCTION(Runtime_WasmExceptionGetElement) { Handle values = Handle::cast(values_obj); CHECK_EQ(values->type(), kExternalUint16Array); CONVERT_SMI_ARG_CHECKED(index, 1); + CHECK(!values->WasNeutered()); CHECK_LT(index, Smi::ToInt(values->length())); auto* vals = reinterpret_cast(values->GetBuffer()->backing_store()); @@ -193,6 +194,7 @@ RUNTIME_FUNCTION(Runtime_WasmExceptionSetElement) { Handle values = Handle::cast(values_obj); CHECK_EQ(values->type(), kExternalUint16Array); CONVERT_SMI_ARG_CHECKED(index, 1); + CHECK(!values->WasNeutered()); CHECK_LT(index, Smi::ToInt(values->length())); CONVERT_SMI_ARG_CHECKED(value, 2); auto* vals =