[vm] More workarounds for false-positives in the profiler due to TSAN not understanding thread_suspend.

TEST=tsan
Bug: #61509
Change-Id: I4e8e2d949f4df88bb401dbe5997b59491988c05a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450102
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/vm/os_thread.h

@@ -9,6 +9,7 @@
 #include "platform/globals.h"
 #include "platform/safe_stack.h"
 #include "platform/synchronization.h"
+#include "platform/thread_sanitizer.h"
 #include "platform/threads.h"
 #include "platform/utils.h"
 #include "vm/allocation.h"
@@ -247,6 +248,8 @@ class OSThread : public BaseThread {
   // We could eliminate this requirement if the windows thread interrupter
   // is implemented differently.
   ThreadState* thread() const { return thread_; }
+  NO_SANITIZE_THREAD
+  ThreadState* thread_ignore_race() const { return thread_; }
   void set_thread(ThreadState* value) { thread_ = value; }
 
   static void Cleanup();

runtime/vm/profiler.cc

@@ -209,6 +209,16 @@ static uword* LoadStackSlot(uword* ptr) {
   return reinterpret_cast<uword*>(*ptr);
 }
 
+#if defined(DART_HOST_OS_MACOS)
+// Mac profiling is cross-thread and TSAN doesn't know that thread_suspend
+// establishes synchronization.
+#define IGNORE_RACE(x) x##_ignore_race
+#define IGNORE_RACE2(x) x##IgnoreRace
+#else
+#define IGNORE_RACE(x) x
+#define IGNORE_RACE2(x) x
+#endif
+
 // The layout of C stack frames.
 #if defined(HOST_ARCH_IA32) || defined(HOST_ARCH_X64) ||                       \
     defined(HOST_ARCH_ARM) || defined(HOST_ARCH_ARM64)
@@ -379,7 +389,8 @@ static bool GetAndValidateThreadStackBounds(OSThread* os_thread,
 
 #if defined(DART_INCLUDE_SIMULATOR)
   const bool use_simulator_stack_bounds =
-      FLAG_use_simulator && thread != nullptr && thread->IsExecutingDartCode();
+      FLAG_use_simulator && thread != nullptr &&
+      thread->IGNORE_RACE2(IsExecutingDartCode)();
   if (use_simulator_stack_bounds) {
     Isolate* isolate = thread->isolate();
     ASSERT(isolate != nullptr);
@@ -1047,8 +1058,8 @@ class ProfilerDartStackWalker : public ProfilerStackWalker {
                           uword lr,
                           bool allocation_sample,
                           intptr_t skip_count = 0)
-      : ProfilerStackWalker((thread->isolate() != nullptr)
-                                ? thread->isolate()->main_port()
+      : ProfilerStackWalker((thread->IGNORE_RACE(isolate)() != nullptr)
+                                ? thread->IGNORE_RACE(isolate)()->main_port()
                                 : ILLEGAL_PORT,
                             sample,
                             sample_buffer,
@@ -1061,12 +1072,13 @@ class ProfilerDartStackWalker : public ProfilerStackWalker {
 
   void walk() {
     RELEASE_ASSERT(StubCode::HasBeenInitialized());
-    if (thread_->IsDeoptimizing()) {
+    if (thread_->IGNORE_RACE2(IsDeoptimizing)()) {
       sample_->set_ignore_sample(true);
       return;
     }
 
-    uword* exit_fp = reinterpret_cast<uword*>(thread_->top_exit_frame_info());
+    uword* exit_fp =
+        reinterpret_cast<uword*>(thread_->IGNORE_RACE(top_exit_frame_info)());
     bool has_exit_frame = exit_fp != nullptr;
     if (has_exit_frame) {
       // Exited from compiled code or interpreter.
@@ -1077,13 +1089,14 @@ class ProfilerDartStackWalker : public ProfilerStackWalker {
       pc_ = CallerPC();
       fp_ = CallerFP();
     } else {
-      if (thread_->vm_tag() == VMTag::kDartTagId) {
+      if (thread_->IGNORE_RACE(vm_tag)() == VMTag::kDartTagId) {
         // Running compiled code.
         // Use the FP and PC from the thread interrupt or simulator; already set
         // in the constructor.
 
 #if defined(DART_DYNAMIC_MODULES)
-      } else if (thread_->vm_tag() == VMTag::kDartInterpretedTagId) {
+      } else if (thread_->IGNORE_RACE(vm_tag)() ==
+                 VMTag::kDartInterpretedTagId) {
         // Running interpreter.
         pc_ = reinterpret_cast<uword*>(thread_->interpreter()->get_pc());
         fp_ = reinterpret_cast<uword*>(thread_->interpreter()->get_fp());
@@ -1286,15 +1299,15 @@ static Sample* SetupSample(Thread* thread,
                            bool allocation_sample,
                            ThreadId tid) {
   ASSERT(thread != nullptr);
-  Isolate* isolate = thread->isolate();
+  Isolate* isolate = thread->IGNORE_RACE(isolate)();
   SampleBlockBuffer* buffer = Profiler::sample_block_buffer();
   Sample* sample = allocation_sample ? buffer->ReserveAllocationSample(isolate)
                                      : buffer->ReserveCPUSample(isolate);
   if (sample == nullptr) {
     return nullptr;
   }
   sample->Init(isolate->main_port(), OS::GetCurrentMonotonicMicros(), tid);
-  uword vm_tag = thread->vm_tag();
+  uword vm_tag = thread->IGNORE_RACE(vm_tag)();
 #if defined(DART_INCLUDE_SIMULATOR)
   // When running in the simulator, the runtime entry function address
   // (stored as the vm tag) is the address of a redirect function.
@@ -1307,7 +1320,7 @@ static Sample* SetupSample(Thread* thread,
   }
 #endif
   sample->set_vm_tag(vm_tag);
-  sample->set_user_tag(thread->user_tag());
+  sample->set_user_tag(thread->IGNORE_RACE(user_tag)());
   sample->set_thread_task(thread->task_kind());
   return sample;
 }
@@ -1388,7 +1401,7 @@ void Profiler::SampleThreadSingleFrame(Thread* thread,
   ASSERT(thread != nullptr);
   OSThread* os_thread = thread->os_thread();
   ASSERT(os_thread != nullptr);
-  Isolate* isolate = thread->isolate();
+  Isolate* isolate = thread->IGNORE_RACE(isolate)();
 
   ASSERT(Profiler::sample_block_buffer() != nullptr);
 
@@ -1406,9 +1419,9 @@ void Profiler::SampleThreadSingleFrame(Thread* thread,
 void Profiler::SampleThread(Thread* thread,
                             const InterruptedThreadState& state) {
   ASSERT(thread != nullptr);
-  OSThread* os_thread = thread->os_thread();
+  OSThread* os_thread = thread->IGNORE_RACE(os_thread)();
   ASSERT(os_thread != nullptr);
-  Isolate* isolate = thread->isolate();
+  Isolate* isolate = thread->IGNORE_RACE(isolate)();
 
   // Double check if interrupts are disabled
   // after the thread interrupter decided to send a signal.
@@ -1430,7 +1443,7 @@ void Profiler::SampleThread(Thread* thread,
     return;
   }
 
-  const bool in_dart_code = thread->IsExecutingDartCode();
+  const bool in_dart_code = thread->IGNORE_RACE2(IsExecutingDartCode)();
 
   uintptr_t sp = 0;
   uintptr_t fp = state.fp;
@@ -1478,7 +1491,7 @@ void Profiler::SampleThread(Thread* thread,
   }
 
   if (thread->IsDartMutatorThread()) {
-    if (thread->IsDeoptimizing()) {
+    if (thread->IGNORE_RACE2(IsDeoptimizing)()) {
       counters_.single_frame_sample_deoptimizing.fetch_add(1);
       SampleThreadSingleFrame(thread, sample, pc);
       return;
@@ -1509,7 +1522,7 @@ void Profiler::SampleThread(Thread* thread,
       &counters_, (isolate != nullptr) ? isolate->main_port() : ILLEGAL_PORT,
       sample, isolate->current_sample_block(), stack_lower, stack_upper, pc, fp,
       sp);
-  const bool exited_dart_code = thread->HasExitedDartCode();
+  const bool exited_dart_code = thread->IGNORE_RACE2(HasExitedDartCode)();
   ProfilerDartStackWalker dart_stack_walker(
       thread, sample, isolate->current_sample_block(), pc, fp, sp, lr,
       /* allocation_sample*/ false);

runtime/vm/thread.cc

@@ -1062,10 +1062,20 @@ bool Thread::IsExecutingDartCode() const {
   return (top_exit_frame_info() == 0) && VMTag::IsDartTag(vm_tag());
 }
 
+bool Thread::IsExecutingDartCodeIgnoreRace() const {
+  return (top_exit_frame_info_ignore_race() == 0) &&
+         VMTag::IsDartTag(vm_tag_ignore_race());
+}
+
 bool Thread::HasExitedDartCode() const {
   return (top_exit_frame_info() != 0) && !VMTag::IsDartTag(vm_tag());
 }
 
+bool Thread::HasExitedDartCodeIgnoreRace() const {
+  return (top_exit_frame_info_ignore_race() != 0) &&
+         !VMTag::IsDartTag(vm_tag_ignore_race());
+}
+
 template <class C>
 C* Thread::AllocateReusableHandle() {
   C* handle = reinterpret_cast<C*>(reusable_handles_.AllocateScopedHandle());

runtime/vm/thread.h

@@ -16,6 +16,7 @@
 #include "platform/assert.h"
 #include "platform/atomic.h"
 #include "platform/safe_stack.h"
+#include "platform/thread_sanitizer.h"
 #include "vm/bitfield.h"
 #include "vm/compiler/runtime_api.h"
 #include "vm/constants.h"
@@ -570,6 +571,8 @@ class Thread : public ThreadState, public IntrusiveDListEntry<Thread> {
 
   // The isolate that this thread is operating on, or nullptr if none.
   Isolate* isolate() const { return isolate_; }
+  NO_SANITIZE_THREAD
+  Isolate* isolate_ignore_race() const { return isolate_; }
   static intptr_t isolate_offset() { return OFFSET_OF(Thread, isolate_); }
   static intptr_t isolate_group_offset() {
     return OFFSET_OF(Thread, isolate_group_);
@@ -625,9 +628,11 @@ class Thread : public ThreadState, public IntrusiveDListEntry<Thread> {
 
   // Is |this| executing Dart code?
   bool IsExecutingDartCode() const;
+  bool IsExecutingDartCodeIgnoreRace() const;
 
   // Has |this| exited Dart code?
   bool HasExitedDartCode() const;
+  bool HasExitedDartCodeIgnoreRace() const;
 
   bool HasCompilerState() const { return compiler_state_ != nullptr; }
 
@@ -735,6 +740,8 @@ class Thread : public ThreadState, public IntrusiveDListEntry<Thread> {
   }
 
   uword top_exit_frame_info() const { return top_exit_frame_info_; }
+  NO_SANITIZE_THREAD
+  uword top_exit_frame_info_ignore_race() const { return top_exit_frame_info_; }
   void set_top_exit_frame_info(uword top_exit_frame_info) {
     top_exit_frame_info_ = top_exit_frame_info;
   }
@@ -865,6 +872,8 @@ class Thread : public ThreadState, public IntrusiveDListEntry<Thread> {
 #endif
 
   uword vm_tag() const { return vm_tag_; }
+  NO_SANITIZE_THREAD
+  uword vm_tag_ignore_race() const { return vm_tag_; }
   void set_vm_tag(uword tag) { vm_tag_ = tag; }
   static intptr_t vm_tag_offset() { return OFFSET_OF(Thread, vm_tag_); }
 
@@ -1334,6 +1343,8 @@ class Thread : public ThreadState, public IntrusiveDListEntry<Thread> {
   }
 
   bool IsDeoptimizing() const { return deopt_context_ != nullptr; }
+  NO_SANITIZE_THREAD
+  bool IsDeoptimizingIgnoreRace() const { return deopt_context_ != nullptr; }
   DeoptContext* deopt_context() const { return deopt_context_; }
   void set_deopt_context(DeoptContext* value) {
     ASSERT(value == nullptr || deopt_context_ == nullptr);
@@ -1352,6 +1363,8 @@ class Thread : public ThreadState, public IntrusiveDListEntry<Thread> {
   }
 
   uword user_tag() const { return user_tag_; }
+  NO_SANITIZE_THREAD
+  uword user_tag_ignore_race() const { return user_tag_; }
   static intptr_t user_tag_offset() { return OFFSET_OF(Thread, user_tag_); }
   static intptr_t current_tag_offset() {
     return OFFSET_OF(Thread, current_tag_);

runtime/vm/thread_interrupter_macos.cc

@@ -71,7 +71,7 @@ class ThreadInterrupterMacOS {
     kern_return_t res = KERN_FAILURE;
 #endif
     ASSERT(res == KERN_SUCCESS);
-    Thread* thread = static_cast<Thread*>(os_thread_->thread());
+    Thread* thread = static_cast<Thread*>(os_thread_->thread_ignore_race());
     if (thread == nullptr) {
       return;
     }

runtime/vm/thread_state.h

@@ -6,6 +6,7 @@
 #define RUNTIME_VM_THREAD_STATE_H_
 
 #include "include/dart_api.h"
+#include "platform/thread_sanitizer.h"
 #include "vm/os_thread.h"
 
 namespace dart {
@@ -31,6 +32,8 @@ class ThreadState : public BaseThread {
 
   // OSThread corresponding to this thread.
   OSThread* os_thread() const { return os_thread_; }
+  NO_SANITIZE_THREAD
+  OSThread* os_thread_ignore_race() const { return os_thread_; }
   void set_os_thread(OSThread* os_thread) { os_thread_ = os_thread; }
 
   // The topmost zone used for allocation in this thread.