[heap] Fix remaining test failures with --shared-heap

This CL fixes the remaining test failures when running test with the
--shared-heap flag locally:

* Remove uses of shared_isolate()
* Fix slot recording in Mark-Compact and Scavenger
* Fixes DCHECKs in tests that do not hold with --shared-heap

Bug: v8:13267
Change-Id: I6869ece70f1e6156d9bb1281e6cd876cf8d471eb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3918377
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83530}

src/execution/isolate.h

@@ -2005,14 +2005,14 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   // Returns true when this isolate supports allocation in shared spaces.
   bool has_shared_heap() const {
-    return shared_isolate() || shared_space_isolate();
+    return v8_flags.shared_space ? shared_space_isolate() : shared_isolate();
   }
 
   // Returns the isolate that owns the shared spaces.
   Isolate* shared_heap_isolate() const {
     DCHECK(has_shared_heap());
     Isolate* isolate =
-        shared_isolate() ? shared_isolate() : shared_space_isolate();
+        v8_flags.shared_space ? shared_space_isolate() : shared_isolate();
     DCHECK_NOT_NULL(isolate);
     return isolate;
   }

src/heap/mark-compact.cc

@@ -1792,7 +1792,7 @@ class EvacuateVisitorBase : public HeapObjectVisitor {
       if (V8_UNLIKELY(v8_flags.minor_mc)) {
         base->record_visitor_->MarkArrayBufferExtensionPromoted(dst);
       }
-    } else if (dest == MAP_SPACE) {
+    } else if (dest == MAP_SPACE || dest == SHARED_SPACE) {
       DCHECK_OBJECT_SIZE(size);
       DCHECK(IsAligned(size, kTaggedSize));
       base->heap_->CopyBlock(dst_addr, src_addr, size);
@@ -1846,8 +1846,7 @@ class EvacuateVisitorBase : public HeapObjectVisitor {
     Map map = object.map(cage_base());
     AllocationAlignment alignment = HeapObject::RequiredAlignment(map);
     AllocationResult allocation;
-    if (ShouldPromoteIntoSharedHeap(map)) {
-      DCHECK_EQ(target_space, OLD_SPACE);
+    if (target_space == OLD_SPACE && ShouldPromoteIntoSharedHeap(map)) {
       DCHECK_NOT_NULL(shared_old_allocator_);
       allocation = shared_old_allocator_->AllocateRaw(size, alignment,
                                                       AllocationOrigin::kGC);
@@ -4871,9 +4870,7 @@ class RememberedSetUpdatingItem : public UpdatingItem {
         marking_state_(marking_state),
         chunk_(chunk),
         updating_mode_(updating_mode),
-        record_old_to_shared_slots_(
-            heap->isolate()->has_shared_heap() &&
-            !heap->isolate()->is_shared_space_isolate()) {}
+        record_old_to_shared_slots_(heap->isolate()->has_shared_heap()) {}
   ~RememberedSetUpdatingItem() override = default;
 
   void Process() override {

src/heap/scavenger.cc

@@ -672,19 +672,22 @@ void Scavenger::AddPageToSweeperIfNecessary(MemoryChunk* page) {
 
 void Scavenger::ScavengePage(MemoryChunk* page) {
   CodePageMemoryModificationScope memory_modification_scope(page);
-  const bool has_shared_isolate = heap_->isolate()->shared_isolate();
+  const bool record_old_to_shared_slots = heap_->isolate()->has_shared_heap();
 
   if (page->slot_set<OLD_TO_NEW, AccessMode::ATOMIC>() != nullptr) {
     InvalidatedSlotsFilter filter = InvalidatedSlotsFilter::OldToNew(
         page, InvalidatedSlotsFilter::LivenessCheck::kNo);
     RememberedSet<OLD_TO_NEW>::IterateAndTrackEmptyBuckets(
         page,
-        [this, page, has_shared_isolate, &filter](MaybeObjectSlot slot) {
+        [this, page, record_old_to_shared_slots,
+         &filter](MaybeObjectSlot slot) {
           if (!filter.IsValid(slot.address())) return REMOVE_SLOT;
           SlotCallbackResult result = CheckAndScavengeObject(heap_, slot);
           // A new space string might have been promoted into the shared heap
           // during GC.
-          if (has_shared_isolate) CheckOldToNewSlotForSharedUntyped(page, slot);
+          if (record_old_to_shared_slots) {
+            CheckOldToNewSlotForSharedUntyped(page, slot);
+          }
           return result;
         },
         &empty_chunks_local_);
@@ -701,11 +704,11 @@ void Scavenger::ScavengePage(MemoryChunk* page) {
         return UpdateTypedSlotHelper::UpdateTypedSlot(
             heap_, slot_type, slot_address,
             [this, page, slot_type, slot_address,
-             has_shared_isolate](FullMaybeObjectSlot slot) {
+             record_old_to_shared_slots](FullMaybeObjectSlot slot) {
               SlotCallbackResult result = CheckAndScavengeObject(heap(), slot);
               // A new space string might have been promoted into the shared
               // heap during GC.
-              if (has_shared_isolate) {
+              if (record_old_to_shared_slots) {
                 CheckOldToNewSlotForSharedTyped(page, slot_type, slot_address);
               }
               return result;

src/init/bootstrapper.cc

@@ -4673,8 +4673,11 @@ void Genesis::InitializeGlobal_harmony_struct() {
         isolate()->factory()->NewDescriptorArray(1, 0,
                                                  AllocationType::kSharedOld);
     Descriptor descriptor = Descriptor::AccessorConstant(
-        isolate()->shared_isolate()->factory()->length_string(),
-        isolate()->shared_isolate()->factory()->shared_array_length_accessor(),
+        isolate()->shared_heap_isolate()->factory()->length_string(),
+        isolate()
+            ->shared_heap_isolate()
+            ->factory()
+            ->shared_array_length_accessor(),
         ALL_ATTRIBUTES_MASK);
     descriptors->Set(InternalIndex(0), &descriptor);
     shared_array_fun->initial_map().InitializeDescriptors(isolate(),

src/objects/value-serializer.cc

@@ -1096,7 +1096,7 @@ Maybe<bool> ValueSerializer::WriteWasmMemory(Handle<WasmMemoryObject> object) {
 #endif  // V8_ENABLE_WEBASSEMBLY
 
 Maybe<bool> ValueSerializer::WriteSharedObject(Handle<HeapObject> object) {
-  if (!delegate_ || isolate_->shared_isolate() == nullptr) {
+  if (!delegate_ || !isolate_->has_shared_heap()) {
     return ThrowDataCloneError(MessageTemplate::kDataCloneError, object);
   }
 

src/snapshot/shared-heap-serializer.cc

@@ -95,7 +95,7 @@ bool SharedHeapSerializer::SerializeUsingSharedHeapObjectCache(
   // not present in the startup snapshot to be serialized.
   if (ShouldReconstructSharedHeapObjectCacheForTesting()) {
     std::vector<Object>* existing_cache =
-        isolate()->shared_isolate()->shared_heap_object_cache();
+        isolate()->shared_heap_isolate()->shared_heap_object_cache();
     const size_t existing_cache_size = existing_cache->size();
     // This is strictly < because the existing cache contains the terminating
     // undefined value, which the reconstructed cache does not.
@@ -201,12 +201,12 @@ bool SharedHeapSerializer::ShouldReconstructSharedHeapObjectCacheForTesting()
   // need to reconstruct the shared heap object cache because it is not actually
   // shared.
   return reconstruct_read_only_and_shared_object_caches_for_testing() &&
-         isolate()->shared_isolate() != nullptr;
+         isolate()->has_shared_heap();
 }
 
 void SharedHeapSerializer::ReconstructSharedHeapObjectCacheForTesting() {
   std::vector<Object>* cache =
-      isolate()->shared_isolate()->shared_heap_object_cache();
+      isolate()->shared_heap_isolate()->shared_heap_object_cache();
   // Don't reconstruct the final element, which is always undefined and marks
   // the end of the cache, since serializing the live Isolate may extend the
   // shared object cache.

src/snapshot/snapshot.cc

@@ -321,7 +321,7 @@ void Snapshot::SerializeDeserializeAndVerifyForTesting(
     Snapshot::SerializerFlags flags(
         Snapshot::kAllowUnknownExternalReferencesForTesting |
         Snapshot::kAllowActiveIsolateForTesting |
-        ((isolate->shared_isolate() || ReadOnlyHeap::IsReadOnlySpaceShared())
+        ((isolate->has_shared_heap() || ReadOnlyHeap::IsReadOnlySpaceShared())
              ? Snapshot::kReconstructReadOnlyAndSharedObjectCachesForTesting
              : 0));
     serialized_data = Snapshot::Create(isolate, *default_context,

test/cctest/test-serialize.cc

@@ -5103,7 +5103,10 @@ UNINITIALIZED_TEST(SharedStrings) {
 
   v8_flags.shared_string_table = true;
 
-  TestSerializer::InitializeProcessWideSharedIsolateFromBlob(blobs);
+  if (!v8_flags.shared_space) {
+    TestSerializer::InitializeProcessWideSharedIsolateFromBlob(blobs);
+  }
+
   v8::Isolate* isolate1 = TestSerializer::NewIsolateFromBlob(blobs);
   v8::Isolate* isolate2 = TestSerializer::NewIsolateFromBlob(blobs);
   Isolate* i_isolate1 = reinterpret_cast<Isolate*>(isolate1);
@@ -5117,11 +5120,14 @@ UNINITIALIZED_TEST(SharedStrings) {
     // Because both isolate1 and isolate2 are considered running on the main
     // thread, one must be parked to avoid deadlock in the shared heap
     // verification that may happen on client heap disposal.
-    ParkedScope parked(i_isolate2->main_thread_local_isolate());
-    isolate1->Dispose();
+    ParkedScope parked(i_isolate1->main_thread_local_isolate());
+    isolate2->Dispose();
+  }
+  isolate1->Dispose();
+
+  if (!v8_flags.shared_space) {
+    TestSerializer::DeleteProcessWideSharedIsolate();
   }
-  isolate2->Dispose();
-  TestSerializer::DeleteProcessWideSharedIsolate();
 
   blobs.Dispose();
   FreeCurrentEmbeddedBlob();

test/cctest/test-shared-strings.cc

@@ -792,7 +792,7 @@ UNINITIALIZED_TEST(PromotionMarkCompact) {
 
     // In-place-internalizable strings are promoted into the shared heap when
     // sharing.
-    CHECK(!heap->Contains(*one_byte_seq));
+    CHECK_IMPLIES(!v8_flags.shared_space, !heap->Contains(*one_byte_seq));
     CHECK(heap->SharedHeapContains(*one_byte_seq));
   }
 }
@@ -831,7 +831,6 @@ UNINITIALIZED_TEST(PromotionScavenge) {
 
     // In-place-internalizable strings are promoted into the shared heap when
     // sharing.
-    CHECK(!heap->Contains(*one_byte_seq));
     CHECK(heap->SharedHeapContains(*one_byte_seq));
   }
 }
@@ -880,7 +879,6 @@ UNINITIALIZED_TEST(PromotionScavengeOldToShared) {
 
     // In-place-internalizable strings are promoted into the shared heap when
     // sharing.
-    CHECK(!heap->Contains(*one_byte_seq));
     CHECK(heap->SharedHeapContains(*one_byte_seq));
 
     // Since the GC promoted that string into shared heap, it also needs to
@@ -929,7 +927,6 @@ UNINITIALIZED_TEST(PromotionMarkCompactNewToShared) {
 
     // In-place-internalizable strings are promoted into the shared heap when
     // sharing.
-    CHECK(!heap->Contains(*one_byte_seq));
     CHECK(heap->SharedHeapContains(*one_byte_seq));
 
     // Since the GC promoted that string into shared heap, it also needs to
@@ -991,7 +988,6 @@ UNINITIALIZED_TEST(PromotionMarkCompactOldToShared) {
 
     // In-place-internalizable strings are promoted into the shared heap when
     // sharing.
-    CHECK(!heap->Contains(*one_byte_seq));
     CHECK(heap->SharedHeapContains(*one_byte_seq));
 
     // Since the GC promoted that string into shared heap, it also needs to

test/unittests/heap/global-safepoint-unittest.cc

@@ -125,8 +125,9 @@ TEST_F(GlobalSafepointTest, Interrupt) {
     // as of FeedbackVectors, and we wouldn't be testing the interrupt check.
     base::OS::Sleep(base::TimeDelta::FromMilliseconds(500));
     GlobalSafepointScope global_safepoint(i_main_isolate);
-    i_main_isolate->shared_isolate()->global_safepoint()->IterateClientIsolates(
-        [](Isolate* client) {
+    i_main_isolate->shared_heap_isolate()
+        ->global_safepoint()
+        ->IterateClientIsolates([](Isolate* client) {
           client->stack_guard()->RequestTerminateExecution();
         });
   }