diff --git a/src/debug/debug-interface.cc b/src/debug/debug-interface.cc index 50c63e8f8e..5ec5c023f7 100644 --- a/src/debug/debug-interface.cc +++ b/src/debug/debug-interface.cc @@ -1267,7 +1267,7 @@ MaybeLocal GetMessageFromPromise(Local p) { } std::unique_ptr PropertyIterator::Create( - Local context, Local object) { + Local context, Local object, bool skip_indices) { internal::Isolate* isolate = reinterpret_cast(object->GetIsolate()); if (IsExecutionTerminatingCheck(isolate)) { @@ -1275,8 +1275,8 @@ std::unique_ptr PropertyIterator::Create( } CallDepthScope call_depth_scope(isolate, context); - auto result = - i::DebugPropertyIterator::Create(isolate, Utils::OpenHandle(*object)); + auto result = i::DebugPropertyIterator::Create( + isolate, Utils::OpenHandle(*object), skip_indices); if (!result) { DCHECK(isolate->has_pending_exception()); call_depth_scope.Escape(); diff --git a/src/debug/debug-interface.h b/src/debug/debug-interface.h index b186ab5689..104cc02105 100644 --- a/src/debug/debug-interface.h +++ b/src/debug/debug-interface.h @@ -621,7 +621,8 @@ class V8_EXPORT_PRIVATE PropertyIterator { // Creating a PropertyIterator can potentially throw an exception. // The returned std::unique_ptr is empty iff that happens. V8_WARN_UNUSED_RESULT static std::unique_ptr Create( - v8::Local context, v8::Local object); + v8::Local context, v8::Local object, + bool skip_indices = false); virtual ~PropertyIterator() = default; diff --git a/src/debug/debug-property-iterator.cc b/src/debug/debug-property-iterator.cc index 5d7ecda979..b0bca65e30 100644 --- a/src/debug/debug-property-iterator.cc +++ b/src/debug/debug-property-iterator.cc @@ -15,15 +15,14 @@ namespace v8 { namespace internal { std::unique_ptr DebugPropertyIterator::Create( - Isolate* isolate, Handle receiver) { + Isolate* isolate, Handle receiver, bool skip_indices) { // Can't use std::make_unique as Ctor is private. auto iterator = std::unique_ptr( - new DebugPropertyIterator(isolate, receiver)); + new DebugPropertyIterator(isolate, receiver, skip_indices)); if (receiver->IsJSProxy()) { iterator->AdvanceToPrototype(); } - if (iterator->Done()) return iterator; if (!iterator->FillKeysForCurrentPrototypeAndStage()) return nullptr; if (iterator->should_move_to_next_stage() && !iterator->AdvanceInternal()) { @@ -34,10 +33,15 @@ std::unique_ptr DebugPropertyIterator::Create( } DebugPropertyIterator::DebugPropertyIterator(Isolate* isolate, - Handle receiver) + Handle receiver, + bool skip_indices) : isolate_(isolate), prototype_iterator_(isolate, receiver, kStartAtReceiver, - PrototypeIterator::END_AT_NULL) {} + PrototypeIterator::END_AT_NULL), + skip_indices_(skip_indices), + current_key_index_(0), + current_keys_(isolate_->factory()->empty_fixed_array()), + current_keys_length_(0) {} bool DebugPropertyIterator::Done() const { return is_done_; } @@ -54,13 +58,13 @@ bool DebugPropertyIterator::AdvanceInternal() { calculated_native_accessor_flags_ = false; while (should_move_to_next_stage()) { switch (stage_) { - case Stage::kExoticIndices: - stage_ = Stage::kEnumerableStrings; + case kExoticIndices: + stage_ = kEnumerableStrings; break; - case Stage::kEnumerableStrings: - stage_ = Stage::kAllProperties; + case kEnumerableStrings: + stage_ = kAllProperties; break; - case Stage::kAllProperties: + case kAllProperties: AdvanceToPrototype(); break; } @@ -70,20 +74,17 @@ bool DebugPropertyIterator::AdvanceInternal() { } bool DebugPropertyIterator::is_native_accessor() { - if (stage_ == kExoticIndices) return false; CalculateNativeAccessorFlags(); return native_accessor_flags_; } bool DebugPropertyIterator::has_native_getter() { - if (stage_ == kExoticIndices) return false; CalculateNativeAccessorFlags(); return native_accessor_flags_ & static_cast(debug::NativeAccessorType::HasGetter); } bool DebugPropertyIterator::has_native_setter() { - if (stage_ == kExoticIndices) return false; CalculateNativeAccessorFlags(); return native_accessor_flags_ & static_cast(debug::NativeAccessorType::HasSetter); @@ -95,7 +96,7 @@ Handle DebugPropertyIterator::raw_name() const { return isolate_->factory()->SizeToString(current_key_index_); } else { return Handle::cast(FixedArray::get( - *keys_, static_cast(current_key_index_), isolate_)); + *current_keys_, static_cast(current_key_index_), isolate_)); } } @@ -140,42 +141,38 @@ bool DebugPropertyIterator::is_own() { return is_own_; } bool DebugPropertyIterator::is_array_index() { if (stage_ == kExoticIndices) return true; - uint32_t index = 0; - return raw_name()->AsArrayIndex(&index); + PropertyKey key(isolate_, raw_name()); + return key.is_element(); } bool DebugPropertyIterator::FillKeysForCurrentPrototypeAndStage() { current_key_index_ = 0; - exotic_length_ = 0; - keys_ = Handle::null(); + current_keys_ = isolate_->factory()->empty_fixed_array(); + current_keys_length_ = 0; if (is_done_) return true; Handle receiver = PrototypeIterator::GetCurrent(prototype_iterator_); - bool has_exotic_indices = receiver->IsJSTypedArray(); if (stage_ == kExoticIndices) { - if (!has_exotic_indices) return true; + if (skip_indices_ || !receiver->IsJSTypedArray()) return true; Handle typed_array = Handle::cast(receiver); - exotic_length_ = typed_array->WasDetached() ? 0 : typed_array->length(); + current_keys_length_ = + typed_array->WasDetached() ? 0 : typed_array->length(); return true; } - bool skip_indices = has_exotic_indices; PropertyFilter filter = stage_ == kEnumerableStrings ? ENUMERABLE_STRINGS : ALL_PROPERTIES; - if (!KeyAccumulator::GetKeys(receiver, KeyCollectionMode::kOwnOnly, filter, - GetKeysConversion::kConvertToString, false, - skip_indices) - .ToHandle(&keys_)) { - keys_ = Handle::null(); - return false; + if (KeyAccumulator::GetKeys(receiver, KeyCollectionMode::kOwnOnly, filter, + GetKeysConversion::kConvertToString, false, + skip_indices_ || receiver->IsJSTypedArray()) + .ToHandle(¤t_keys_)) { + current_keys_length_ = current_keys_->length(); + return true; } - return true; + return false; } bool DebugPropertyIterator::should_move_to_next_stage() const { - if (is_done_) return false; - if (stage_ == kExoticIndices) return current_key_index_ >= exotic_length_; - return keys_.is_null() || - current_key_index_ >= static_cast(keys_->length()); + return !is_done_ && current_key_index_ >= current_keys_length_; } namespace { @@ -210,10 +207,14 @@ base::Flags GetNativeAccessorDescriptorInternal( void DebugPropertyIterator::CalculateNativeAccessorFlags() { if (calculated_native_accessor_flags_) return; - Handle receiver = - PrototypeIterator::GetCurrent(prototype_iterator_); - native_accessor_flags_ = - GetNativeAccessorDescriptorInternal(receiver, raw_name()); + if (stage_ == kExoticIndices) { + native_accessor_flags_ = 0; + } else { + Handle receiver = + PrototypeIterator::GetCurrent(prototype_iterator_); + native_accessor_flags_ = + GetNativeAccessorDescriptorInternal(receiver, raw_name()); + } calculated_native_accessor_flags_ = true; } } // namespace internal diff --git a/src/debug/debug-property-iterator.h b/src/debug/debug-property-iterator.h index 4e6a93f10e..b28fe78ac8 100644 --- a/src/debug/debug-property-iterator.h +++ b/src/debug/debug-property-iterator.h @@ -24,7 +24,7 @@ class JSReceiver; class DebugPropertyIterator final : public debug::PropertyIterator { public: V8_WARN_UNUSED_RESULT static std::unique_ptr Create( - Isolate* isolate, Handle receiver); + Isolate* isolate, Handle receiver, bool skip_indices); ~DebugPropertyIterator() override = default; DebugPropertyIterator(const DebugPropertyIterator&) = delete; DebugPropertyIterator& operator=(const DebugPropertyIterator&) = delete; @@ -43,7 +43,8 @@ class DebugPropertyIterator final : public debug::PropertyIterator { bool is_array_index() override; private: - DebugPropertyIterator(Isolate* isolate, Handle receiver); + DebugPropertyIterator(Isolate* isolate, Handle receiver, + bool skip_indices); V8_WARN_UNUSED_RESULT bool FillKeysForCurrentPrototypeAndStage(); bool should_move_to_next_stage() const; @@ -54,12 +55,16 @@ class DebugPropertyIterator final : public debug::PropertyIterator { Isolate* isolate_; PrototypeIterator prototype_iterator_; - enum Stage { kExoticIndices = 0, kEnumerableStrings = 1, kAllProperties = 2 }; - Stage stage_ = kExoticIndices; + enum { + kExoticIndices = 0, + kEnumerableStrings = 1, + kAllProperties = 2 + } stage_ = kExoticIndices; + bool skip_indices_; - size_t current_key_index_ = 0; - Handle keys_; - size_t exotic_length_ = 0; + size_t current_key_index_; + Handle current_keys_; + size_t current_keys_length_; bool calculated_native_accessor_flags_ = false; int native_accessor_flags_ = 0; diff --git a/src/inspector/value-mirror.cc b/src/inspector/value-mirror.cc index 57eebb0c80..e269b1357a 100644 --- a/src/inspector/value-mirror.cc +++ b/src/inspector/value-mirror.cc @@ -1211,7 +1211,8 @@ bool ValueMirror::getProperties(v8::Local context, } } - auto iterator = v8::debug::PropertyIterator::Create(context, object); + auto iterator = v8::debug::PropertyIterator::Create(context, object, + nonIndexedPropertiesOnly); if (!iterator) { CHECK(tryCatch.HasCaught()); return false; @@ -1219,14 +1220,6 @@ bool ValueMirror::getProperties(v8::Local context, while (!iterator->Done()) { bool isOwn = iterator->is_own(); if (!isOwn && ownProperties) break; - bool isIndex = iterator->is_array_index(); - if (isIndex && nonIndexedPropertiesOnly) { - if (!iterator->Advance().FromMaybe(false)) { - CHECK(tryCatch.HasCaught()); - return false; - } - continue; - } v8::Local v8Name = iterator->name(); v8::Maybe result = set->Has(context, v8Name); if (result.IsNothing()) return false; @@ -1324,7 +1317,7 @@ bool ValueMirror::getProperties(v8::Local context, configurable, enumerable, isOwn, - isIndex, + iterator->is_array_index(), isAccessorProperty && valueMirror, std::move(valueMirror), std::move(getterMirror), diff --git a/test/unittests/debug/debug-property-iterator-unittest.cc b/test/unittests/debug/debug-property-iterator-unittest.cc index fa14c7dc0a..feb4f4f2ad 100644 --- a/test/unittests/debug/debug-property-iterator-unittest.cc +++ b/test/unittests/debug/debug-property-iterator-unittest.cc @@ -98,6 +98,56 @@ TEST_F(DebugPropertyIteratorTest, DoestWalksPrototypeChainIfInaccesible) { ASSERT_TRUE(iterator->Done()); } +TEST_F(DebugPropertyIteratorTest, SkipsIndicesOnArrays) { + TryCatch try_catch(isolate()); + + Local elements[2] = { + Number::New(isolate(), 21), + Number::New(isolate(), 42), + }; + auto array = Array::New(isolate(), elements, arraysize(elements)); + + auto iterator = PropertyIterator::Create(context(), array, true); + while (!iterator->Done()) { + ASSERT_FALSE(iterator->is_array_index()); + ASSERT_TRUE(iterator->Advance().FromMaybe(false)); + } +} + +TEST_F(DebugPropertyIteratorTest, SkipsIndicesOnObjects) { + TryCatch try_catch(isolate()); + + Local names[2] = { + String::NewFromUtf8Literal(isolate(), "42"), + String::NewFromUtf8Literal(isolate(), "x"), + }; + Local values[arraysize(names)] = { + Number::New(isolate(), 42), + Number::New(isolate(), 21), + }; + Local object = + Object::New(isolate(), Null(isolate()), names, values, arraysize(names)); + + auto iterator = PropertyIterator::Create(context(), object, true); + while (!iterator->Done()) { + ASSERT_FALSE(iterator->is_array_index()); + ASSERT_TRUE(iterator->Advance().FromMaybe(false)); + } +} + +TEST_F(DebugPropertyIteratorTest, SkipsIndicesOnTypedArrays) { + TryCatch try_catch(isolate()); + + auto buffer = ArrayBuffer::New(isolate(), 1024 * 1024); + auto array = Uint8Array::New(buffer, 0, 1024 * 1024); + + auto iterator = PropertyIterator::Create(context(), array, true); + while (!iterator->Done()) { + ASSERT_FALSE(iterator->is_array_index()); + ASSERT_TRUE(iterator->Advance().FromMaybe(false)); + } +} + } // namespace } // namespace debug } // namespace v8