Reland "[runtime] Improve handling of enumeration index on global dictionary"

sethbrenith · Commit Bot · commit 5cf576755298 · 2020-03-02T15:41:15.000Z
This is a reland of 25d1657 Changes from original: replaced slow test with fast test Original change's description: > [runtime] Improve handling of enumeration index on global dictionary > > Bug: chromium:1056054 > Change-Id: Ie1f2da98bc54a2ad5189cbe2ee1686fe1ef7019a > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2079035 > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> > Cr-Commit-Position: refs/heads/master@{#66504} Bug: chromium:1056054 Change-Id: I45b9a096b1e37bf1dc5e792f106cdfadd47fabf9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2080855 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Cr-Commit-Position: refs/heads/master@{#66535}
diff --git a/src/objects/objects.cc b/src/objects/objects.cc
@@ -7280,10 +7280,9 @@ int BaseNameDictionary<Derived, Shape>::NextEnumerationIndex(
   // 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();
-
     Handle<FixedArray> iteration_order = IterationIndices(isolate, dictionary);
-    DCHECK_EQ(length, iteration_order->length());
+    int length = iteration_order->length();
+    DCHECK_LE(length, dictionary->NumberOfElements());
 
     // Iterate over the dictionary using the enumeration order and update
     // the dictionary with new enumeration indices.
@@ -7527,8 +7526,8 @@ void BaseNameDictionary<Derived, Shape>::CopyEnumKeysTo(
 template <typename Derived, typename Shape>
 Handle<FixedArray> BaseNameDictionary<Derived, Shape>::IterationIndices(
     Isolate* isolate, Handle<Derived> dictionary) {
-  int length = dictionary->NumberOfElements();
-  Handle<FixedArray> array = isolate->factory()->NewFixedArray(length);
+  Handle<FixedArray> array =
+      isolate->factory()->NewFixedArray(dictionary->NumberOfElements());
   ReadOnlyRoots roots(isolate);
   int array_size = 0;
   {
@@ -7540,7 +7539,13 @@ Handle<FixedArray> BaseNameDictionary<Derived, Shape>::IterationIndices(
       array->set(array_size++, Smi::FromInt(i.as_int()));
     }
 
-    DCHECK_EQ(array_size, length);
+    // The global dictionary doesn't track its deletion count, so we may iterate
+    // fewer entries than the count of elements claimed by the dictionary.
+    if (std::is_same<Derived, GlobalDictionary>::value) {
+      DCHECK_LE(array_size, dictionary->NumberOfElements());
+    } else {
+      DCHECK_EQ(array_size, dictionary->NumberOfElements());
+    }
 
     EnumIndexComparator<Derived> cmp(raw_dictionary);
     // Use AtomicSlot wrapper to ensure that std::sort uses atomic load and
diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn
@@ -38,9 +38,7 @@ v8_executable("unittests") {
     "//testing/gtest",
   ]
 
-  data_deps = [
-    "../../tools:v8_testrunner",
-  ]
+  data_deps = [ "../../tools:v8_testrunner" ]
 
   data = [
     "testcfg.py",
@@ -212,6 +210,7 @@ v8_source_set("unittests_sources") {
     "parser/preparser-unittest.cc",
     "profiler/strings-storage-unittest.cc",
     "regress/regress-crbug-1041240-unittest.cc",
+    "regress/regress-crbug-1056054-unittest.cc",
     "regress/regress-crbug-938251-unittest.cc",
     "run-all-unittests.cc",
     "strings/char-predicates-unittest.cc",
diff --git a/test/unittests/regress/regress-crbug-1056054-unittest.cc b/test/unittests/regress/regress-crbug-1056054-unittest.cc
@@ -0,0 +1,26 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/execution/isolate.h"
+#include "src/heap/factory.h"
+#include "test/unittests/test-utils.h"
+
+namespace v8 {
+namespace internal {
+
+using EnumIndexOverflowTest = TestWithNativeContextAndZone;
+
+TEST_F(EnumIndexOverflowTest, GlobalObject) {
+  Handle<GlobalDictionary> dictionary(
+      isolate()->global_object()->global_dictionary(), isolate());
+  dictionary->set_next_enumeration_index(
+      PropertyDetails::DictionaryStorageField::kMax);
+  Handle<Object> value(Smi::FromInt(static_cast<int>(42)), isolate());
+  Handle<Name> name = factory()->InternalizeUtf8String("eeeee");
+  JSObject::AddProperty(isolate(), isolate()->global_object(), name, value,
+                        NONE);
+}
+
+}  // namespace internal
+}  // namespace v8
