Skip to content

Commit f7771e5

Browse files
verwaestCommit Bot
verwaest
authored and
Commit Bot
committed
[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 <[email protected]> Reviewed-by: Jakob Kummerow <[email protected]> Reviewed-by: Victor Gomes <[email protected]> Cr-Commit-Position: refs/heads/master@{#65690}
1 parent 4453f89 commit f7771e5

7 files changed

Lines changed: 39 additions & 38 deletions

File tree

‎src/objects/dictionary-inl.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,13 @@ BaseNameDictionary<Derived, Shape>::BaseNameDictionary(Address ptr)
6565
: Dictionary<Derived, Shape>(ptr) {}
6666

6767
template <typename Derived, typename Shape>
68-
void BaseNameDictionary<Derived, Shape>::SetNextEnumerationIndex(int index) {
69-
DCHECK_NE(0, index);
68+
void BaseNameDictionary<Derived, Shape>::set_next_enumeration_index(int index) {
69+
DCHECK_LT(0, index);
7070
this->set(kNextEnumerationIndexIndex, Smi::FromInt(index));
7171
}
7272

7373
template <typename Derived, typename Shape>
74-
int BaseNameDictionary<Derived, Shape>::NextEnumerationIndex() {
74+
int BaseNameDictionary<Derived, Shape>::next_enumeration_index() {
7575
return Smi::ToInt(this->get(kNextEnumerationIndexIndex));
7676
}
7777

‎src/objects/dictionary.h‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,6 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) BaseNameDictionary
125125
static const int kObjectHashIndex = kNextEnumerationIndexIndex + 1;
126126
static const int kEntryValueIndex = 1;
127127

128-
// Accessors for next enumeration index.
129-
inline void SetNextEnumerationIndex(int index);
130-
inline int NextEnumerationIndex();
131-
132128
inline void SetHash(int hash);
133129
inline int Hash() const;
134130

@@ -143,6 +139,13 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) BaseNameDictionary
143139
V8_WARN_UNUSED_RESULT static ExceptionStatus CollectKeysTo(
144140
Handle<Derived> dictionary, KeyAccumulator* keys);
145141

142+
// Allocate the next enumeration index. Possibly updates all enumeration
143+
// indices in the table.
144+
static int NextEnumerationIndex(Isolate* isolate, Handle<Derived> dictionary);
145+
// Accessors for next enumeration index.
146+
inline int next_enumeration_index();
147+
inline void set_next_enumeration_index(int index);
148+
146149
// Return the key indices sorted by its enumeration index.
147150
static Handle<FixedArray> IterationIndices(Isolate* isolate,
148151
Handle<Derived> dictionary);
@@ -154,10 +157,6 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) BaseNameDictionary
154157
Handle<FixedArray> storage, KeyCollectionMode mode,
155158
KeyAccumulator* accumulator);
156159

157-
// Ensure enough space for n additional elements.
158-
static Handle<Derived> EnsureCapacity(Isolate* isolate,
159-
Handle<Derived> dictionary, int n);
160-
161160
V8_WARN_UNUSED_RESULT static Handle<Derived> AddNoUpdateNextEnumerationIndex(
162161
Isolate* isolate, Handle<Derived> dictionary, Key key,
163162
Handle<Object> value, PropertyDetails details,

‎src/objects/hash-table.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) HashTable
192192

193193
// Ensure enough space for n additional elements.
194194
V8_WARN_UNUSED_RESULT static Handle<Derived> EnsureCapacity(
195-
Isolate* isolate, Handle<Derived> table, int n,
195+
Isolate* isolate, Handle<Derived> table, int n = 1,
196196
AllocationType allocation = AllocationType::kYoung);
197197

198198
// Returns true if this table has sufficient capacity for adding n elements.

‎src/objects/js-objects.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2910,7 +2910,7 @@ void MigrateFastToSlow(Isolate* isolate, Handle<JSObject> object,
29102910
}
29112911

29122912
// Copy the next enumeration index from instance descriptor.
2913-
dictionary->SetNextEnumerationIndex(real_size + 1);
2913+
dictionary->set_next_enumeration_index(real_size + 1);
29142914

29152915
// From here on we cannot fail and we shouldn't GC anymore.
29162916
DisallowHeapAllocation no_allocation;

‎src/objects/literal-objects.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ class ObjectDescriptor {
365365

366366
void Finalize(Isolate* isolate) {
367367
if (HasDictionaryProperties()) {
368-
properties_dictionary_template_->SetNextEnumerationIndex(
368+
properties_dictionary_template_->set_next_enumeration_index(
369369
next_enumeration_index_);
370370
computed_properties_ = FixedArray::ShrinkOrEmpty(
371371
isolate, computed_properties_, current_computed_index_);

‎src/objects/lookup.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,8 @@ void LookupIterator::PrepareTransitionToDataProperty(
589589
transition_ = cell;
590590
// Assign an enumeration index to the property and update
591591
// SetNextEnumerationIndex.
592-
int index = dictionary->NextEnumerationIndex();
593-
dictionary->SetNextEnumerationIndex(index + 1);
592+
int index = GlobalDictionary::NextEnumerationIndex(isolate_, dictionary);
593+
dictionary->set_next_enumeration_index(index + 1);
594594
property_details_ = PropertyDetails(
595595
kData, attributes, PropertyCellType::kUninitialized, index);
596596
PropertyCellType new_type =

‎src/objects/objects.cc‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6686,7 +6686,7 @@ void StringTable::EnsureCapacityForDeserialization(Isolate* isolate,
66866686
int expected) {
66876687
Handle<StringTable> table = isolate->factory()->string_table();
66886688
// We need a key instance for the virtual hash function.
6689-
table = StringTable::EnsureCapacity(isolate, table, expected);
6689+
table = EnsureCapacity(isolate, table, expected);
66906690
isolate->heap()->SetRootStringTable(*table);
66916691
}
66926692

@@ -6738,7 +6738,7 @@ Handle<String> StringTable::LookupKey(Isolate* isolate, StringTableKey* key) {
67386738

67396739
table = StringTable::CautiousShrink(isolate, table);
67406740
// Adding new string. Grow table if needed.
6741-
table = StringTable::EnsureCapacity(isolate, table, 1);
6741+
table = EnsureCapacity(isolate, table);
67426742
isolate->heap()->SetRootStringTable(*table);
67436743

67446744
return AddKeyNoResize(isolate, key);
@@ -6880,7 +6880,7 @@ Handle<StringSet> StringSet::New(Isolate* isolate) {
68806880
Handle<StringSet> StringSet::Add(Isolate* isolate, Handle<StringSet> stringset,
68816881
Handle<String> name) {
68826882
if (!stringset->Has(isolate, name)) {
6883-
stringset = EnsureCapacity(isolate, stringset, 1);
6883+
stringset = EnsureCapacity(isolate, stringset);
68846884
uint32_t hash = ShapeT::Hash(isolate, *name);
68856885
InternalIndex entry = stringset->FindInsertionEntry(hash);
68866886
stringset->set(EntryToIndex(entry), *name);
@@ -6898,7 +6898,7 @@ Handle<ObjectHashSet> ObjectHashSet::Add(Isolate* isolate,
68986898
Handle<Object> key) {
68996899
int32_t hash = key->GetOrCreateHash(isolate).value();
69006900
if (!set->Has(isolate, key, hash)) {
6901-
set = EnsureCapacity(isolate, set, 1);
6901+
set = EnsureCapacity(isolate, set);
69026902
InternalIndex entry = set->FindInsertionEntry(hash);
69036903
set->set(EntryToIndex(entry), *key);
69046904
set->ElementAdded();
@@ -7094,7 +7094,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutScript(
70947094
src = String::Flatten(isolate, src);
70957095
StringSharedKey key(src, shared, language_mode, kNoSourcePosition);
70967096
Handle<Object> k = key.AsHandle(isolate);
7097-
cache = EnsureCapacity(isolate, cache, 1);
7097+
cache = EnsureCapacity(isolate, cache);
70987098
InternalIndex entry = cache->FindInsertionEntry(key.Hash());
70997099
cache->set(EntryToIndex(entry), *k);
71007100
cache->set(EntryToIndex(entry) + 1, *value);
@@ -7126,7 +7126,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutEval(
71267126
}
71277127
}
71287128

7129-
cache = EnsureCapacity(isolate, cache, 1);
7129+
cache = EnsureCapacity(isolate, cache);
71307130
InternalIndex entry = cache->FindInsertionEntry(key.Hash());
71317131
Handle<Object> k =
71327132
isolate->factory()->NewNumber(static_cast<double>(key.Hash()));
@@ -7140,7 +7140,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutRegExp(
71407140
Isolate* isolate, Handle<CompilationCacheTable> cache, Handle<String> src,
71417141
JSRegExp::Flags flags, Handle<FixedArray> value) {
71427142
RegExpKey key(src, flags);
7143-
cache = EnsureCapacity(isolate, cache, 1);
7143+
cache = EnsureCapacity(isolate, cache);
71447144
InternalIndex entry = cache->FindInsertionEntry(key.Hash());
71457145
// We store the value in the key slot, and compare the search key
71467146
// to the stored value with a custon IsMatch function during lookups.
@@ -7202,15 +7202,16 @@ Handle<Derived> BaseNameDictionary<Derived, Shape>::New(
72027202
Handle<Derived> dict = Dictionary<Derived, Shape>::New(
72037203
isolate, at_least_space_for, allocation, capacity_option);
72047204
dict->SetHash(PropertyArray::kNoHashSentinel);
7205-
dict->SetNextEnumerationIndex(PropertyDetails::kInitialIndex);
7205+
dict->set_next_enumeration_index(PropertyDetails::kInitialIndex);
72067206
return dict;
72077207
}
72087208

72097209
template <typename Derived, typename Shape>
7210-
Handle<Derived> BaseNameDictionary<Derived, Shape>::EnsureCapacity(
7211-
Isolate* isolate, Handle<Derived> dictionary, int n) {
7212-
// Check whether there are enough enumeration indices to add n elements.
7213-
if (!PropertyDetails::IsValidIndex(dictionary->NextEnumerationIndex() + n)) {
7210+
int BaseNameDictionary<Derived, Shape>::NextEnumerationIndex(
7211+
Isolate* isolate, Handle<Derived> dictionary) {
7212+
int index = dictionary->next_enumeration_index();
7213+
// Check whether the next enumeration index is valid.
7214+
if (!PropertyDetails::IsValidIndex(index)) {
72147215
// If not, we generate new indices for the properties.
72157216
int length = dictionary->NumberOfElements();
72167217

@@ -7231,11 +7232,12 @@ Handle<Derived> BaseNameDictionary<Derived, Shape>::EnsureCapacity(
72317232
dictionary->DetailsAtPut(isolate, index, new_details);
72327233
}
72337234

7234-
// Set the next enumeration index.
7235-
dictionary->SetNextEnumerationIndex(PropertyDetails::kInitialIndex +
7236-
length);
7235+
index = PropertyDetails::kInitialIndex + length;
72377236
}
7238-
return HashTable<Derived, Shape>::EnsureCapacity(isolate, dictionary, n);
7237+
7238+
// Don't update the next enumeration index here, since we might be looking at
7239+
// an immutable empty dictionary.
7240+
return index;
72397241
}
72407242

72417243
template <typename Derived, typename Shape>
@@ -7284,13 +7286,13 @@ Handle<Derived> BaseNameDictionary<Derived, Shape>::Add(
72847286
DCHECK_EQ(0, details.dictionary_index());
72857287
// Assign an enumeration index to the property and update
72867288
// SetNextEnumerationIndex.
7287-
int index = dictionary->NextEnumerationIndex();
7289+
int index = Derived::NextEnumerationIndex(isolate, dictionary);
72887290
details = details.set_index(index);
72897291
dictionary = AddNoUpdateNextEnumerationIndex(isolate, dictionary, key, value,
72907292
details, entry_out);
72917293
// Update enumeration index here in order to avoid potential modification of
72927294
// the canonical empty dictionary which lives in read only space.
7293-
dictionary->SetNextEnumerationIndex(index + 1);
7295+
dictionary->set_next_enumeration_index(index + 1);
72947296
return dictionary;
72957297
}
72967298

@@ -7304,7 +7306,7 @@ Handle<Derived> Dictionary<Derived, Shape>::Add(Isolate* isolate,
73047306
// Validate that the key is absent.
73057307
SLOW_DCHECK(dictionary->FindEntry(isolate, key).is_not_found());
73067308
// Check whether the dictionary should be extended.
7307-
dictionary = Derived::EnsureCapacity(isolate, dictionary, 1);
7309+
dictionary = Derived::EnsureCapacity(isolate, dictionary);
73087310

73097311
// Compute the key object.
73107312
Handle<Object> k = Shape::AsHandle(isolate, key);
@@ -7651,7 +7653,7 @@ Handle<Derived> ObjectHashTableBase<Derived, Shape>::Put(Isolate* isolate,
76517653
}
76527654

76537655
// Check whether the hash table should be extended.
7654-
table = Derived::EnsureCapacity(isolate, table, 1);
7656+
table = Derived::EnsureCapacity(isolate, table);
76557657
table->AddEntry(table->FindInsertionEntry(hash), *key, *value);
76567658
return table;
76577659
}
@@ -7900,8 +7902,8 @@ Handle<PropertyCell> PropertyCell::PrepareForValue(
79007902
// Preserve the enumeration index unless the property was deleted or never
79017903
// initialized.
79027904
if (cell->value().IsTheHole(isolate)) {
7903-
index = dictionary->NextEnumerationIndex();
7904-
dictionary->SetNextEnumerationIndex(index + 1);
7905+
index = GlobalDictionary::NextEnumerationIndex(isolate, dictionary);
7906+
dictionary->set_next_enumeration_index(index + 1);
79057907
} else {
79067908
index = original_details.dictionary_index();
79077909
}

0 commit comments

Comments
 (0)