[IdentityMap] Fix size if GC short-cuts objects.

BUG=chromium:704132

Change-Id: I6146c907d4f26147676f7dde4974c44fe541e8fe
Reviewed-on: https://chromium-review.googlesource.com/541362
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46059}

src/identity-map.cc

@@ -243,6 +243,13 @@ void IdentityMapBase::Rehash() {
   for (auto pair : reinsert) {
     int index = InsertKey(pair.first);
     DCHECK_GE(index, 0);
+    if (values_[index] != nullptr) {
+      // During GC some objects have been shortcut to point to the same
+      // underlying object - we can only chose one of the entries so keep the
+      // first one seen and reduce the size of the map appropriately
+      // (see crbug.com/704132 for details).
+      size_--;
+    }
     values_[index] = pair.second;
   }
 }

test/cctest/test-identity-map.cc

@@ -187,7 +187,6 @@ class IdentityMapTester : public HandleAndZoneScope {
   void Rehash() { map.Rehash(); }
 };
 
-
 TEST(Find_smi_not_found) {
   IdentityMapTester t;
   for (int i = 0; i < 100; i++) {
@@ -673,7 +672,6 @@ TEST(Collisions_7) { CollisionTest(7); }
 TEST(Resize) { CollisionTest(9, false, true); }
 TEST(Rehash) { CollisionTest(11, true, false); }
 
-
 TEST(ExplicitGC) {
   IdentityMapTester t;
   Handle<Object> num_keys[] = {t.num(2.1), t.num(2.4), t.num(3.3), t.num(4.3),
@@ -695,7 +693,6 @@ TEST(ExplicitGC) {
   }
 }
 
-
 TEST(CanonicalHandleScope) {
   Isolate* isolate = CcTest::i_isolate();
   Heap* heap = CcTest::heap();
@@ -775,5 +772,54 @@ TEST(CanonicalHandleScope) {
   }
 }
 
+TEST(GCShortCutting) {
+  IdentityMapTester t;
+  Isolate* isolate = CcTest::i_isolate();
+  Factory* factory = isolate->factory();
+  const int kDummyValue = 0;
+
+  for (int i = 0; i < 16; i++) {
+    // Insert a varying number of Smis as padding to ensure some tests straddle
+    // a boundary where the thin string short cutting will cause size_ to be
+    // greater to capacity_ if not corrected by IdentityMap
+    // (see crbug.com/704132).
+    for (int j = 0; j < i; j++) {
+      t.map.Set(t.smi(j), reinterpret_cast<void*>(kDummyValue));
+    }
+
+    Handle<String> thin_string =
+        factory->NewStringFromAsciiChecked("thin_string");
+    Handle<String> internalized_string =
+        factory->InternalizeString(thin_string);
+    DCHECK_IMPLIES(FLAG_thin_strings, thin_string->IsThinString());
+    DCHECK_NE(*thin_string, *internalized_string);
+
+    // Insert both keys into the map.
+    t.map.Set(thin_string, &thin_string);
+    t.map.Set(internalized_string, &internalized_string);
+
+    // Do an explicit, real GC, this should short-cut the thin string to point
+    // to the internalized string.
+    t.heap()->CollectGarbage(i::NEW_SPACE,
+                             i::GarbageCollectionReason::kTesting);
+    DCHECK_IMPLIES(FLAG_thin_strings && !FLAG_optimize_for_size,
+                   *thin_string == *internalized_string);
+
+    // Check that getting the object points to one of the handles.
+    void** thin_string_entry = t.map.Get(thin_string);
+    CHECK(*thin_string_entry == &thin_string ||
+          *thin_string_entry == &internalized_string);
+    void** internalized_string_entry = t.map.Get(internalized_string);
+    CHECK(*internalized_string_entry == &thin_string ||
+          *internalized_string_entry == &internalized_string);
+
+    // Trigger resize.
+    for (int j = 0; j < 16; j++) {
+      t.map.Set(t.smi(j + 16), reinterpret_cast<void*>(kDummyValue));
+    }
+    t.map.Clear();
+  }
+}
+
 }  // namespace internal
 }  // namespace v8