diff --git a/v8/src/sandbox/trusted-pointer-table-inl.h b/v8/src/sandbox/trusted-pointer-table-inl.h index 25200e3f9ea..2e484c7b889 100644 --- a/v8/src/sandbox/trusted-pointer-table-inl.h +++ b/v8/src/sandbox/trusted-pointer-table-inl.h @@ -74,18 +74,35 @@ IndirectPointerTag TrustedPointerTableEntry::GetTag() const { } void TrustedPointerTableEntry::Unpublish() { + // Use a CAS loop instead of a plain load/store so that we don't drop a + // mark bit set concurrently by the GC marker (TrustedPointerTableEntry::Mark + // runs on a background thread). auto old_payload = payload_.load(std::memory_order_relaxed); - auto new_payload = old_payload; - new_payload.SetTag(kUnpublishedIndirectPointerTag); - payload_.store(new_payload, std::memory_order_release); + while (true) { + auto new_payload = old_payload; + new_payload.SetTag(kUnpublishedIndirectPointerTag); + if (payload_.compare_exchange_weak(old_payload, new_payload, + std::memory_order_release, + std::memory_order_relaxed)) { + break; + } + } } void TrustedPointerTableEntry::Publish(IndirectPointerTag tag) { + // Use a CAS loop instead of a plain load/store so that we don't drop a + // mark bit set concurrently by the GC marker. auto old_payload = payload_.load(std::memory_order_relaxed); - CHECK(old_payload.IsTaggedWith(kUnpublishedIndirectPointerTag)); - auto new_payload = old_payload; - new_payload.SetTag(tag); - payload_.store(new_payload, std::memory_order_release); + while (true) { + CHECK(old_payload.IsTaggedWith(kUnpublishedIndirectPointerTag)); + auto new_payload = old_payload; + new_payload.SetTag(tag); + if (payload_.compare_exchange_weak(old_payload, new_payload, + std::memory_order_release, + std::memory_order_relaxed)) { + break; + } + } } bool TrustedPointerTableEntry::IsFreelistEntry() const { @@ -99,19 +116,18 @@ std::optional TrustedPointerTableEntry::GetNextFreelistEntryIndex() } void TrustedPointerTableEntry::Mark() { + // The mutator may concurrently retag this entry via Publish()/Unpublish(). + // Retry on CAS failure so that we never leave a live entry unmarked. auto old_payload = payload_.load(std::memory_order_relaxed); - DCHECK(old_payload.ContainsPointer()); - - auto new_payload = old_payload; - new_payload.SetMarkBit(); - - // We don't need to perform the CAS in a loop since it can only fail if a new - // value has been written into the entry. This, however, will also have set - // the marking bit. - bool success = payload_.compare_exchange_strong(old_payload, new_payload, - std::memory_order_relaxed); - DCHECK(success || old_payload.HasMarkBitSet()); - USE(success); + while (!old_payload.HasMarkBitSet()) { + DCHECK(old_payload.ContainsPointer()); + auto new_payload = old_payload; + new_payload.SetMarkBit(); + if (payload_.compare_exchange_weak(old_payload, new_payload, + std::memory_order_relaxed)) { + break; + } + } } void TrustedPointerTableEntry::Unmark() {