Allow in-place internalizable strings in non-shared old space

It is now considered best effort, that in-place internalizable strings
are promoted into the shared old space instead of non-shared old space.
This was previously an invariant, but it doesn't hold if the whole page
containing the shared string is promoted instead of individual objects.
In addition with conservative stack scanning individual objects won't be
moved.

Bug: v8:12007
Change-Id: I7474738b02b0c18080cb2e82268a02bf9b480c40
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3688512Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Patrick Thier <pthier@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80969}

src/heap/factory.cc

@@ -886,6 +886,11 @@ StringTransitionStrategy Factory::ComputeInternalizationStrategyForString(
   if (Heap::InYoungGeneration(*string)) {
     return StringTransitionStrategy::kCopy;
   }
+  // If the string table is shared, we need to copy if the string is not already
+  // in the shared heap.
+  if (FLAG_shared_string_table && !string->InSharedHeap()) {
+    return StringTransitionStrategy::kCopy;
+  }
   DCHECK_NOT_NULL(internalized_map);
   DisallowGarbageCollection no_gc;
   // This method may be called concurrently, so snapshot the map from the input

src/heap/heap.cc

@@ -4502,15 +4502,12 @@ bool Heap::SharedHeapContains(HeapObject value) const {
   return false;
 }
 
-bool Heap::ShouldBeInSharedOldSpace(HeapObject value) {
+bool Heap::MustBeInSharedOldSpace(HeapObject value) {
   if (isolate()->OwnsStringTables()) return false;
   if (ReadOnlyHeap::Contains(value)) return false;
   if (Heap::InYoungGeneration(value)) return false;
   if (value.IsExternalString()) return false;
-  if (value.IsString()) {
-    return value.IsInternalizedString() ||
-           String::IsInPlaceInternalizable(String::cast(value));
-  }
+  if (value.IsInternalizedString()) return true;
   return false;
 }
 

src/heap/heap.h

@@ -1268,8 +1268,8 @@ class Heap {
   // heaps is required.
   V8_EXPORT_PRIVATE bool SharedHeapContains(HeapObject value) const;
 
-  // Returns whether the object should be in the shared old space.
-  V8_EXPORT_PRIVATE bool ShouldBeInSharedOldSpace(HeapObject value);
+  // Returns whether the object must be in the shared old space.
+  V8_EXPORT_PRIVATE bool MustBeInSharedOldSpace(HeapObject value);
 
   // Checks whether an address/object in a space.
   // Currently used by tests, serialization and heap verification only.

src/heap/mark-compact.cc

@@ -279,7 +279,7 @@ class FullMarkingVerifier : public MarkingVerifier {
         BasicMemoryChunk::FromHeapObject(heap_object)->InSharedHeap())
       return;
 
-    if (heap_->ShouldBeInSharedOldSpace(heap_object)) {
+    if (heap_->MustBeInSharedOldSpace(heap_object)) {
       CHECK(heap_->SharedHeapContains(heap_object));
     }
 

test/cctest/test-serialize.cc

@@ -4977,7 +4977,10 @@ void CheckObjectsAreInSharedHeap(Isolate* isolate) {
   DisallowGarbageCollection no_gc;
   for (HeapObject obj = iterator.Next(); !obj.is_null();
        obj = iterator.Next()) {
-    if (heap->ShouldBeInSharedOldSpace(obj)) {
+    const bool expected_in_shared_old =
+        heap->MustBeInSharedOldSpace(obj) ||
+        (obj.IsString() && String::IsInPlaceInternalizable(String::cast(obj)));
+    if (expected_in_shared_old) {
       CHECK(obj.InSharedHeap());
     }
   }