[ext-code-space] Fix TSAN issues in JSFunctionRef::code()

This CL
1) adds relaxed version of CodeDataContainer::code_cage_base accessors
   and use them from relaxed CodeDataContainer::code accessors,
2) uses relaxed version of FromCodeT() in JSFunctionRef::code().

Bug: v8:11880, chromium:1293642
Change-Id: Idc9ba59a97a44a0963197cad50b5e5b440f9629e
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3450423Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79030}

src/compiler/heap-refs.cc

@@ -2165,8 +2165,10 @@ BIMODAL_ACCESSOR(JSFunction, SharedFunctionInfo, shared)
 #undef JSFUNCTION_BIMODAL_ACCESSOR_WITH_DEP_C
 
 CodeRef JSFunctionRef::code() const {
-  return MakeRefAssumeMemoryFence(broker(),
-                                  FromCodeT(object()->code(kAcquireLoad)));
+  CodeT code = object()->code(kAcquireLoad);
+  // Safe to do a relaxed conversion to Code here since CodeT::code field is
+  // modified only by GC and the CodeT was acquire-loaded.
+  return MakeRefAssumeMemoryFence(broker(), FromCodeT(code, kRelaxedLoad));
 }
 
 NativeContextRef JSFunctionRef::native_context() const {

src/heap/factory-base.cc

@@ -82,7 +82,8 @@ Handle<CodeDataContainer> FactoryBase<Impl>::NewCodeDataContainer(
                                     SKIP_WRITE_BARRIER);
   data_container.set_kind_specific_flags(flags, kRelaxedStore);
   if (V8_EXTERNAL_CODE_SPACE_BOOL) {
-    data_container.set_code_cage_base(impl()->isolate()->code_cage_base());
+    data_container.set_code_cage_base(impl()->isolate()->code_cage_base(),
+                                      kRelaxedStore);
     Isolate* isolate_for_sandbox = impl()->isolate_for_sandbox();
     data_container.AllocateExternalPointerEntries(isolate_for_sandbox);
     data_container.set_raw_code(Smi::zero(), SKIP_WRITE_BARRIER);

src/objects/code-inl.h

@@ -881,7 +881,7 @@ void CodeDataContainer::set_raw_code(Object value, WriteBarrierMode mode) {
 }
 
 Object CodeDataContainer::raw_code(RelaxedLoadTag tag) const {
-  PtrComprCageBase cage_base = code_cage_base();
+  PtrComprCageBase cage_base = code_cage_base(tag);
   return CodeDataContainer::raw_code(cage_base, tag);
 }
 
@@ -915,6 +915,28 @@ void CodeDataContainer::set_code_cage_base(Address code_cage_base) {
 #endif
 }
 
+PtrComprCageBase CodeDataContainer::code_cage_base(RelaxedLoadTag) const {
+#ifdef V8_EXTERNAL_CODE_SPACE
+  // TODO(v8:10391): consider protecting this value with the sandbox.
+  Address code_cage_base_hi =
+      Relaxed_ReadField<Tagged_t>(kCodeCageBaseUpper32BitsOffset);
+  return PtrComprCageBase(code_cage_base_hi << 32);
+#else
+  return GetPtrComprCageBase(*this);
+#endif
+}
+
+void CodeDataContainer::set_code_cage_base(Address code_cage_base,
+                                           RelaxedStoreTag) {
+#ifdef V8_EXTERNAL_CODE_SPACE
+  Tagged_t code_cage_base_hi = static_cast<Tagged_t>(code_cage_base >> 32);
+  Relaxed_WriteField<Tagged_t>(kCodeCageBaseUpper32BitsOffset,
+                               code_cage_base_hi);
+#else
+  UNREACHABLE();
+#endif
+}
+
 void CodeDataContainer::AllocateExternalPointerEntries(Isolate* isolate) {
   CHECK(V8_EXTERNAL_CODE_SPACE_BOOL);
   InitExternalPointerField(kCodeEntryPointOffset, isolate, kCodeEntryPointTag);
@@ -930,7 +952,7 @@ Code CodeDataContainer::code(PtrComprCageBase cage_base) const {
 }
 
 Code CodeDataContainer::code(RelaxedLoadTag tag) const {
-  PtrComprCageBase cage_base = code_cage_base();
+  PtrComprCageBase cage_base = code_cage_base(tag);
   return CodeDataContainer::code(cage_base, tag);
 }
 

src/objects/code.h

@@ -80,6 +80,8 @@ class CodeDataContainer : public HeapObject {
   // the full value is not guaranteed.
   inline PtrComprCageBase code_cage_base() const;
   inline void set_code_cage_base(Address code_cage_base);
+  inline PtrComprCageBase code_cage_base(RelaxedLoadTag) const;
+  inline void set_code_cage_base(Address code_cage_base, RelaxedStoreTag);
 
   // Cached value of code().InstructionStart().
   // Available only when V8_EXTERNAL_CODE_SPACE is defined.

src/objects/objects-inl.h

@@ -159,6 +159,21 @@ T Object::Relaxed_ReadField(size_t offset) const {
       reinterpret_cast<AtomicT*>(field_address(offset))));
 }
 
+template <class T,
+          typename std::enable_if<(std::is_arithmetic<T>::value ||
+                                   std::is_enum<T>::value) &&
+                                      !std::is_floating_point<T>::value,
+                                  int>::type>
+void Object::Relaxed_WriteField(size_t offset, T value) {
+  // Pointer compression causes types larger than kTaggedSize to be
+  // unaligned. Atomic stores must be aligned.
+  DCHECK_IMPLIES(COMPRESS_POINTERS_BOOL, sizeof(T) <= kTaggedSize);
+  using AtomicT = typename base::AtomicTypeFromByteWidth<sizeof(T)>::type;
+  base::AsAtomicImpl<AtomicT>::Relaxed_Store(
+      reinterpret_cast<AtomicT*>(field_address(offset)),
+      static_cast<AtomicT>(value));
+}
+
 bool HeapObject::InSharedHeap() const {
   if (IsReadOnlyHeapObject(*this)) return V8_SHARED_RO_HEAP_BOOL;
   return InSharedWritableHeap();

src/objects/objects.h

@@ -674,6 +674,13 @@ class Object : public TaggedImpl<HeapObjectReferenceType::STRONG, Address> {
     return ReadMaybeUnalignedValue<T>(field_address(offset));
   }
 
+  template <class T, typename std::enable_if<std::is_arithmetic<T>::value ||
+                                                 std::is_enum<T>::value,
+                                             int>::type = 0>
+  inline void WriteField(size_t offset, T value) const {
+    return WriteMaybeUnalignedValue<T>(field_address(offset), value);
+  }
+
   // Atomically reads a field using relaxed memory ordering. Can only be used
   // with integral types whose size is <= kTaggedSize (to guarantee alignment).
   template <class T,
@@ -683,12 +690,14 @@ class Object : public TaggedImpl<HeapObjectReferenceType::STRONG, Address> {
                                     int>::type = 0>
   inline T Relaxed_ReadField(size_t offset) const;
 
-  template <class T, typename std::enable_if<std::is_arithmetic<T>::value ||
-                                                 std::is_enum<T>::value,
-                                             int>::type = 0>
-  inline void WriteField(size_t offset, T value) const {
-    return WriteMaybeUnalignedValue<T>(field_address(offset), value);
-  }
+  // Atomically writes a field using relaxed memory ordering. Can only be used
+  // with integral types whose size is <= kTaggedSize (to guarantee alignment).
+  template <class T,
+            typename std::enable_if<(std::is_arithmetic<T>::value ||
+                                     std::is_enum<T>::value) &&
+                                        !std::is_floating_point<T>::value,
+                                    int>::type = 0>
+  inline void Relaxed_WriteField(size_t offset, T value);
 
   //
   // SandboxedPointer_t field accessors.