From 6108bac6060c4c3164e95e2969781531a477f3fc Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 5 Oct 2021 15:05:06 +0200 Subject: [PATCH] LibJS: Only do a single property lookup in internal_get_own_property() Instead of checking storage_has(), followed by storage_get(), we can do storage_get() directly and avoid a redundant property lookup. This exposed a bug in SimpleIndexedPropertyStorage::get() which would previously succeed for array holes. --- Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp | 2 +- Userland/Libraries/LibJS/Runtime/Object.cpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp b/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp index 090614b28c..6b0ca3b0a9 100644 --- a/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp +++ b/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp @@ -26,7 +26,7 @@ bool SimpleIndexedPropertyStorage::has_index(u32 index) const Optional SimpleIndexedPropertyStorage::get(u32 index) const { - if (index >= m_array_size) + if (!has_index(index)) return {}; return ValueAndAttributes { m_packed_elements[index], default_attributes }; } diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index 836157fba3..d4df5cd051 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -546,14 +546,15 @@ ThrowCompletionOr> Object::internal_get_own_propert VERIFY(property_name.is_valid()); // 2. If O does not have an own property with key P, return undefined. - if (!storage_has(property_name)) + auto maybe_storage_entry = storage_get(property_name); + if (!maybe_storage_entry.has_value()) return Optional {}; // 3. Let D be a newly created Property Descriptor with no fields. PropertyDescriptor descriptor; // 4. Let X be O's own property whose key is P. - auto [value, attributes] = *storage_get(property_name); + auto [value, attributes] = *maybe_storage_entry; // 5. If X is a data property, then if (!value.is_accessor()) {