heap,sandbox: Update EPT's evacuation entries in Scavenger.

Anton Bikineev · V8 LUCI CQ · commit 1a2b08edbec1 · 2024-08-27T11:23:09.000Z
If Scavenger interleaves MarkCompact that performs compaction on EPT, there may be some evacuation entries allocated in the young EPT that would back-point to the Scavenger's from-space. Add a new phase that updates all the evacuation entries in the young EPT up until `start_of_evacation_area`. Bug: 358485426 Change-Id: Ic23e57ff38279d4e93964cf21ee62eb01ebe8e61 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5805957 Reviewed-by: Samuel Groß <saelo@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Anton Bikineev <bikineev@chromium.org> Cr-Commit-Position: refs/heads/main@{#95827}
diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc
@@ -498,6 +498,68 @@ void IncrementalMarking::UpdateMarkingWorklistAfterScavenge() {
   weak_objects_->UpdateAfterScavenge();
 }
 
+void IncrementalMarking::UpdateExternalPointerTableAfterScavenge() {
+#ifdef V8_COMPRESS_POINTERS
+  if (!IsMajorMarking()) return;
+  DCHECK(!v8_flags.separate_gc_phases);
+
+  heap_->isolate()->external_pointer_table().UpdateAllEvacuationEntries(
+      heap_->young_external_pointer_space(), [](Address old_handle_location) {
+        // 1) Resolve object start from the marking bitmap. Note that it's safe
+        //    since there is no black allocation for the young space (and hence
+        //    no range or page marking).
+        // 2) Get a relocated object from the forwaring reference stored in the
+        //    map.
+        // 3) Compute offset from the original object start to the handle
+        //    location.
+        // 4) Compute and return the new handle location.
+        //
+        // Please note that instead of updating the evacuation entries, we
+        // could simply clobber them all, which would still work, but limit
+        // compaction to some extent. We can reconsider this in the future, if
+        // relying on the marking bitmap becomes an issue (e.g. with inlined
+        // mark-bits).
+        const MemoryChunk* chunk =
+            MemoryChunk::FromAddress(old_handle_location);
+        if (!chunk->InYoungGeneration()) {
+          return old_handle_location;
+        }
+        // TODO(358485426): Check that the page is not black.
+
+        Address base = MarkingBitmap::FindPreviousValidObject(
+            static_cast<const PageMetadata*>(chunk->Metadata()),
+            old_handle_location);
+        Tagged<HeapObject> object(HeapObject::FromAddress(base));
+
+        MapWord map_word = object->map_word(kRelaxedLoad);
+        if (!map_word.IsForwardingAddress()) {
+      // There may be objects in the EPT that do not exist anymore. If these
+      // objects are dead at scavenging time, their marking deque entries will
+      // not point to forwarding addresses. Hence, we can discard them.
+#if DEBUG
+          // Check that the handle did reside inside the original dead object.
+          const int object_size = object->Size();
+          // Map slots can never contain external pointers.
+          DCHECK_LT(object.address(), old_handle_location);
+          DCHECK_LT(old_handle_location, object.address() + object_size);
+#endif  // DEBUG
+          return kNullAddress;
+        }
+
+        Tagged<HeapObject> moved_object = map_word.ToForwardingAddress(object);
+#if DEBUG
+        const int object_size = moved_object->Size();
+        // Map slots can never contain external pointers.
+        DCHECK_LT(object.address(), old_handle_location);
+        DCHECK_LT(old_handle_location, object.address() + object_size);
+#endif  // DEBUG
+
+        const ptrdiff_t handle_offset = old_handle_location - base;
+        return moved_object.address() + handle_offset;
+      });
+#endif  // V8_COMPRESS_POINTERS
+}
+
 void IncrementalMarking::UpdateMarkedBytesAfterScavenge(
     size_t dead_bytes_in_new_space) {
   if (!IsMajorMarking()) return;
diff --git a/src/heap/incremental-marking.h b/src/heap/incremental-marking.h
@@ -102,6 +102,7 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
   bool Stop();
 
   void UpdateMarkingWorklistAfterScavenge();
+  void UpdateExternalPointerTableAfterScavenge();
   void UpdateMarkedBytesAfterScavenge(size_t dead_bytes_in_new_space);
 
   // Performs incremental marking step and finalizes marking if complete.
diff --git a/src/heap/scavenger.cc b/src/heap/scavenger.cc
@@ -506,6 +506,7 @@ void ScavengerCollector::CollectGarbage() {
         &Heap::UpdateYoungReferenceInExternalStringTableEntry);
 
     heap_->incremental_marking()->UpdateMarkingWorklistAfterScavenge();
+    heap_->incremental_marking()->UpdateExternalPointerTableAfterScavenge();
 
     if (V8_UNLIKELY(v8_flags.always_use_string_forwarding_table)) {
       isolate_->string_forwarding_table()->UpdateAfterYoungEvacuation();
diff --git a/src/sandbox/compactible-external-entity-table.h b/src/sandbox/compactible-external-entity-table.h
@@ -42,7 +42,7 @@ enum class ExternalEntityTableCompactionOutcome {
  *    compacted. This decision is mostly based on the absolute and relative
  *    size of the freelist.
  *  - If compaction is needed, this algorithm determines by how many segments
- *    it would like to shrink the space (N). It will then attempts to move all
+ *    it would like to shrink the space (N). It will then attempt to move all
  *    live entries out of these segments so that they can be deallocated
  *    afterwards during sweeping.
  *  - The algorithm then simply selects the last N segments for evacuation, and
diff --git a/src/sandbox/external-pointer-table.cc b/src/sandbox/external-pointer-table.cc
@@ -42,7 +42,7 @@ class SegmentsIterator {
   using const_iterator = typename std::set<Segment>::const_reverse_iterator;
 
  public:
-  SegmentsIterator() {}
+  SegmentsIterator() = default;
 
   void AddSegments(const std::set<Segment>& segments, Data data) {
     streams_.emplace_back(segments.rbegin(), segments.rend(), data);
@@ -126,7 +126,7 @@ uint32_t ExternalPointerTable::EvacuateAndSweepAndCompact(Space* space,
     segments_iter.AddSegments(from_space_segments, from_space_compaction);
 
     FreelistHead empty_freelist;
-    from_space->freelist_head_.store(empty_freelist, std::memory_order_release);
+    from_space->freelist_head_.store(empty_freelist, std::memory_order_relaxed);
 
     for (Address field : from_space->invalidated_fields_)
       space->invalidated_fields_.push_back(field);
@@ -176,6 +176,13 @@ uint32_t ExternalPointerTable::EvacuateAndSweepAndCompact(Space* space,
         Address handle_location =
             payload.ExtractEvacuationEntryHandleLocation();
 
+        // The evacuation entry may be invalidated by the Scavenger that has
+        // freed the object.
+        if (handle_location == kNullAddress) {
+          AddToFreelist(i);
+          continue;
+        }
+
         // The external pointer field may have been invalidated in the meantime
         // (for example if the host object has been in-place converted to a
         // different type of object). In that case, the field no longer
@@ -295,6 +302,40 @@ void ExternalPointerTable::ResolveEvacuationEntryDuringSweeping(
   }
 }
 
+void ExternalPointerTable::UpdateAllEvacuationEntries(
+    Space* space, std::function<Address(Address)> function) {
+  DCHECK(space->BelongsTo(this));
+  DCHECK(!space->is_internal_read_only_space());
+
+  if (!space->IsCompacting()) return;
+
+  // Lock the space. Technically this is not necessary since no other thread can
+  // allocate entries at this point, but some of the methods we call on the
+  // space assert that the lock is held.
+  base::MutexGuard guard(&space->mutex_);
+  // Same for the invalidated fields mutex.
+  base::MutexGuard invalidated_fields_guard(&space->invalidated_fields_mutex_);
+
+  const uint32_t start_of_evacuation_area =
+      space->start_of_evacuation_area_.load(std::memory_order_relaxed);
+
+  // Iterate until the start of evacuation area.
+  for (auto& segment : space->segments_) {
+    if (segment.first_entry() == start_of_evacuation_area) return;
+    for (uint32_t i = segment.first_entry(); i < segment.last_entry() + 1;
+         ++i) {
+      ExternalPointerTableEntry& entry = at(i);
+      ExternalPointerTableEntry::Payload payload = entry.GetRawPayload();
+      if (!payload.ContainsEvacuationEntry()) {
+        continue;
+      }
+      Address new_location =
+          function(payload.ExtractEvacuationEntryHandleLocation());
+      entry.MakeEvacuationEntry(new_location);
+    }
+  }
+}
+
 }  // namespace internal
 }  // namespace v8
 
diff --git a/src/sandbox/external-pointer-table.h b/src/sandbox/external-pointer-table.h
@@ -351,6 +351,10 @@ class V8_EXPORT_PRIVATE ExternalPointerTable
   uint32_t SweepAndCompact(Space* space, Counters* counters);
   uint32_t Sweep(Space* space, Counters* counters);
 
+  // Updates all evacuation entries with new handle locations. The function
+  // takes the old hanlde location and returns the new one.
+  void UpdateAllEvacuationEntries(Space*, std::function<Address(Address)>);
+
   inline bool Contains(Space* space, ExternalPointerHandle handle) const;
 
   // A resource outside of the V8 heap whose lifetime is tied to something
