[shared-struct] Handle HeapNumbers in Object::IsShared

HeapNumbers that are in the shared heap were incorrectly considered
!IsShared().

TBR=jkummerow@chromium.org

Bug: v8:12547
Change-Id: Ie4b9575445d841a7045c947ff4439bf53a22869d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3504085
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Auto-Submit: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79372}

src/codegen/code-stub-assembler.cc

@@ -6991,15 +6991,13 @@ TNode<BoolT> CodeStubAssembler::IsNumberArrayIndex(TNode<Number> number) {
       [=] { return IsHeapNumberUint32(CAST(number)); });
 }
 
-TNode<BoolT> CodeStubAssembler::IsReadOnlyHeapObject(TNode<HeapObject> object) {
+TNode<IntPtrT> CodeStubAssembler::LoadBasicMemoryChunkFlags(
+    TNode<HeapObject> object) {
   TNode<IntPtrT> object_word = BitcastTaggedToWord(object);
   TNode<IntPtrT> page = PageFromAddress(object_word);
-  TNode<IntPtrT> flags = UncheckedCast<IntPtrT>(
+  return UncheckedCast<IntPtrT>(
       Load(MachineType::Pointer(), page,
            IntPtrConstant(BasicMemoryChunk::kFlagsOffset)));
-  return WordNotEqual(
-      WordAnd(flags, IntPtrConstant(BasicMemoryChunk::READ_ONLY_HEAP)),
-      IntPtrConstant(0));
 }
 
 template <typename TIndex>
@@ -15935,33 +15933,63 @@ void CodeStubAssembler::SwissNameDictionaryAdd(TNode<SwissNameDictionary> table,
 }
 
 void CodeStubAssembler::SharedValueBarrier(
-    TNode<Context> context, TNode<Object> value,
-    TVariable<Object>* var_shared_value) {
+    TNode<Context> context, TVariable<Object>* var_shared_value) {
   // The barrier ensures that the value can be shared across Isolates.
   // The fast paths should be kept in sync with Object::Share.
 
-  Label skip_barrier(this);
+  TNode<Object> value = var_shared_value->value();
+  Label check_in_shared_heap(this), slow(this), skip_barrier(this), done(this);
 
   // Fast path: Smis are trivially shared.
-  GotoIf(TaggedIsSmi(value), &skip_barrier);
+  GotoIf(TaggedIsSmi(value), &done);
   // Fast path: Shared memory features imply shared RO space, so RO objects are
   // trivially shared.
   DCHECK(ReadOnlyHeap::IsReadOnlySpaceShared());
-  GotoIf(IsReadOnlyHeapObject(CAST(value)), &skip_barrier);
+  TNode<IntPtrT> page_flags = LoadBasicMemoryChunkFlags(CAST(value));
+  GotoIf(WordNotEqual(WordAnd(page_flags,
+                              IntPtrConstant(BasicMemoryChunk::READ_ONLY_HEAP)),
+                      IntPtrConstant(0)),
+         &skip_barrier);
 
   // Fast path: Check if the HeapObject is already shared.
   TNode<Uint16T> value_instance_type =
       LoadMapInstanceType(LoadMap(CAST(value)));
   GotoIf(IsSharedStringInstanceType(value_instance_type), &skip_barrier);
   GotoIf(IsJSSharedStructInstanceType(value_instance_type), &skip_barrier);
+  GotoIf(IsHeapNumberInstanceType(value_instance_type), &check_in_shared_heap);
+  Goto(&slow);
+
+  BIND(&check_in_shared_heap);
+  {
+    Branch(
+        WordNotEqual(WordAnd(page_flags,
+                             IntPtrConstant(BasicMemoryChunk::IN_SHARED_HEAP)),
+                     IntPtrConstant(0)),
+        &skip_barrier, &slow);
+  }
 
   // Slow path: Call out to runtime to share primitives and to throw on
   // non-shared JS objects.
-  *var_shared_value =
-      CallRuntime(Runtime::kSharedValueBarrierSlow, context, value);
-  Goto(&skip_barrier);
+  BIND(&slow);
+  {
+    *var_shared_value =
+        CallRuntime(Runtime::kSharedValueBarrierSlow, context, value);
+    Goto(&skip_barrier);
+  }
 
   BIND(&skip_barrier);
+  {
+    CSA_DCHECK(
+        this,
+        WordNotEqual(
+            WordAnd(LoadBasicMemoryChunkFlags(CAST(var_shared_value->value())),
+                    IntPtrConstant(BasicMemoryChunk::READ_ONLY_HEAP |
+                                   BasicMemoryChunk::IN_SHARED_HEAP)),
+            IntPtrConstant(0)));
+    Goto(&done);
+  }
+
+  BIND(&done);
 }
 
 }  // namespace internal

src/codegen/code-stub-assembler.h

@@ -2446,8 +2446,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
                                    TVariable<Numeric>* var_numeric,
                                    TVariable<Smi>* var_feedback);
 
-  // Ensures that {value} is shareable across Isolates, and throws if not.
-  void SharedValueBarrier(TNode<Context> context, TNode<Object> value,
+  // Ensures that {var_shared_value} is shareable across Isolates, and throws if
+  // not.
+  void SharedValueBarrier(TNode<Context> context,
                           TVariable<Object>* var_shared_value);
 
   TNode<WordT> TimesSystemPointerSize(TNode<WordT> value);
@@ -2644,7 +2645,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<BoolT> IsUndetectableMap(TNode<Map> map);
   TNode<BoolT> IsNotWeakFixedArraySubclass(TNode<HeapObject> object);
   TNode<BoolT> IsZeroOrContext(TNode<Object> object);
-  TNode<BoolT> IsReadOnlyHeapObject(TNode<HeapObject> object);
 
   TNode<BoolT> IsPromiseResolveProtectorCellInvalid();
   TNode<BoolT> IsPromiseThenProtectorCellInvalid();
@@ -2654,6 +2654,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<BoolT> IsRegExpSpeciesProtectorCellInvalid();
   TNode<BoolT> IsPromiseSpeciesProtectorCellInvalid();
 
+  TNode<IntPtrT> LoadBasicMemoryChunkFlags(TNode<HeapObject> object);
+
   TNode<BoolT> LoadRuntimeFlag(ExternalReference address_of_flag) {
     TNode<Word32T> flag_value = UncheckedCast<Word32T>(
         Load(MachineType::Uint8(), ExternalConstant(address_of_flag)));

src/ic/accessor-assembler.cc

@@ -1198,7 +1198,7 @@ void AccessorAssembler::HandleStoreICSmiHandlerJSSharedStructFieldCase(
                   Int32Constant(Representation::kTagged)));
 
   TVARIABLE(Object, shared_value, value);
-  SharedValueBarrier(context, value, &shared_value);
+  SharedValueBarrier(context, &shared_value);
 
   TNode<BoolT> is_inobject =
       IsSetWord32<StoreHandler::IsInobjectBits>(handler_word);
@@ -1720,7 +1720,7 @@ void AccessorAssembler::StoreJSSharedStructField(
       LoadMapInstanceSizeInWords(shared_struct_map);
 
   TVARIABLE(Object, shared_value, maybe_local_value);
-  SharedValueBarrier(context, maybe_local_value, &shared_value);
+  SharedValueBarrier(context, &shared_value);
 
   Label inobject(this), backing_store(this);
   Branch(UintPtrLessThan(field_index, instance_size_in_words), &inobject,

src/objects/objects-inl.h

@@ -1162,6 +1162,9 @@ Object Object::GetHash() {
 }
 
 bool Object::IsShared() const {
+  // This logic should be kept in sync with fast paths in
+  // CodeStubAssembler::SharedValueBarrier.
+
   // Smis are trivially shared.
   if (IsSmi()) return true;
 
@@ -1186,6 +1189,8 @@ bool Object::IsShared() const {
         return true;
       }
       return false;
+    case HEAP_NUMBER_TYPE:
+      return object.InSharedWritableHeap();
     default:
       return false;
   }

test/mjsunit/shared-memory/shared-struct-surface.js

@@ -18,7 +18,9 @@ let S = new SharedStructType(['field']);
 (function TestPrimitives() {
   // All primitives can be stored in fields.
   let s = new S();
-  for (let prim of [42, -0, undefined, null, true, false, "foo"]) {
+  for (let prim of [42, -0, Math.random(),
+                    undefined, null, true, false,
+                    "foo"]) {
     s.field = prim;
     assertEquals(s.field, prim);
   }