Backport: Fix Object.entries/values with changing elements
Bug: 111274046
Test: m -j proxy_resolver_v8_unittest && adb sync && adb shell \
/data/nativetest64/proxy_resolver_v8_unittest/proxy_resolver_v8_unittest
Merged-In: I705fc512cc5837e9364ed187559cc75d079aa5cb
Change-Id: I58c84a46949b79db44b1bb4f17794b54ebb9a301
(cherry picked from commit 275d7152a6029056732896bd45fd05a243b0bf4f)
diff --git a/src/elements.cc b/src/elements.cc
index ccbdb40..6175ee7 100644
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -509,6 +509,21 @@
return Just<int64_t>(-1);
}
+// The InternalElementsAccessor is a helper class to expose otherwise protected
+// methods to its subclasses. Namely, we don't want to publicly expose methods
+// that take an entry (instead of an index) as an argument.
+class InternalElementsAccessor : public ElementsAccessor {
+ public:
+ explicit InternalElementsAccessor(const char* name)
+ : ElementsAccessor(name) {}
+
+ virtual uint32_t GetEntryForIndex(Isolate* isolate, JSObject* holder,
+ FixedArrayBase* backing_store,
+ uint32_t index) = 0;
+
+ virtual PropertyDetails GetDetails(JSObject* holder, uint32_t entry) = 0;
+};
+
// Base class for element handler implementations. Contains the
// the common logic for objects with different ElementsKinds.
// Subclasses must specialize method for which the element
@@ -527,10 +542,10 @@
// CRTP to guarantee aggressive compile time optimizations (i.e. inlining and
// specialization of SomeElementsAccessor methods).
template <typename Subclass, typename ElementsTraitsParam>
-class ElementsAccessorBase : public ElementsAccessor {
+class ElementsAccessorBase : public InternalElementsAccessor {
public:
explicit ElementsAccessorBase(const char* name)
- : ElementsAccessor(name) { }
+ : InternalElementsAccessor(name) {}
typedef ElementsTraitsParam ElementsTraits;
typedef typename ElementsTraitsParam::BackingStore BackingStore;
@@ -1014,35 +1029,66 @@
Isolate* isolate, Handle<JSObject> object,
Handle<FixedArray> values_or_entries, bool get_entries, int* nof_items,
PropertyFilter filter) {
- int count = 0;
+ DCHECK_EQ(*nof_items, 0);
KeyAccumulator accumulator(isolate, KeyCollectionMode::kOwnOnly,
ALL_PROPERTIES);
Subclass::CollectElementIndicesImpl(
object, handle(object->elements(), isolate), &accumulator);
Handle<FixedArray> keys = accumulator.GetKeys();
- for (int i = 0; i < keys->length(); ++i) {
+ int count = 0;
+ int i = 0;
+ Handle<Map> original_map(object->map(), isolate);
+
+ for (; i < keys->length(); ++i) {
Handle<Object> key(keys->get(i), isolate);
- Handle<Object> value;
uint32_t index;
if (!key->ToUint32(&index)) continue;
+ DCHECK_EQ(object->map(), *original_map);
uint32_t entry = Subclass::GetEntryForIndexImpl(
isolate, *object, object->elements(), index, filter);
if (entry == kMaxUInt32) continue;
PropertyDetails details = Subclass::GetDetailsImpl(*object, entry);
+ Handle<Object> value;
if (details.kind() == kData) {
value = Subclass::GetImpl(object, entry);
} else {
+ // This might modify the elements and/or change the elements kind.
LookupIterator it(isolate, object, index, LookupIterator::OWN);
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate, value, Object::GetProperty(&it), Nothing<bool>());
}
- if (get_entries) {
- value = MakeEntryPair(isolate, index, value);
+ if (get_entries) value = MakeEntryPair(isolate, index, value);
+ values_or_entries->set(count++, *value);
+ if (object->map() != *original_map) break;
+ }
+
+ // Slow path caused by changes in elements kind during iteration.
+ for (; i < keys->length(); i++) {
+ Handle<Object> key(keys->get(i), isolate);
+ uint32_t index;
+ if (!key->ToUint32(&index)) continue;
+
+ if (filter & ONLY_ENUMERABLE) {
+ InternalElementsAccessor* accessor =
+ reinterpret_cast<InternalElementsAccessor*>(
+ object->GetElementsAccessor());
+ uint32_t entry = accessor->GetEntryForIndex(isolate, *object,
+ object->elements(), index);
+ if (entry == kMaxUInt32) continue;
+ PropertyDetails details = accessor->GetDetails(*object, entry);
+ if (!details.IsEnumerable()) continue;
}
+
+ Handle<Object> value;
+ LookupIterator it(isolate, object, index, LookupIterator::OWN);
+ ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, value, Object::GetProperty(&it),
+ Nothing<bool>());
+
+ if (get_entries) value = MakeEntryPair(isolate, index, value);
values_or_entries->set(count++, *value);
}
@@ -1616,12 +1662,13 @@
return result;
}
}
-
+ Handle<Map> original_map(receiver->map(), isolate);
Handle<SeededNumberDictionary> dictionary(
SeededNumberDictionary::cast(receiver->elements()), isolate);
// Iterate through entire range, as accessing elements out of order is
// observable
for (uint32_t k = start_from; k < length; ++k) {
+ DCHECK_EQ(receiver->map(), *original_map);
int entry = dictionary->FindEntry(k);
if (entry == SeededNumberDictionary::kNotFound) {
if (search_for_hole) return Just(true);
@@ -1683,11 +1730,13 @@
uint32_t start_from, uint32_t length) {
DCHECK(JSObject::PrototypeHasNoElements(isolate, *receiver));
+ Handle<Map> original_map(receiver->map(), isolate);
Handle<SeededNumberDictionary> dictionary(
SeededNumberDictionary::cast(receiver->elements()), isolate);
// Iterate through entire range, as accessing elements out of order is
// observable.
for (uint32_t k = start_from; k < length; ++k) {
+ DCHECK_EQ(receiver->map(), *original_map);
int entry = dictionary->FindEntry(k);
if (entry == SeededNumberDictionary::kNotFound) {
continue;
@@ -3143,12 +3192,13 @@
Handle<Object> value,
uint32_t start_from, uint32_t length) {
DCHECK(JSObject::PrototypeHasNoElements(isolate, *object));
- Handle<Map> original_map = handle(object->map(), isolate);
+ Handle<Map> original_map(object->map(), isolate);
Handle<FixedArray> parameter_map(FixedArray::cast(object->elements()),
isolate);
bool search_for_hole = value->IsUndefined(isolate);
for (uint32_t k = start_from; k < length; ++k) {
+ DCHECK_EQ(object->map(), *original_map);
uint32_t entry = GetEntryForIndexImpl(isolate, *object, *parameter_map, k,
ALL_PROPERTIES);
if (entry == kMaxUInt32) {
@@ -3184,11 +3234,12 @@
Handle<Object> value,
uint32_t start_from, uint32_t length) {
DCHECK(JSObject::PrototypeHasNoElements(isolate, *object));
- Handle<Map> original_map = handle(object->map(), isolate);
+ Handle<Map> original_map(object->map(), isolate);
Handle<FixedArray> parameter_map(FixedArray::cast(object->elements()),
isolate);
for (uint32_t k = start_from; k < length; ++k) {
+ DCHECK_EQ(object->map(), *original_map);
uint32_t entry = GetEntryForIndexImpl(isolate, *object, *parameter_map, k,
ALL_PROPERTIES);
if (entry == kMaxUInt32) {
diff --git a/src/elements.h b/src/elements.h
index fc2e6a4..4becbe4 100644
--- a/src/elements.h
+++ b/src/elements.h
@@ -54,7 +54,6 @@
virtual Handle<Object> Get(Handle<JSObject> holder, uint32_t entry) = 0;
- virtual PropertyDetails GetDetails(JSObject* holder, uint32_t entry) = 0;
virtual bool HasAccessors(JSObject* holder) = 0;
virtual uint32_t NumberOfElements(JSObject* holder) = 0;
@@ -65,9 +64,6 @@
// element that is non-deletable.
virtual void SetLength(Handle<JSArray> holder, uint32_t new_length) = 0;
- // Deletes an element in an object.
- virtual void Delete(Handle<JSObject> holder, uint32_t entry) = 0;
-
// If kCopyToEnd is specified as the copy_size to CopyElements, it copies all
// of elements from source after source_start to the destination array.
static const int kCopyToEnd = -1;
@@ -124,11 +120,6 @@
virtual void Set(Handle<JSObject> holder, uint32_t entry, Object* value) = 0;
- virtual void Reconfigure(Handle<JSObject> object,
- Handle<FixedArrayBase> backing_store, uint32_t entry,
- Handle<Object> value,
- PropertyAttributes attributes) = 0;
-
virtual void Add(Handle<JSObject> object, uint32_t index,
Handle<Object> value, PropertyAttributes attributes,
uint32_t new_capacity) = 0;
@@ -190,6 +181,15 @@
FixedArrayBase* backing_store,
uint32_t index) = 0;
+ virtual PropertyDetails GetDetails(JSObject* holder, uint32_t entry) = 0;
+ virtual void Reconfigure(Handle<JSObject> object,
+ Handle<FixedArrayBase> backing_store, uint32_t entry,
+ Handle<Object> value,
+ PropertyAttributes attributes) = 0;
+
+ // Deletes an element in an object.
+ virtual void Delete(Handle<JSObject> holder, uint32_t entry) = 0;
+
// NOTE: this method violates the handlified function signature convention:
// raw pointer parameter |source_holder| in the function that allocates.
// This is done intentionally to avoid ArrayConcat() builtin performance
