From fd2c27ff4ea101e7b67262be507e6abe55a704cb Mon Sep 17 00:00:00 2001
From: Ryan Macnak <rmacnak@google.com>
Date: Tue, 6 Feb 2024 22:53:59 +0000
Subject: [PATCH] [vm, gc] Avoid TSAN warning for accessing Page::top_.

TEST=tsan
Bug: https://github.com/dart-lang/sdk/issues/54766
Change-Id: Ic297b426977b835d11c2a98357839f1df8cfbc39
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349711
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
---
 runtime/platform/atomic.h    | 14 --------------
 runtime/vm/heap/compactor.cc |  2 +-
 runtime/vm/heap/page.h       | 10 +++++-----
 3 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/runtime/platform/atomic.h b/runtime/platform/atomic.h
index 13176ce5c6b..526c70fd98d 100644
--- a/runtime/platform/atomic.h
+++ b/runtime/platform/atomic.h
@@ -153,20 +153,6 @@ static inline T LoadRelaxed(const T* ptr) {
       std::memory_order_relaxed);
 }
 
-template <typename T>
-static inline T LoadAcquire(const T* ptr) {
-  static_assert(sizeof(std::atomic<T>) == sizeof(T));
-  return reinterpret_cast<const std::atomic<T>*>(ptr)->load(
-      std::memory_order_acquire);
-}
-
-template <typename T>
-static inline void StoreRelease(T* ptr, T value) {
-  static_assert(sizeof(std::atomic<T>) == sizeof(T));
-  reinterpret_cast<std::atomic<T>*>(ptr)->store(value,
-                                                std::memory_order_release);
-}
-
 }  // namespace dart
 
 #endif  // RUNTIME_PLATFORM_ATOMIC_H_
diff --git a/runtime/vm/heap/compactor.cc b/runtime/vm/heap/compactor.cc
index 09e9ef04792..aa83b9a5875 100644
--- a/runtime/vm/heap/compactor.cc
+++ b/runtime/vm/heap/compactor.cc
@@ -114,7 +114,7 @@ void Page::AllocateForwardingPage() {
   ASSERT((object_start() + sizeof(ForwardingPage)) < object_end());
   ASSERT(Utils::IsAligned(sizeof(ForwardingPage), kObjectAlignment));
   top_ -= sizeof(ForwardingPage);
-  forwarding_page_ = reinterpret_cast<ForwardingPage*>(top_);
+  forwarding_page_ = reinterpret_cast<ForwardingPage*>(top_.load());
 }
 
 struct Partition {
diff --git a/runtime/vm/heap/page.h b/runtime/vm/heap/page.h
index af21f283973..75de57017b6 100644
--- a/runtime/vm/heap/page.h
+++ b/runtime/vm/heap/page.h
@@ -130,8 +130,8 @@ class Page {
   // the TLAB was acquired, not the current boundaries. An object between
   // original_top and top may still be in use by Dart code that has eliminated
   // write barriers.
-  uword original_top() const { return LoadAcquire(&top_); }
-  uword original_end() const { return LoadRelaxed(&end_); }
+  uword original_top() const { return top_.load(std::memory_order_acquire); }
+  uword original_end() const { return end_.load(std::memory_order_relaxed); }
   static intptr_t original_top_offset() { return OFFSET_OF(Page, top_); }
   static intptr_t original_end_offset() { return OFFSET_OF(Page, end_); }
 
@@ -209,7 +209,7 @@ class Page {
     owner_ = nullptr;
     uword old_top = top_;
     uword new_top = thread->top();
-    StoreRelease(&top_, new_top);
+    top_.store(new_top, std::memory_order_release);
     thread->set_top(0);
     thread->set_end(0);
     thread->set_true_end(0);
@@ -314,10 +314,10 @@ class Page {
   // The address of the next allocation. If owner is non-NULL, this value is
   // stale and the current value is at owner->top_. Called "NEXT" in the
   // original Cheney paper.
-  uword top_;
+  RelaxedAtomic<uword> top_;
 
   // The address after the last allocatable byte in this page.
-  uword end_;
+  RelaxedAtomic<uword> end_;
 
   // Objects below this address have survived a scavenge.
   uword survivor_end_;