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

BUG=chromium:704132

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

src/identity-map.cc

@@ -66,6 +66,8 @@ int IdentityMapBase::InsertKey(Object* address) {
     for (int index = start; --limit > 0; index = (index + 1) & mask_) {
       if (keys_[index] == address) return index;  // Found.
       if (keys_[index] == not_mapped) {           // Free entry.
+        size_++;
+        DCHECK_LE(size_, capacity_);
         keys_[index] = address;
         return index;
       }
@@ -132,8 +134,6 @@ int IdentityMapBase::LookupOrInsert(Object* key) {
     // Miss; rehash if there was a GC, then insert.
     if (gc_counter_ != heap_->gc_count()) Rehash();
     index = InsertKey(key);
-    size_++;
-    DCHECK_LE(size_, capacity_);
   }
   DCHECK_GE(index, 0);
   return index;
@@ -236,6 +236,7 @@ void IdentityMapBase::Rehash() {
         keys_[i] = not_mapped;
         values_[i] = nullptr;
         last_empty = i;
+        size_--;
       }
     }
   }
@@ -258,6 +259,7 @@ void IdentityMapBase::Resize(int new_capacity) {
   capacity_ = new_capacity;
   mask_ = capacity_ - 1;
   gc_counter_ = heap_->gc_count();
+  size_ = 0;
 
   keys_ = reinterpret_cast<Object**>(NewPointerArray(capacity_));
   Object* not_mapped = heap_->not_mapped_symbol();

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