[ext-code-space] Fix two more TSAN issues

1) add relaxed version of Code::main_cage_base accessors and use them
   from in those cases where they can be called from backround thread,
2) pass the main cage base value to IsCode() predicate to avoid
   accessing non-acomic Heap pointer value in page headers from
   background compilation thread.

Drive-by cleanup: use MarkingVerifier::cage_base() instead of
Code::main_cage_base().

Bug: v8:11880, v8:12611
Change-Id: I9fd28c1a3babb862d08fec09f6cfc369beaad231
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3494238Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79313}

src/compiler/heap-refs.cc

@@ -66,14 +66,14 @@ enum ObjectDataKind {
 
 namespace {
 
-bool IsReadOnlyHeapObjectForCompiler(HeapObject object) {
+bool IsReadOnlyHeapObjectForCompiler(PtrComprCageBase cage_base,
+                                     HeapObject object) {
   DisallowGarbageCollection no_gc;
   // TODO(jgruber): Remove this compiler-specific predicate and use the plain
   // heap predicate instead. This would involve removing the special cases for
   // builtins.
-  return (object.IsCode() && Code::cast(object).is_builtin()) ||
-         (object.IsHeapObject() &&
-          ReadOnlyHeap::Contains(HeapObject::cast(object)));
+  return (object.IsCode(cage_base) && Code::cast(object).is_builtin()) ||
+         ReadOnlyHeap::Contains(object);
 }
 
 }  // namespace
@@ -103,17 +103,18 @@ class ObjectData : public ZoneObject {
     // handles from read only root table or builtins table which is what
     // canonical scope uses as well. For all other objects we should have
     // created ObjectData in canonical handle scope on the main thread.
-    CHECK_IMPLIES(
-        broker->mode() == JSHeapBroker::kDisabled ||
-            broker->mode() == JSHeapBroker::kSerializing,
-        broker->isolate()->handle_scope_data()->canonical_scope != nullptr);
+    Isolate* isolate = broker->isolate();
+    CHECK_IMPLIES(broker->mode() == JSHeapBroker::kDisabled ||
+                      broker->mode() == JSHeapBroker::kSerializing,
+                  isolate->handle_scope_data()->canonical_scope != nullptr);
     CHECK_IMPLIES(broker->mode() == JSHeapBroker::kSerialized,
                   kind == kUnserializedReadOnlyHeapObject || kind == kSmi ||
                       kind == kNeverSerializedHeapObject ||
                       kind == kBackgroundSerializedHeapObject);
-    CHECK_IMPLIES(kind == kUnserializedReadOnlyHeapObject,
-                  object->IsHeapObject() && IsReadOnlyHeapObjectForCompiler(
-                                                HeapObject::cast(*object)));
+    CHECK_IMPLIES(
+        kind == kUnserializedReadOnlyHeapObject,
+        object->IsHeapObject() && IsReadOnlyHeapObjectForCompiler(
+                                      isolate, HeapObject::cast(*object)));
   }
 
 #define DECLARE_IS(Name) bool Is##Name() const;
@@ -1005,7 +1006,7 @@ ObjectData* JSHeapBroker::TryGetOrCreateData(Handle<Object> object,
     return nullptr;
   }
 
-  if (IsReadOnlyHeapObjectForCompiler(HeapObject::cast(*object))) {
+  if (IsReadOnlyHeapObjectForCompiler(isolate(), HeapObject::cast(*object))) {
     entry = refs_->LookupOrInsert(object.address());
     return zone()->New<ObjectData>(this, &entry->value, object,
                                    kUnserializedReadOnlyHeapObject);

src/heap/factory.cc

@@ -231,7 +231,7 @@ MaybeHandle<Code> Factory::CodeBuilder::BuildInternal(
     raw_code.clear_padding();
 
     if (V8_EXTERNAL_CODE_SPACE_BOOL) {
-      raw_code.set_main_cage_base(isolate_->cage_base());
+      raw_code.set_main_cage_base(isolate_->cage_base(), kRelaxedStore);
       data_container->SetCodeAndEntryPoint(isolate_, raw_code);
     }
 #ifdef VERIFY_HEAP

src/heap/mark-compact.cc

@@ -4909,8 +4909,7 @@ class YoungGenerationMarkingVerifier : public MarkingVerifier {
     VerifyHeapObjectImpl(target);
   }
   void VisitEmbeddedPointer(Code host, RelocInfo* rinfo) override {
-    PtrComprCageBase cage_base = host.main_cage_base();
-    VerifyHeapObjectImpl(rinfo->target_object(cage_base));
+    VerifyHeapObjectImpl(rinfo->target_object(cage_base()));
   }
   void VerifyRootPointers(FullObjectSlot start, FullObjectSlot end) override {
     VerifyPointersImpl(start, end);

src/objects/code-inl.h

@@ -171,11 +171,11 @@ INT32_ACCESSORS(Code, unwinding_info_offset, kUnwindingInfoOffsetOffset)
   }
 
 // Same as RELEASE_ACQUIRE_ACCESSORS_CHECKED2 macro but with Code as a host and
-// using main_cage_base() for computing the base.
+// using main_cage_base(kRelaxedLoad) for computing the base.
 #define RELEASE_ACQUIRE_CODE_ACCESSORS_CHECKED2(name, type, offset,           \
                                                 get_condition, set_condition) \
   type Code::name(AcquireLoadTag tag) const {                                 \
-    PtrComprCageBase cage_base = main_cage_base();                            \
+    PtrComprCageBase cage_base = main_cage_base(kRelaxedLoad);                \
     return Code::name(cage_base, tag);                                        \
   }                                                                           \
   type Code::name(PtrComprCageBase cage_base, AcquireLoadTag) const {         \
@@ -236,17 +236,27 @@ PtrComprCageBase Code::main_cage_base() const {
 #endif
 }
 
-void Code::set_main_cage_base(Address cage_base) {
+PtrComprCageBase Code::main_cage_base(RelaxedLoadTag) const {
+#ifdef V8_EXTERNAL_CODE_SPACE
+  Address cage_base_hi =
+      Relaxed_ReadField<Tagged_t>(kMainCageBaseUpper32BitsOffset);
+  return PtrComprCageBase(cage_base_hi << 32);
+#else
+  return GetPtrComprCageBase(*this);
+#endif
+}
+
+void Code::set_main_cage_base(Address cage_base, RelaxedStoreTag) {
 #ifdef V8_EXTERNAL_CODE_SPACE
   Tagged_t cage_base_hi = static_cast<Tagged_t>(cage_base >> 32);
-  WriteField<Tagged_t>(kMainCageBaseUpper32BitsOffset, cage_base_hi);
+  Relaxed_WriteField<Tagged_t>(kMainCageBaseUpper32BitsOffset, cage_base_hi);
 #else
   UNREACHABLE();
 #endif
 }
 
 CodeDataContainer Code::GCSafeCodeDataContainer(AcquireLoadTag) const {
-  PtrComprCageBase cage_base = main_cage_base();
+  PtrComprCageBase cage_base = main_cage_base(kRelaxedLoad);
   HeapObject object =
       TaggedField<HeapObject, kCodeDataContainerOffset>::Acquire_Load(cage_base,
                                                                       *this);
@@ -318,7 +328,7 @@ void Code::WipeOutHeader() {
   WRITE_FIELD(*this, kPositionTableOffset, Smi::FromInt(0));
   WRITE_FIELD(*this, kCodeDataContainerOffset, Smi::FromInt(0));
   if (V8_EXTERNAL_CODE_SPACE_BOOL) {
-    set_main_cage_base(kNullAddress);
+    set_main_cage_base(kNullAddress, kRelaxedStore);
   }
 }
 

src/objects/code.h

@@ -508,7 +508,8 @@ class Code : public HeapObject {
   // This field contains cage base value which is used for decompressing
   // the references to non-Code objects (map, deoptimization_data, etc.).
   inline PtrComprCageBase main_cage_base() const;
-  inline void set_main_cage_base(Address cage_base);
+  inline PtrComprCageBase main_cage_base(RelaxedLoadTag) const;
+  inline void set_main_cage_base(Address cage_base, RelaxedStoreTag);
 
   // Clear uninitialized padding space. This ensures that the snapshot content
   // is deterministic. Depending on the V8 build mode there could be no padding.

src/snapshot/deserializer.cc

@@ -1140,7 +1140,7 @@ int Deserializer<IsolateT>::ReadSingleBytecodeData(byte data,
 
         Code code = Code::cast(*slot_accessor.object());
         if (V8_EXTERNAL_CODE_SPACE_BOOL) {
-          code.set_main_cage_base(isolate()->cage_base());
+          code.set_main_cage_base(isolate()->cage_base(), kRelaxedStore);
         }
         DeserializerRelocInfoVisitor visitor(this, &preserialized_objects);
         for (RelocIterator it(code, Code::BodyDescriptor::kRelocModeMask);