[runtime] Recompute enumeration indices of dictionaries upon bitfield overflow

Otherwise we'll get weird semantics when enumerating objects after many
deletes/reinserts.

Bug: chromium:1033771
Change-Id: If0a459169c3794a30d9632d09e80da3cfcd4302c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993966
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65690}

src/objects/dictionary-inl.h

@@ -65,13 +65,13 @@ BaseNameDictionary<Derived, Shape>::BaseNameDictionary(Address ptr)
     : Dictionary<Derived, Shape>(ptr) {}
 
 template <typename Derived, typename Shape>
-void BaseNameDictionary<Derived, Shape>::SetNextEnumerationIndex(int index) {
-  DCHECK_NE(0, index);
+void BaseNameDictionary<Derived, Shape>::set_next_enumeration_index(int index) {
+  DCHECK_LT(0, index);
   this->set(kNextEnumerationIndexIndex, Smi::FromInt(index));
 }
 
 template <typename Derived, typename Shape>
-int BaseNameDictionary<Derived, Shape>::NextEnumerationIndex() {
+int BaseNameDictionary<Derived, Shape>::next_enumeration_index() {
   return Smi::ToInt(this->get(kNextEnumerationIndexIndex));
 }
 

src/objects/dictionary.h

@@ -125,10 +125,6 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) BaseNameDictionary
   static const int kObjectHashIndex = kNextEnumerationIndexIndex + 1;
   static const int kEntryValueIndex = 1;
 
-  // Accessors for next enumeration index.
-  inline void SetNextEnumerationIndex(int index);
-  inline int NextEnumerationIndex();
-
   inline void SetHash(int hash);
   inline int Hash() const;
 
@@ -143,6 +139,13 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) BaseNameDictionary
   V8_WARN_UNUSED_RESULT static ExceptionStatus CollectKeysTo(
       Handle<Derived> dictionary, KeyAccumulator* keys);
 
+  // Allocate the next enumeration index. Possibly updates all enumeration
+  // indices in the table.
+  static int NextEnumerationIndex(Isolate* isolate, Handle<Derived> dictionary);
+  // Accessors for next enumeration index.
+  inline int next_enumeration_index();
+  inline void set_next_enumeration_index(int index);
+
   // Return the key indices sorted by its enumeration index.
   static Handle<FixedArray> IterationIndices(Isolate* isolate,
                                              Handle<Derived> dictionary);
@@ -154,10 +157,6 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) BaseNameDictionary
                              Handle<FixedArray> storage, KeyCollectionMode mode,
                              KeyAccumulator* accumulator);
 
-  // Ensure enough space for n additional elements.
-  static Handle<Derived> EnsureCapacity(Isolate* isolate,
-                                        Handle<Derived> dictionary, int n);
-
   V8_WARN_UNUSED_RESULT static Handle<Derived> AddNoUpdateNextEnumerationIndex(
       Isolate* isolate, Handle<Derived> dictionary, Key key,
       Handle<Object> value, PropertyDetails details,

src/objects/hash-table.h

@@ -192,7 +192,7 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) HashTable
 
   // Ensure enough space for n additional elements.
   V8_WARN_UNUSED_RESULT static Handle<Derived> EnsureCapacity(
-      Isolate* isolate, Handle<Derived> table, int n,
+      Isolate* isolate, Handle<Derived> table, int n = 1,
       AllocationType allocation = AllocationType::kYoung);
 
   // Returns true if this table has sufficient capacity for adding n elements.

src/objects/js-objects.cc

@@ -2910,7 +2910,7 @@ void MigrateFastToSlow(Isolate* isolate, Handle<JSObject> object,
   }
 
   // Copy the next enumeration index from instance descriptor.
-  dictionary->SetNextEnumerationIndex(real_size + 1);
+  dictionary->set_next_enumeration_index(real_size + 1);
 
   // From here on we cannot fail and we shouldn't GC anymore.
   DisallowHeapAllocation no_allocation;

src/objects/literal-objects.cc

@@ -365,7 +365,7 @@ class ObjectDescriptor {
 
   void Finalize(Isolate* isolate) {
     if (HasDictionaryProperties()) {
-      properties_dictionary_template_->SetNextEnumerationIndex(
+      properties_dictionary_template_->set_next_enumeration_index(
           next_enumeration_index_);
       computed_properties_ = FixedArray::ShrinkOrEmpty(
           isolate, computed_properties_, current_computed_index_);

src/objects/lookup.cc

@@ -589,8 +589,8 @@ void LookupIterator::PrepareTransitionToDataProperty(
       transition_ = cell;
       // Assign an enumeration index to the property and update
       // SetNextEnumerationIndex.
-      int index = dictionary->NextEnumerationIndex();
-      dictionary->SetNextEnumerationIndex(index + 1);
+      int index = GlobalDictionary::NextEnumerationIndex(isolate_, dictionary);
+      dictionary->set_next_enumeration_index(index + 1);
       property_details_ = PropertyDetails(
           kData, attributes, PropertyCellType::kUninitialized, index);
       PropertyCellType new_type =

src/objects/objects.cc

@@ -6686,7 +6686,7 @@ void StringTable::EnsureCapacityForDeserialization(Isolate* isolate,
                                                    int expected) {
   Handle<StringTable> table = isolate->factory()->string_table();
   // We need a key instance for the virtual hash function.
-  table = StringTable::EnsureCapacity(isolate, table, expected);
+  table = EnsureCapacity(isolate, table, expected);
   isolate->heap()->SetRootStringTable(*table);
 }
 
@@ -6738,7 +6738,7 @@ Handle<String> StringTable::LookupKey(Isolate* isolate, StringTableKey* key) {
 
   table = StringTable::CautiousShrink(isolate, table);
   // Adding new string. Grow table if needed.
-  table = StringTable::EnsureCapacity(isolate, table, 1);
+  table = EnsureCapacity(isolate, table);
   isolate->heap()->SetRootStringTable(*table);
 
   return AddKeyNoResize(isolate, key);
@@ -6880,7 +6880,7 @@ Handle<StringSet> StringSet::New(Isolate* isolate) {
 Handle<StringSet> StringSet::Add(Isolate* isolate, Handle<StringSet> stringset,
                                  Handle<String> name) {
   if (!stringset->Has(isolate, name)) {
-    stringset = EnsureCapacity(isolate, stringset, 1);
+    stringset = EnsureCapacity(isolate, stringset);
     uint32_t hash = ShapeT::Hash(isolate, *name);
     InternalIndex entry = stringset->FindInsertionEntry(hash);
     stringset->set(EntryToIndex(entry), *name);
@@ -6898,7 +6898,7 @@ Handle<ObjectHashSet> ObjectHashSet::Add(Isolate* isolate,
                                          Handle<Object> key) {
   int32_t hash = key->GetOrCreateHash(isolate).value();
   if (!set->Has(isolate, key, hash)) {
-    set = EnsureCapacity(isolate, set, 1);
+    set = EnsureCapacity(isolate, set);
     InternalIndex entry = set->FindInsertionEntry(hash);
     set->set(EntryToIndex(entry), *key);
     set->ElementAdded();
@@ -7094,7 +7094,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutScript(
   src = String::Flatten(isolate, src);
   StringSharedKey key(src, shared, language_mode, kNoSourcePosition);
   Handle<Object> k = key.AsHandle(isolate);
-  cache = EnsureCapacity(isolate, cache, 1);
+  cache = EnsureCapacity(isolate, cache);
   InternalIndex entry = cache->FindInsertionEntry(key.Hash());
   cache->set(EntryToIndex(entry), *k);
   cache->set(EntryToIndex(entry) + 1, *value);
@@ -7126,7 +7126,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutEval(
     }
   }
 
-  cache = EnsureCapacity(isolate, cache, 1);
+  cache = EnsureCapacity(isolate, cache);
   InternalIndex entry = cache->FindInsertionEntry(key.Hash());
   Handle<Object> k =
       isolate->factory()->NewNumber(static_cast<double>(key.Hash()));
@@ -7140,7 +7140,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutRegExp(
     Isolate* isolate, Handle<CompilationCacheTable> cache, Handle<String> src,
     JSRegExp::Flags flags, Handle<FixedArray> value) {
   RegExpKey key(src, flags);
-  cache = EnsureCapacity(isolate, cache, 1);
+  cache = EnsureCapacity(isolate, cache);
   InternalIndex entry = cache->FindInsertionEntry(key.Hash());
   // We store the value in the key slot, and compare the search key
   // to the stored value with a custon IsMatch function during lookups.
@@ -7202,15 +7202,16 @@ Handle<Derived> BaseNameDictionary<Derived, Shape>::New(
   Handle<Derived> dict = Dictionary<Derived, Shape>::New(
       isolate, at_least_space_for, allocation, capacity_option);
   dict->SetHash(PropertyArray::kNoHashSentinel);
-  dict->SetNextEnumerationIndex(PropertyDetails::kInitialIndex);
+  dict->set_next_enumeration_index(PropertyDetails::kInitialIndex);
   return dict;
 }
 
 template <typename Derived, typename Shape>
-Handle<Derived> BaseNameDictionary<Derived, Shape>::EnsureCapacity(
-    Isolate* isolate, Handle<Derived> dictionary, int n) {
-  // Check whether there are enough enumeration indices to add n elements.
-  if (!PropertyDetails::IsValidIndex(dictionary->NextEnumerationIndex() + n)) {
+int BaseNameDictionary<Derived, Shape>::NextEnumerationIndex(
+    Isolate* isolate, Handle<Derived> dictionary) {
+  int index = dictionary->next_enumeration_index();
+  // Check whether the next enumeration index is valid.
+  if (!PropertyDetails::IsValidIndex(index)) {
     // If not, we generate new indices for the properties.
     int length = dictionary->NumberOfElements();
 
@@ -7231,11 +7232,12 @@ Handle<Derived> BaseNameDictionary<Derived, Shape>::EnsureCapacity(
       dictionary->DetailsAtPut(isolate, index, new_details);
     }
 
-    // Set the next enumeration index.
-    dictionary->SetNextEnumerationIndex(PropertyDetails::kInitialIndex +
-                                        length);
+    index = PropertyDetails::kInitialIndex + length;
   }
-  return HashTable<Derived, Shape>::EnsureCapacity(isolate, dictionary, n);
+
+  // Don't update the next enumeration index here, since we might be looking at
+  // an immutable empty dictionary.
+  return index;
 }
 
 template <typename Derived, typename Shape>
@@ -7284,13 +7286,13 @@ Handle<Derived> BaseNameDictionary<Derived, Shape>::Add(
   DCHECK_EQ(0, details.dictionary_index());
   // Assign an enumeration index to the property and update
   // SetNextEnumerationIndex.
-  int index = dictionary->NextEnumerationIndex();
+  int index = Derived::NextEnumerationIndex(isolate, dictionary);
   details = details.set_index(index);
   dictionary = AddNoUpdateNextEnumerationIndex(isolate, dictionary, key, value,
                                                details, entry_out);
   // Update enumeration index here in order to avoid potential modification of
   // the canonical empty dictionary which lives in read only space.
-  dictionary->SetNextEnumerationIndex(index + 1);
+  dictionary->set_next_enumeration_index(index + 1);
   return dictionary;
 }
 
@@ -7304,7 +7306,7 @@ Handle<Derived> Dictionary<Derived, Shape>::Add(Isolate* isolate,
   // Validate that the key is absent.
   SLOW_DCHECK(dictionary->FindEntry(isolate, key).is_not_found());
   // Check whether the dictionary should be extended.
-  dictionary = Derived::EnsureCapacity(isolate, dictionary, 1);
+  dictionary = Derived::EnsureCapacity(isolate, dictionary);
 
   // Compute the key object.
   Handle<Object> k = Shape::AsHandle(isolate, key);
@@ -7651,7 +7653,7 @@ Handle<Derived> ObjectHashTableBase<Derived, Shape>::Put(Isolate* isolate,
   }
 
   // Check whether the hash table should be extended.
-  table = Derived::EnsureCapacity(isolate, table, 1);
+  table = Derived::EnsureCapacity(isolate, table);
   table->AddEntry(table->FindInsertionEntry(hash), *key, *value);
   return table;
 }
@@ -7900,8 +7902,8 @@ Handle<PropertyCell> PropertyCell::PrepareForValue(
   // Preserve the enumeration index unless the property was deleted or never
   // initialized.
   if (cell->value().IsTheHole(isolate)) {
-    index = dictionary->NextEnumerationIndex();
-    dictionary->SetNextEnumerationIndex(index + 1);
+    index = GlobalDictionary::NextEnumerationIndex(isolate, dictionary);
+    dictionary->set_next_enumeration_index(index + 1);
   } else {
     index = original_details.dictionary_index();
   }