Reland "Reland "[heap] Support client-to-shared refs in Code objects""

This is a reland of 4b8f1b1c

After landing https://crrev.com/c/3447371, we can reland this CL as-is
correctness-wise.

What's new in this CL is that we now treat references from client
objects into the shared heap as roots for the --track-retaining-path
feature.

Original change's description:
> Reland "[heap] Support client-to-shared refs in Code objects"
>
> This is a reland of 12e46091
>
> Original change's description:
> > [heap] Support client-to-shared refs in Code objects
> >
> > Support references from code objects in the client heaps to shared heap objects. Such references are stored in a remembered set during marking, which is later used for updating pointers.
> >
> > Bug: v8:11708
> > Change-Id: I8aeb508ddd14514ca65fa5acf3030dd8c2040168
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3401588
> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> > Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#78819}
>
> Bug: v8:11708
> Change-Id: I47bcf44b452fcffe8675fba03244b736ede14247
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3422630
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#78838}

Bug: v8:11708
Change-Id: I5b48e942fa469eabb40e797e221d06c25af16443
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3425358Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79023}

src/d8/d8.cc

@@ -50,6 +50,7 @@
 #include "src/execution/vm-state-inl.h"
 #include "src/flags/flags.h"
 #include "src/handles/maybe-handles.h"
+#include "src/heap/parked-scope.h"
 #include "src/init/v8.h"
 #include "src/interpreter/interpreter.h"
 #include "src/logging/counters.h"
@@ -4626,6 +4627,12 @@ int Shell::RunMain(Isolate* isolate, bool last_run) {
     }
   }
   CollectGarbage(isolate);
+
+  // Park the main thread here to prevent deadlocks in shared GCs when waiting
+  // in JoinThread.
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  i::ParkedScope parked(i_isolate->main_thread_local_isolate());
+
   for (int i = 1; i < options.num_isolates; ++i) {
     if (last_run) {
       options.isolate_sources[i].JoinThread();

src/heap/heap.cc

@@ -2385,8 +2385,9 @@ void Heap::PerformSharedGarbageCollection(Isolate* initiator,
   v8::Locker locker(reinterpret_cast<v8::Isolate*>(isolate()));
   v8::Isolate::Scope isolate_scope(reinterpret_cast<v8::Isolate*>(isolate()));
 
-  tracer()->StartObservablePause(GarbageCollector::MARK_COMPACTOR, gc_reason,
-                                 nullptr);
+  const GarbageCollector collector = GarbageCollector::MARK_COMPACTOR;
+
+  tracer()->StartObservablePause(collector, gc_reason, nullptr);
 
   DCHECK_NOT_NULL(isolate()->global_safepoint());
 
@@ -2396,12 +2397,23 @@ void Heap::PerformSharedGarbageCollection(Isolate* initiator,
     // As long as we need to iterate the client heap to find references into the
     // shared heap, all client heaps need to be iterable.
     client->heap()->MakeHeapIterable();
+
+    if (FLAG_concurrent_marking) {
+      client->heap()->concurrent_marking()->Pause();
+    }
   });
 
   PerformGarbageCollection(GarbageCollector::MARK_COMPACTOR, gc_reason,
                            nullptr);
 
-  tracer()->StopObservablePause(GarbageCollector::MARK_COMPACTOR);
+  tracer()->StopObservablePause(collector);
+
+  isolate()->global_safepoint()->IterateClientIsolates([](Isolate* client) {
+    if (FLAG_concurrent_marking &&
+        client->heap()->incremental_marking()->IsMarking()) {
+      client->heap()->concurrent_marking()->RescheduleJobIfNeeded();
+    }
+  });
 }
 
 void Heap::CompleteSweepingYoung(GarbageCollector collector) {

src/heap/mark-compact.cc

@@ -34,6 +34,7 @@
 #include "src/heap/marking-barrier.h"
 #include "src/heap/marking-visitor-inl.h"
 #include "src/heap/marking-visitor.h"
+#include "src/heap/memory-chunk-layout.h"
 #include "src/heap/memory-measurement-inl.h"
 #include "src/heap/memory-measurement.h"
 #include "src/heap/object-stats.h"
@@ -1177,7 +1178,9 @@ class MarkCompactCollector::CustomRootBodyMarkingVisitor final
  private:
   V8_INLINE void MarkObject(HeapObject host, Object object) {
     if (!object.IsHeapObject()) return;
-    collector_->MarkObject(host, HeapObject::cast(object));
+    HeapObject heap_object = HeapObject::cast(object);
+    if (!collector_->is_shared_heap() && heap_object.InSharedHeap()) return;
+    collector_->MarkObject(host, heap_object);
   }
 
   MarkCompactCollector* const collector_;
@@ -1229,28 +1232,38 @@ class MarkCompactCollector::SharedHeapObjectVisitor final
   }
 
   void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
-#if DEBUG
     Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
-    DCHECK(!BasicMemoryChunk::FromHeapObject(target)->InSharedHeap());
-#endif  // DEBUG
+    RecordRelocSlot(host, rinfo, target);
   }
 
   void VisitEmbeddedPointer(Code host, RelocInfo* rinfo) override {
-#if DEBUG
     HeapObject target = rinfo->target_object(cage_base());
-    DCHECK(!BasicMemoryChunk::FromHeapObject(target)->InSharedHeap());
-#endif  // DEBUG
+    RecordRelocSlot(host, rinfo, target);
   }
 
  private:
   V8_INLINE void MarkObject(HeapObject host, ObjectSlot slot, Object object) {
-    DCHECK(!BasicMemoryChunk::FromHeapObject(host)->InSharedHeap());
+    DCHECK(!host.InSharedHeap());
     if (!object.IsHeapObject()) return;
     HeapObject heap_object = HeapObject::cast(object);
-    if (!BasicMemoryChunk::FromHeapObject(heap_object)->InSharedHeap()) return;
+    if (!heap_object.InSharedHeap()) return;
     RememberedSet<CLIENT_TO_SHARED>::Insert<AccessMode::NON_ATOMIC>(
         MemoryChunk::FromHeapObject(host), slot.address());
-    collector_->MarkObject(host, heap_object);
+    collector_->MarkRootObject(Root::kClientHeap, heap_object);
+  }
+
+  V8_INLINE void RecordRelocSlot(Code host, RelocInfo* rinfo,
+                                 HeapObject target) {
+    if (ShouldRecordRelocSlot(host, rinfo, target)) {
+      RecordRelocSlotInfo info = ProcessRelocInfo(host, rinfo, target);
+      RememberedSet<CLIENT_TO_SHARED>::InsertTyped(info.memory_chunk,
+                                                   info.slot_type, info.offset);
+    }
+  }
+
+  V8_INLINE bool ShouldRecordRelocSlot(Code host, RelocInfo* rinfo,
+                                       HeapObject target) {
+    return BasicMemoryChunk::FromHeapObject(target)->InSharedHeap();
   }
 
   MarkCompactCollector* const collector_;
@@ -4622,6 +4635,8 @@ void MarkCompactCollector::UpdatePointersInClientHeap(Isolate* client) {
 
   while (chunk_iterator.HasNext()) {
     MemoryChunk* chunk = chunk_iterator.Next();
+    CodePageMemoryModificationScope unprotect_code_page(chunk);
+
     RememberedSet<CLIENT_TO_SHARED>::Iterate(
         chunk,
         [cage_base](MaybeObjectSlot slot) {
@@ -4630,6 +4645,20 @@ void MarkCompactCollector::UpdatePointersInClientHeap(Isolate* client) {
         SlotSet::KEEP_EMPTY_BUCKETS);
 
     chunk->ReleaseSlotSet<CLIENT_TO_SHARED>();
+
+    RememberedSet<CLIENT_TO_SHARED>::IterateTyped(
+        chunk, [this](SlotType slot_type, Address slot) {
+          // Using UpdateStrongSlot is OK here, because there are no weak
+          // typed slots.
+          PtrComprCageBase cage_base = heap_->isolate();
+          return UpdateTypedSlotHelper::UpdateTypedSlot(
+              heap_, slot_type, slot, [cage_base](FullMaybeObjectSlot slot) {
+                return UpdateStrongSlot<AccessMode::NON_ATOMIC>(cage_base,
+                                                                slot);
+              });
+        });
+
+    chunk->ReleaseTypedSlotSet<CLIENT_TO_SHARED>();
   }
 
 #ifdef VERIFY_HEAP

src/heap/marking-visitor-inl.h

@@ -43,8 +43,7 @@ template <typename THeapObjectSlot>
 void MarkingVisitorBase<ConcreteVisitor, MarkingState>::ProcessStrongHeapObject(
     HeapObject host, THeapObjectSlot slot, HeapObject heap_object) {
   concrete_visitor()->SynchronizePageAccess(heap_object);
-  BasicMemoryChunk* target_page = BasicMemoryChunk::FromHeapObject(heap_object);
-  if (!is_shared_heap_ && target_page->InSharedHeap()) return;
+  if (!is_shared_heap_ && heap_object.InSharedHeap()) return;
   MarkObject(host, heap_object);
   concrete_visitor()->RecordSlot(host, slot, heap_object);
 }

src/heap/memory-chunk.cc

@@ -9,6 +9,7 @@
 #include "src/heap/code-object-registry.h"
 #include "src/heap/memory-allocator.h"
 #include "src/heap/memory-chunk-inl.h"
+#include "src/heap/memory-chunk-layout.h"
 #include "src/heap/spaces.h"
 #include "src/objects/heap-object.h"
 
@@ -133,6 +134,8 @@ MemoryChunk* MemoryChunk::Initialize(BasicMemoryChunk* basic_chunk, Heap* heap,
                                        nullptr);
   base::AsAtomicPointer::Release_Store(&chunk->typed_slot_set_[OLD_TO_OLD],
                                        nullptr);
+  base::AsAtomicPointer::Release_Store(
+      &chunk->typed_slot_set_[CLIENT_TO_SHARED], nullptr);
   chunk->invalidated_slots_[OLD_TO_NEW] = nullptr;
   chunk->invalidated_slots_[OLD_TO_OLD] = nullptr;
   if (V8_EXTERNAL_CODE_SPACE_BOOL) {
@@ -313,6 +316,7 @@ void MemoryChunk::ReleaseSlotSet(SlotSet** slot_set) {
 
 template TypedSlotSet* MemoryChunk::AllocateTypedSlotSet<OLD_TO_NEW>();
 template TypedSlotSet* MemoryChunk::AllocateTypedSlotSet<OLD_TO_OLD>();
+template TypedSlotSet* MemoryChunk::AllocateTypedSlotSet<CLIENT_TO_SHARED>();
 
 template <RememberedSetType type>
 TypedSlotSet* MemoryChunk::AllocateTypedSlotSet() {
@@ -329,6 +333,7 @@ TypedSlotSet* MemoryChunk::AllocateTypedSlotSet() {
 
 template void MemoryChunk::ReleaseTypedSlotSet<OLD_TO_NEW>();
 template void MemoryChunk::ReleaseTypedSlotSet<OLD_TO_OLD>();
+template void MemoryChunk::ReleaseTypedSlotSet<CLIENT_TO_SHARED>();
 
 template <RememberedSetType type>
 void MemoryChunk::ReleaseTypedSlotSet() {

src/objects/visitors.h

@@ -42,6 +42,7 @@ class CodeDataContainer;
   V(kWrapperTracing, "(Wrapper tracing)")               \
   V(kWriteBarrier, "(Write barrier)")                   \
   V(kRetainMaps, "(Retain maps)")                       \
+  V(kClientHeap, "(Client heap)")                       \
   V(kUnknown, "(Unknown)")
 
 class VisitorSynchronization : public AllStatic {

src/runtime/runtime-test.cc

@@ -1476,5 +1476,11 @@ RUNTIME_FUNCTION(Runtime_IsSharedString) {
                                     Handle<String>::cast(obj)->IsShared());
 }
 
+RUNTIME_FUNCTION(Runtime_SharedGC) {
+  SealHandleScope scope(isolate);
+  isolate->heap()->CollectSharedGarbage(GarbageCollectionReason::kTesting);
+  return ReadOnlyRoots(isolate).undefined_value();
+}
+
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime.h

@@ -545,6 +545,7 @@ namespace internal {
   F(SetAllocationTimeout, -1 /* 2 || 3 */, 1) \
   F(SetForceSlowPath, 1, 1)                   \
   F(SetIteratorProtector, 0, 1)               \
+  F(SharedGC, 0, 1)                           \
   F(SimulateNewspaceFull, 0, 1)               \
   F(StringIteratorProtector, 0, 1)            \
   F(SystemBreak, 0, 1)                        \

test/mjsunit/shared-memory/shared-string-in-code-object.js

+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Flags: --shared-string-table --allow-natives-syntax --stress-compaction
+
+function foo() { return "foo"; }
+
+%PrepareFunctionForOptimization(foo);
+let value = foo();
+assertTrue(%IsSharedString(value));
+%OptimizeFunctionOnNextCall(foo);
+value = foo();
+assertTrue(%IsSharedString(value));
+%SharedGC();
+value = foo();
+assertTrue(%IsSharedString(value));
+assertEquals("foo", value);