From 723d14dce330b0980ec22d6bf21de78ae71c98d0 Mon Sep 17 00:00:00 2001
From: pthier <pthier@chromium.org>
Date: Tue, 29 Nov 2022 16:55:29 +0100
Subject: [PATCH] Clear StubCaches after shared GC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

StubCaches contain keys (internalized strings), which are stored in
shared space if the string table is shared.
After a GC, StubCaches are simply cleared to not contain pointers to
invalid objects (e.g a key was evacuated).
This CL clears the StubCaches also for shared GCs, as keys might not be
valid afterwards.

Clearing the caches is now done in GC directly instead of invoking
Callbacks. First, GC callbacks are not supported for shared GCs (and it
is preferred to not support them) and second, this way it is guaranteed
that the caches are cleared before any other callback might access the
stub caches.

Bug: v8:13537
Change-Id: I8c9782596e4a208b07a7273ebc59544f8095474f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4064259
Commit-Queue: Patrick Thier <pthier@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84562}
---
 src/heap/heap.cc     | 20 ++++++++++++++++++++
 src/ic/stub-cache.cc | 19 -------------------
 src/ic/stub-cache.h  |  4 ----
 3 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index 7b7af30797..a79ab4514d 100644
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -2118,6 +2118,7 @@ void Heap::UpdateSurvivalStatistics(int start_new_space_size) {
 }
 
 namespace {
+
 GCTracer::Scope::ScopeId CollectorScopeId(GarbageCollector collector) {
   switch (collector) {
     case GarbageCollector::MARK_COMPACTOR:
@@ -2129,6 +2130,21 @@ GCTracer::Scope::ScopeId CollectorScopeId(GarbageCollector collector) {
   }
   UNREACHABLE();
 }
+
+void ClearStubCaches(Isolate* isolate) {
+  isolate->load_stub_cache()->Clear();
+  isolate->store_stub_cache()->Clear();
+
+  if (isolate->is_shared_heap_isolate()) {
+    isolate->global_safepoint()->IterateClientIsolates([](Isolate* client) {
+      if (client->is_shared_heap_isolate()) return;
+
+      client->load_stub_cache()->Clear();
+      client->store_stub_cache()->Clear();
+    });
+  }
+}
+
 }  // namespace
 
 size_t Heap::PerformGarbageCollection(GarbageCollector collector,
@@ -2265,6 +2281,10 @@ size_t Heap::PerformGarbageCollection(GarbageCollector collector,
 
   RecomputeLimits(collector);
 
+  if (collector == GarbageCollector::MARK_COMPACTOR) {
+    ClearStubCaches(isolate());
+  }
+
   GarbageCollectionEpilogueInSafepoint(collector);
 
   tracer()->StopInSafepoint();
diff --git a/src/ic/stub-cache.cc b/src/ic/stub-cache.cc
index 4dd60fdfa9..31d0567b9f 100644
--- a/src/ic/stub-cache.cc
+++ b/src/ic/stub-cache.cc
@@ -14,29 +14,10 @@
 namespace v8 {
 namespace internal {
 
-// static
-void StubCache::ClearCallback(v8::Isolate* isolate, v8::GCType type,
-                              v8::GCCallbackFlags flags, void* data) {
-  StubCache* cache = static_cast<StubCache*>(data);
-  cache->Clear();
-}
-
 StubCache::StubCache(Isolate* isolate) : isolate_(isolate) {
   // Ensure the nullptr (aka Smi::zero()) which StubCache::Get() returns
   // when the entry is not found is not considered as a handler.
   DCHECK(!IC::IsHandler(MaybeObject()));
-
-  // The stub caches are not traversed during GC; clear them to force
-  // their lazy re-initialization. This must be done after the
-  // GC, because it relies on the new address of certain old space
-  // objects (empty string, illegal builtin).
-
-  isolate_->heap()->AddGCEpilogueCallback(ClearCallback,
-                                          kGCTypeMarkSweepCompact, this);
-}
-
-StubCache::~StubCache() {
-  isolate_->heap()->RemoveGCEpilogueCallback(ClearCallback, this);
 }
 
 void StubCache::Initialize() {
diff --git a/src/ic/stub-cache.h b/src/ic/stub-cache.h
index 0372919f18..ce69dca2f6 100644
--- a/src/ic/stub-cache.h
+++ b/src/ic/stub-cache.h
@@ -98,12 +98,8 @@ class V8_EXPORT_PRIVATE StubCache {
   static int PrimaryOffsetForTesting(Name name, Map map);
   static int SecondaryOffsetForTesting(Name name, Map map);
 
-  static void ClearCallback(v8::Isolate* isolate, v8::GCType type,
-                            v8::GCCallbackFlags flags, void* data);
-
   // The constructor is made public only for the purposes of testing.
   explicit StubCache(Isolate* isolate);
-  ~StubCache();
   StubCache(const StubCache&) = delete;
   StubCache& operator=(const StubCache&) = delete;