Fix clearing of slots on large page uncommit

BUG=chromium:624544
LOG=NO

Review-Url: https://codereview.chromium.org/2110213003
Cr-Commit-Position: refs/heads/master@{#37451}

src/heap/remembered-set.h

@@ -21,39 +21,65 @@ class RememberedSet {
  public:
   // Given a page and a slot in that page, this function adds the slot to the
   // remembered set.
-  static void Insert(Page* page, Address slot_addr) {
-    DCHECK(page->Contains(slot_addr));
-    SlotSet* slot_set = GetSlotSet(page);
+  static void Insert(MemoryChunk* chunk, Address slot_addr) {
+    DCHECK(chunk->Contains(slot_addr));
+    SlotSet* slot_set = GetSlotSet(chunk);
     if (slot_set == nullptr) {
-      slot_set = AllocateSlotSet(page);
+      slot_set = AllocateSlotSet(chunk);
     }
-    uintptr_t offset = slot_addr - page->address();
+    uintptr_t offset = slot_addr - chunk->address();
     slot_set[offset / Page::kPageSize].Insert(offset % Page::kPageSize);
   }
 
   // Given a page and a slot in that page, this function removes the slot from
   // the remembered set.
   // If the slot was never added, then the function does nothing.
-  static void Remove(Page* page, Address slot_addr) {
-    DCHECK(page->Contains(slot_addr));
-    SlotSet* slot_set = GetSlotSet(page);
+  static void Remove(MemoryChunk* chunk, Address slot_addr) {
+    DCHECK(chunk->Contains(slot_addr));
+    SlotSet* slot_set = GetSlotSet(chunk);
     if (slot_set != nullptr) {
-      uintptr_t offset = slot_addr - page->address();
+      uintptr_t offset = slot_addr - chunk->address();
       slot_set[offset / Page::kPageSize].Remove(offset % Page::kPageSize);
     }
   }
 
   // Given a page and a range of slots in that page, this function removes the
   // slots from the remembered set.
-  static void RemoveRange(Page* page, Address start, Address end) {
-    SlotSet* slot_set = GetSlotSet(page);
+  static void RemoveRange(MemoryChunk* chunk, Address start, Address end) {
+    SlotSet* slot_set = GetSlotSet(chunk);
     if (slot_set != nullptr) {
-      uintptr_t start_offset = start - page->address();
-      uintptr_t end_offset = end - page->address();
+      uintptr_t start_offset = start - chunk->address();
+      uintptr_t end_offset = end - chunk->address();
       DCHECK_LT(start_offset, end_offset);
-      DCHECK_LE(end_offset, static_cast<uintptr_t>(Page::kPageSize));
-      slot_set->RemoveRange(static_cast<uint32_t>(start_offset),
-                            static_cast<uint32_t>(end_offset));
+      if (end_offset < static_cast<uintptr_t>(Page::kPageSize)) {
+        slot_set->RemoveRange(static_cast<int>(start_offset),
+                              static_cast<int>(end_offset));
+      } else {
+        // The large page has multiple slot sets.
+        // Compute slot set indicies for the range [start_offset, end_offset).
+        int start_chunk = static_cast<int>(start_offset / Page::kPageSize);
+        int end_chunk = static_cast<int>((end_offset - 1) / Page::kPageSize);
+        int offset_in_start_chunk =
+            static_cast<int>(start_offset % Page::kPageSize);
+        // Note that using end_offset % Page::kPageSize would be incorrect
+        // because end_offset is one beyond the last slot to clear.
+        int offset_in_end_chunk = static_cast<int>(
+            end_offset - static_cast<uintptr_t>(end_chunk) * Page::kPageSize);
+        if (start_chunk == end_chunk) {
+          slot_set[start_chunk].RemoveRange(offset_in_start_chunk,
+                                            offset_in_end_chunk);
+        } else {
+          // Clear all slots from start_offset to the end of first chunk.
+          slot_set[start_chunk].RemoveRange(offset_in_start_chunk,
+                                            Page::kPageSize);
+          // Clear all slots in intermediate chunks.
+          for (int i = start_chunk + 1; i < end_chunk; i++) {
+            slot_set[i].RemoveRange(0, Page::kPageSize);
+          }
+          // Clear slots from the beginning of the last page to end_offset.
+          slot_set[end_chunk].RemoveRange(0, offset_in_end_chunk);
+        }
+      }
     }
   }
 

src/heap/slot-set.h

@@ -64,6 +64,7 @@ class SlotSet : public Malloced {
   // The slot offsets specify a range of slots at addresses:
   // [page_start_ + start_offset ... page_start_ + end_offset).
   void RemoveRange(int start_offset, int end_offset) {
+    CHECK_LE(end_offset, 1 << kPageSizeBits);
     DCHECK_LE(start_offset, end_offset);
     int start_bucket, start_cell, start_bit;
     SlotToIndices(start_offset, &start_bucket, &start_cell, &start_bit);

src/heap/spaces.cc

@@ -2774,16 +2774,10 @@ Address LargePage::GetAddressToShrink() {
 }
 
 void LargePage::ClearOutOfLiveRangeSlots(Address free_start) {
-  if (old_to_new_slots() != nullptr) {
-    old_to_new_slots()->RemoveRange(
-        static_cast<int>(free_start - address()),
-        static_cast<int>(free_start + size() - address()));
-  }
-  if (old_to_old_slots() != nullptr) {
-    old_to_old_slots()->RemoveRange(
-        static_cast<int>(free_start - address()),
-        static_cast<int>(free_start + size() - address()));
-  }
+  RememberedSet<OLD_TO_NEW>::RemoveRange(this, free_start, area_end());
+  RememberedSet<OLD_TO_OLD>::RemoveRange(this, free_start, area_end());
+  RememberedSet<OLD_TO_NEW>::RemoveRangeTyped(this, free_start, area_end());
+  RememberedSet<OLD_TO_OLD>::RemoveRangeTyped(this, free_start, area_end());
 }
 
 // -----------------------------------------------------------------------------

test/cctest/heap/test-heap.cc

@@ -6814,5 +6814,67 @@ TEST(UncommitUnusedLargeObjectMemory) {
               base::OS::CommitPageSize());
   CHECK_EQ(shrinked_size, chunk->CommittedPhysicalMemory());
 }
+
+TEST(RememberedSetRemoveRange) {
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  Heap* heap = CcTest::heap();
+  Isolate* isolate = heap->isolate();
+
+  Handle<FixedArray> array = isolate->factory()->NewFixedArray(500000);
+  MemoryChunk* chunk = MemoryChunk::FromAddress(array->address());
+  CHECK(chunk->owner()->identity() == LO_SPACE);
+  Address start = array->address();
+  // Maps slot to boolean indicator of whether the slot should be in the set.
+  std::map<Address, bool> slots;
+  slots[start + 0] = true;
+  slots[start + kPointerSize] = true;
+  slots[start + Page::kPageSize - kPointerSize] = true;
+  slots[start + Page::kPageSize] = true;
+  slots[start + Page::kPageSize + kPointerSize] = true;
+  slots[chunk->area_end() - kPointerSize] = true;
+
+  for (auto x : slots) {
+    RememberedSet<OLD_TO_NEW>::Insert(chunk, x.first);
+  }
+
+  RememberedSet<OLD_TO_NEW>::Iterate(chunk, [&slots](Address addr) {
+    CHECK(slots[addr]);
+    return KEEP_SLOT;
+  });
+
+  RememberedSet<OLD_TO_NEW>::RemoveRange(chunk, start, start + kPointerSize);
+  slots[start] = false;
+  RememberedSet<OLD_TO_NEW>::Iterate(chunk, [&slots](Address addr) {
+    CHECK(slots[addr]);
+    return KEEP_SLOT;
+  });
+
+  RememberedSet<OLD_TO_NEW>::RemoveRange(chunk, start + kPointerSize,
+                                         start + Page::kPageSize);
+  slots[start + kPointerSize] = false;
+  slots[start + Page::kPageSize - kPointerSize] = false;
+  RememberedSet<OLD_TO_NEW>::Iterate(chunk, [&slots](Address addr) {
+    CHECK(slots[addr]);
+    return KEEP_SLOT;
+  });
+
+  RememberedSet<OLD_TO_NEW>::RemoveRange(
+      chunk, start, start + Page::kPageSize + kPointerSize);
+  slots[start + Page::kPageSize] = false;
+  RememberedSet<OLD_TO_NEW>::Iterate(chunk, [&slots](Address addr) {
+    CHECK(slots[addr]);
+    return KEEP_SLOT;
+  });
+
+  RememberedSet<OLD_TO_NEW>::RemoveRange(
+      chunk, chunk->area_end() - kPointerSize, chunk->area_end());
+  slots[chunk->area_end() - kPointerSize] = false;
+  RememberedSet<OLD_TO_NEW>::Iterate(chunk, [&slots](Address addr) {
+    CHECK(slots[addr]);
+    return KEEP_SLOT;
+  });
+}
+
 }  // namespace internal
 }  // namespace v8