[shared-struct] Use external pointer table in JSAtomicsMutex when compressing pointers

Using the external pointer table when compressing pointers lets us ease
the alignment requirement for the state field from 8-byte-aligned to
4-byte-aligned, as 8-byte alignment is not supported during compaction.

Bug: v8:12547
Change-Id: Ibbcb0d71f09f9bac66acc81459ab71e354ea405f
Cq-Include-Trybots: luci.v8.try:v8_linux64_pointer_compression_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3783077Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81979}

src/heap/concurrent-allocator-inl.h

@@ -33,10 +33,7 @@ AllocationResult ConcurrentAllocator::AllocateRaw(int size_in_bytes,
   }
 
   AllocationResult result;
-  // TODO(v8:12547): We cannot use USE_ALLOCATION_ALIGNMENT_BOOL here as
-  // JSAtomicsMutex has alignment restrictions that are not optional and is
-  // allocated using ConcurrentAllocator in the shared heap.
-  if (alignment != kTaggedAligned) {
+  if (USE_ALLOCATION_ALIGNMENT_BOOL && alignment != kTaggedAligned) {
     result = lab_.AllocateRawAligned(size_in_bytes, alignment);
   } else {
     result = lab_.AllocateRawUnaligned(size_in_bytes);

src/heap/factory-inl.h

@@ -58,27 +58,6 @@ Handle<JSArray> Factory::NewJSArrayWithElements(Handle<FixedArrayBase> elements,
                                 allocation);
 }
 
-Handle<JSObject> Factory::NewJSObjectFromMap(
-    Handle<Map> map, AllocationType allocation,
-    Handle<AllocationSite> allocation_site) {
-  return NewJSObjectFromMapInternal(map, allocation, allocation_site,
-                                    kTaggedAligned);
-}
-
-Handle<JSObject> Factory::NewSystemPointerAlignedJSObjectFromMap(
-    Handle<Map> map, AllocationType allocation,
-    Handle<AllocationSite> allocation_site) {
-  AllocationAlignment alignment;
-  if (kTaggedSize == kSystemPointerSize) {
-    alignment = kTaggedAligned;
-  } else {
-    DCHECK_EQ(kDoubleSize, kSystemPointerSize);
-    alignment = kDoubleAligned;
-  }
-  return NewJSObjectFromMapInternal(map, allocation, allocation_site,
-                                    alignment);
-}
-
 Handle<JSObject> Factory::NewFastOrSlowJSObjectFromMap(
     Handle<Map> map, int number_of_slow_properties, AllocationType allocation,
     Handle<AllocationSite> allocation_site) {

src/heap/factory.cc

@@ -343,7 +343,7 @@ HeapObject Factory::AllocateRaw(int size, AllocationType allocation,
 
 HeapObject Factory::AllocateRawWithAllocationSite(
     Handle<Map> map, AllocationType allocation,
-    Handle<AllocationSite> allocation_site, AllocationAlignment alignment) {
+    Handle<AllocationSite> allocation_site) {
   DCHECK(map->instance_type() != MAP_TYPE);
   int size = map->instance_size();
   if (!allocation_site.is_null()) {
@@ -351,7 +351,7 @@ HeapObject Factory::AllocateRawWithAllocationSite(
     size += AllocationMemento::kSize;
   }
   HeapObject result = allocator()->AllocateRawWith<HeapAllocator::kRetryOrFail>(
-      size, allocation, AllocationOrigin::kRuntime, alignment);
+      size, allocation);
   WriteBarrierMode write_barrier_mode = allocation == AllocationType::kYoung
                                             ? SKIP_WRITE_BARRIER
                                             : UPDATE_WRITE_BARRIER;
@@ -2730,9 +2730,9 @@ void Factory::InitializeJSObjectBody(JSObject obj, Map map, int start_offset) {
   }
 }
 
-Handle<JSObject> Factory::NewJSObjectFromMapInternal(
+Handle<JSObject> Factory::NewJSObjectFromMap(
     Handle<Map> map, AllocationType allocation,
-    Handle<AllocationSite> allocation_site, AllocationAlignment alignment) {
+    Handle<AllocationSite> allocation_site) {
   // JSFunctions should be allocated using AllocateFunction to be
   // properly initialized.
   DCHECK(!InstanceTypeChecker::IsJSFunction((map->instance_type())));
@@ -2741,8 +2741,8 @@ Handle<JSObject> Factory::NewJSObjectFromMapInternal(
   // AllocateGlobalObject to be properly initialized.
   DCHECK(map->instance_type() != JS_GLOBAL_OBJECT_TYPE);
 
-  JSObject js_obj = JSObject::cast(AllocateRawWithAllocationSite(
-      map, allocation, allocation_site, alignment));
+  JSObject js_obj = JSObject::cast(
+      AllocateRawWithAllocationSite(map, allocation, allocation_site));
 
   InitializeJSObjectFromMap(js_obj, *empty_fixed_array(), *map);
 

src/heap/factory.h

@@ -558,10 +558,7 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
   // points to the site.
   // JS objects are pretenured when allocated by the bootstrapper and
   // runtime.
-  inline Handle<JSObject> NewJSObjectFromMap(
-      Handle<Map> map, AllocationType allocation = AllocationType::kYoung,
-      Handle<AllocationSite> allocation_site = Handle<AllocationSite>::null());
-  inline Handle<JSObject> NewSystemPointerAlignedJSObjectFromMap(
+  Handle<JSObject> NewJSObjectFromMap(
       Handle<Map> map, AllocationType allocation = AllocationType::kYoung,
       Handle<AllocationSite> allocation_site = Handle<AllocationSite>::null());
   // Like NewJSObjectFromMap, but includes allocating a properties dictionary.
@@ -1098,12 +1095,7 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
 
   HeapObject AllocateRawWithAllocationSite(
       Handle<Map> map, AllocationType allocation,
-      Handle<AllocationSite> allocation_site,
-      AllocationAlignment alignment = kTaggedAligned);
-
-  Handle<JSObject> NewJSObjectFromMapInternal(
-      Handle<Map> map, AllocationType allocation,
-      Handle<AllocationSite> allocation_site, AllocationAlignment alignment);
+      Handle<AllocationSite> allocation_site);
 
   Handle<JSArrayBufferView> NewJSArrayBufferView(
       Handle<Map> map, Handle<FixedArrayBase> elements,

src/heap/mark-compact.cc

@@ -2213,20 +2213,18 @@ void MarkCompactCollector::MarkObjectsFromClientHeaps() {
           obj.IterateFast(cage_base, &visitor);
         }
 
-#ifdef V8_ENABLE_SANDBOX
-        if (IsSandboxedExternalPointerType(kWaiterQueueNodeTag)) {
-          // Custom marking for the external pointer table entry used to hold
-          // client Isolates' WaiterQueueNode, which is used by JS mutexes and
-          // condition variables.
-          DCHECK(IsSharedExternalPointerType(kWaiterQueueNodeTag));
-          ExternalPointerHandle waiter_queue_ext;
-          if (client->GetWaiterQueueNodeExternalPointer().To(
-                  &waiter_queue_ext)) {
-            uint32_t index = waiter_queue_ext >> kExternalPointerIndexShift;
-            client->shared_external_pointer_table().Mark(index);
-          }
+#ifdef V8_COMPRESS_POINTERS
+        DCHECK(IsSandboxedExternalPointerType(kWaiterQueueNodeTag));
+        DCHECK(IsSharedExternalPointerType(kWaiterQueueNodeTag));
+        // Custom marking for the external pointer table entry used to hold
+        // client Isolates' WaiterQueueNode, which is used by JS mutexes and
+        // condition variables.
+        ExternalPointerHandle waiter_queue_ext;
+        if (client->GetWaiterQueueNodeExternalPointer().To(&waiter_queue_ext)) {
+          uint32_t index = waiter_queue_ext >> kExternalPointerIndexShift;
+          client->shared_external_pointer_table().Mark(index);
         }
-#endif  // V8_ENABLE_SANDBOX
+#endif  // V8_COMPRESS_POINTERS
       });
 }
 

src/objects/js-atomics-synchronization.cc

@@ -28,14 +28,14 @@ class V8_NODISCARD WaiterQueueNode final {
   template <typename T>
   static typename T::StateT EncodeHead(Isolate* requester,
                                        WaiterQueueNode* head) {
-#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+#ifdef V8_COMPRESS_POINTERS
     if (head == nullptr) return 0;
     auto state = static_cast<typename T::StateT>(
         requester->InsertWaiterQueueNodeIntoSharedExternalPointerTable(
             reinterpret_cast<Address>(head)));
 #else
     auto state = base::bit_cast<typename T::StateT>(head);
-#endif  // V8_SANDBOXED_EXTERNAL_POINTERS
+#endif  // V8_COMPRESS_POINTERS
 
     DCHECK_EQ(0, state & T::kLockBitsMask);
     return state;
@@ -46,10 +46,7 @@ class V8_NODISCARD WaiterQueueNode final {
   template <typename T>
   static WaiterQueueNode* DestructivelyDecodeHead(Isolate* requester,
                                                   typename T::StateT state) {
-#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-    ExternalPointer_t ptr =
-        static_cast<ExternalPointer_t>(state & T::kWaiterQueueHeadMask);
-    if (ptr == 0) return nullptr;
+#ifdef V8_COMPRESS_POINTERS
     ExternalPointerHandle handle =
         static_cast<ExternalPointerHandle>(state & T::kWaiterQueueHeadMask);
     if (handle == 0) return nullptr;
@@ -60,7 +57,7 @@ class V8_NODISCARD WaiterQueueNode final {
             handle, kNullAddress, kWaiterQueueNodeTag));
 #else
     return base::bit_cast<WaiterQueueNode*>(state & T::kWaiterQueueHeadMask);
-#endif  // V8_SANDBOXED_EXTERNAL_POINTERS
+#endif  // V8_COMPRESS_POINTERS
   }
 
   // Enqueues {new_tail}, mutating {head} to be the new head.
@@ -134,8 +131,7 @@ Handle<JSAtomicsMutex> JSAtomicsMutex::Create(Isolate* isolate) {
   auto* factory = isolate->factory();
   Handle<Map> map = isolate->js_atomics_mutex_map();
   Handle<JSAtomicsMutex> mutex = Handle<JSAtomicsMutex>::cast(
-      factory->NewSystemPointerAlignedJSObjectFromMap(
-          map, AllocationType::kSharedOld));
+      factory->NewJSObjectFromMap(map, AllocationType::kSharedOld));
   mutex->set_state(kUnlocked);
   mutex->set_owner_thread_id(ThreadId::Invalid().ToInteger());
   return mutex;

src/objects/js-atomics-synchronization.h

@@ -110,9 +110,9 @@ class JSAtomicsMutex
   static constexpr int kIsWaiterQueueLockedBit = 1 << 1;
   static constexpr int kLockBitsSize = 2;
 
-#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+#ifdef V8_COMPRESS_POINTERS
   using StateT = uint32_t;
-  static_assert(sizeof(StateT) == kExternalPointerSlotSize);
+  static_assert(sizeof(StateT) == sizeof(ExternalPointerHandle));
 #else
   using StateT = uintptr_t;
 #endif

src/objects/js-atomics-synchronization.tq

@@ -3,13 +3,11 @@
 // found in the LICENSE file.
 
 extern class JSAtomicsMutex extends JSObject {
-  // owner_thread_id must come first to ensure that the state field is
-  // uintptr-aligned.
-  @if(V8_NOT_SANDBOXED_EXTERNAL_POINTERS_AND_TAGGED_SIZE_8_BYTES)
-  owner_thread_id: intptr;
-  @ifnot(V8_NOT_SANDBOXED_EXTERNAL_POINTERS_AND_TAGGED_SIZE_8_BYTES)
+  @if(TAGGED_SIZE_8_BYTES) state: uintptr;
+  @ifnot(TAGGED_SIZE_8_BYTES) state: uint32;
+
   owner_thread_id: int32;
 
-  @if(V8_SANDBOXED_EXTERNAL_POINTERS) state: uint32;
-  @ifnot(V8_SANDBOXED_EXTERNAL_POINTERS) state: uintptr;
+  @if(TAGGED_SIZE_8_BYTES) optional_padding: uint32;
+  @ifnot(TAGGED_SIZE_8_BYTES) optional_padding: void;
 }

src/torque/torque-parser.cc

@@ -70,8 +70,6 @@ class BuildFlags : public ContextualClass<BuildFlags> {
     build_flags_["V8_ENABLE_SANDBOX"] = V8_ENABLE_SANDBOX_BOOL;
     build_flags_["V8_SANDBOXED_EXTERNAL_POINTERS"] =
         V8_SANDBOXED_EXTERNAL_POINTERS_BOOL;
-    build_flags_["V8_NOT_SANDBOXED_EXTERNAL_POINTERS_AND_TAGGED_SIZE_8_BYTES"] =
-        !V8_SANDBOXED_EXTERNAL_POINTERS_BOOL && TAGGED_SIZE_8_BYTES;
     build_flags_["DEBUG"] = DEBUG_BOOL;
   }
   static bool GetFlag(const std::string& name, const char* production) {