[Fuchsia] Enable CPU profiling for the standalone VM

runtime/vm/thread_interrupter_fuchsia.cc

Index: runtime/vm/thread_interrupter_fuchsia.cc
diff --git a/runtime/vm/thread_interrupter_fuchsia.cc b/runtime/vm/thread_interrupter_fuchsia.cc
index 2e9b7b98642f39533c3a6080f51e33327eced470..0e6af0e3fd3aea944ec01cddda2564cac2023b75 100644
--- a/runtime/vm/thread_interrupter_fuchsia.cc
+++ b/runtime/vm/thread_interrupter_fuchsia.cc
@@ -11,6 +11,7 @@
 #include <magenta/status.h>
 #include <magenta/syscalls.h>
 #include <magenta/syscalls/debug.h>
+#include <magenta/syscalls/object.h>
 #include <magenta/types.h>
 
 #include "vm/flags.h"
@@ -31,56 +32,91 @@ DECLARE_FLAG(bool, trace_thread_interrupter);
 // When MG-430 is resolved, the code below should be rewritten to use whatever
 // feature is added for it.
 
-// TODO(MG-795): The profiler is currently off by default on Fuchsia because
-// suspending a thread that is in a call to pthread_cond_wait() causes
-// pthread_cond_wait() to return ETIMEDOUT.
+// A scope within which a target thread is suspended. When the scope is exited,
+// the thread is resumed and its handle is closed.
+class ThreadSuspendScope {
+ public:
+  explicit ThreadSuspendScope(mx_handle_t thread_handle)
+      : thread_handle_(thread_handle), suspended_(true) {
+    mx_status_t status = mx_task_suspend(thread_handle);
+    // If a thread is somewhere where suspend is impossible, mx_task_suspend()
+    // can return ERR_NOT_SUPPORTED.
+    if ((status != NO_ERROR) && (status != ERR_NOT_SUPPORTED)) {
+      if (FLAG_trace_thread_interrupter) {
+        OS::PrintErr("ThreadInterrupter: mx_task_suspend failed: %s\n",
+            mx_status_get_string(status));
+      }
+      suspended_ = false;
+    }

[siva, 2017/06/02 18:28:32]

Not sure I understand why suspended_ remains true when status returned is
ERR_NOT_SUPPORTED, when suspended_ is true we end up calling
PollThreadUntilSuspended which would loop for ever right?

[zra, 2017/06/02 20:39:01]

Whoops. You're right. Changed to set suspended_ to false whenever status !=
NO_ERROR.

+  }
+
+  ~ThreadSuspendScope() {
+    if (suspended_) {
+      mx_status_t status = mx_task_resume(thread_handle_, 0);
+      if (status != NO_ERROR) {
+        // If we fail to resume a thread, then it's likely the program will
+        // hang. Crash instead.
+        FATAL1("mx_task_resume failed: %s", mx_status_get_string(status));
+      }
+    }
+    mx_handle_close(thread_handle_);
+  }
+
+  bool suspended() const { return suspended_; }
+
+ private:
+  mx_handle_t thread_handle_;
+  bool suspended_;
+
+  DISALLOW_ALLOCATION();
+  DISALLOW_COPY_AND_ASSIGN(ThreadSuspendScope);
+};
 
 class ThreadInterrupterFuchsia : public AllStatic {
  public:
   static bool GrabRegisters(mx_handle_t thread, InterruptedThreadState* state) {
-    // TODO(zra): Enable this when mx_thread_read_state() works on suspended
-    // threads.
-    while (false) {
-      char buf[MX_MAX_THREAD_STATE_SIZE];
-      uint32_t regset_size = MX_MAX_THREAD_STATE_SIZE;
-      mx_status_t status = mx_thread_read_state(
-          thread, MX_THREAD_STATE_REGSET0, &buf[0], regset_size, &regset_size);
-      if (status != NO_ERROR) {
+    char buf[MX_MAX_THREAD_STATE_SIZE];
+    uint32_t regset_size = MX_MAX_THREAD_STATE_SIZE;
+    mx_status_t status = mx_thread_read_state(
+        thread, MX_THREAD_STATE_REGSET0, &buf[0], regset_size, &regset_size);
+    if (status != NO_ERROR) {
+      if (FLAG_trace_thread_interrupter) {
         OS::PrintErr("ThreadInterrupter failed to get registers: %s\n",
-                     mx_status_get_string(status));
-        return false;
+                   mx_status_get_string(status));
       }
+      return false;
+    }
 #if defined(TARGET_ARCH_X64)
-      mx_x86_64_general_regs_t* regs =
-          reinterpret_cast<mx_x86_64_general_regs_t*>(&buf[0]);
-      state->pc = static_cast<uintptr_t>(regs->rip);
-      state->fp = static_cast<uintptr_t>(regs->rbp);
-      state->csp = static_cast<uintptr_t>(regs->rsp);
-      state->dsp = static_cast<uintptr_t>(regs->rsp);
+    mx_x86_64_general_regs_t* regs =
+        reinterpret_cast<mx_x86_64_general_regs_t*>(&buf[0]);
+    state->pc = static_cast<uintptr_t>(regs->rip);
+    state->fp = static_cast<uintptr_t>(regs->rbp);
+    state->csp = static_cast<uintptr_t>(regs->rsp);
+    state->dsp = static_cast<uintptr_t>(regs->rsp);
 #elif defined(TARGET_ARCH_ARM64)
-      mx_arm64_general_regs_t* regs =
-          reinterpret_cast<mx_arm64_general_regs_t*>(&buf[0]);
-      state->pc = static_cast<uintptr_t>(regs->pc);
-      state->fp = static_cast<uintptr_t>(regs->r[FPREG]);
-      state->csp = static_cast<uintptr_t>(regs->sp);
-      state->dsp = static_cast<uintptr_t>(regs->r[SPREG]);
-      state->lr = static_cast<uintptr_t>(regs->lr);
+    mx_arm64_general_regs_t* regs =
+        reinterpret_cast<mx_arm64_general_regs_t*>(&buf[0]);
+    state->pc = static_cast<uintptr_t>(regs->pc);
+    state->fp = static_cast<uintptr_t>(regs->r[FPREG]);
+    state->csp = static_cast<uintptr_t>(regs->sp);
+    state->dsp = static_cast<uintptr_t>(regs->r[SPREG]);
+    state->lr = static_cast<uintptr_t>(regs->lr);
 #else
 #error "Unsupported architecture"
 #endif
-    }
     return true;
   }
 
 
   static void Interrupt(OSThread* os_thread) {
+    ASSERT(os_thread->id() != MX_KOID_INVALID);
     ASSERT(!OSThread::Compare(OSThread::GetCurrentThreadId(), os_thread->id()));
     mx_status_t status;
 
     // Get a handle on the target thread.
-    mx_koid_t target_thread_koid = os_thread->id();
+    const mx_koid_t target_thread_koid = os_thread->id();
     if (FLAG_trace_thread_interrupter) {
-      OS::Print("ThreadInterrupter: interrupting thread with koid=%d\n",
+      OS::PrintErr("ThreadInterrupter: interrupting thread with koid=%d\n",
                 target_thread_koid);
     }
     mx_handle_t target_thread_handle;
@@ -88,56 +124,96 @@ class ThreadInterrupterFuchsia : public AllStatic {
                                  MX_RIGHT_SAME_RIGHTS, &target_thread_handle);
     if (status != NO_ERROR) {
       if (FLAG_trace_thread_interrupter) {
-        OS::Print("ThreadInterrupter failed to get the thread handle: %s\n",
+        OS::PrintErr("ThreadInterrupter: mx_object_get_child failed: %s\n",
                   mx_status_get_string(status));
       }
-      FATAL1("mx_object_get_child failed: %s", mx_status_get_string(status));
+      return;
     }
     if (target_thread_handle == MX_HANDLE_INVALID) {
-      FATAL("ThreadInterrupter got an invalid target thread handle!");
+      if (FLAG_trace_thread_interrupter) {
+        OS::PrintErr("ThreadInterrupter: mx_object_get_child gave an invalid "
+                  "thread handle!");
+      }
+      return;
+    }
+
+    // This scope suspends the thread. When we exit the scope, the thread is
+    // resumed, and the thread handle is closed.
+    ThreadSuspendScope tss(target_thread_handle);
+    if (!tss.suspended()) {
+      return;
     }
 
-    // Pause the target thread.
-    status = mx_task_suspend(target_thread_handle);
+    // Check that the thread is suspended.
+    status = PollThreadUntilSuspended(target_thread_handle);
     if (status != NO_ERROR) {
-      if (FLAG_trace_thread_interrupter) {
-        OS::Print("ThreadInterrupter failed to suspend thread %ld: %s\n",
-                  static_cast<intptr_t>(os_thread->id()),
-                  mx_status_get_string(status));
-      }
-      mx_handle_close(target_thread_handle);
-      FATAL1("mx_task_suspend failed: %s", mx_status_get_string(status));
+      return;
+    }
+
+    // Grab the target thread's registers.
+    InterruptedThreadState its;
+    if (!GrabRegisters(target_thread_handle, &its)) {
+      return;
+    }

[siva, 2017/06/02 18:28:32]

Why not change the API of mx_thread_read_state(...) to return an error
indicating the thread is running MX_THREAD_STATE_RUNNING when called on a
running thread.

That way we could

suspend the thread
while (true) {
  retval = GrabRegisters
  if (retval == NO_ERROR) {
     Sample the thread;
     return;
  }
  if (retval == MX_THREAD_STATE_RUNNING) {
    continue;
  }
  handle error case;
}

[dje, 2017/06/02 18:48:51]

This exists as ERR_BAD_STATE.
https://fuchsia.googlesource.com/magenta/+/master/docs/syscalls/thread_read_s...

[siva, 2017/06/02 19:43:57]

ERR_BAD_STATE could potentially be returned for other kinds of errors too right
not just for the fact that the thread has not been suspended.

+    // Currently we sample only threads that are associated
+    // with an isolate. It is safe to call 'os_thread->thread()'
+    // here as the thread which is being queried is suspended.
+    Thread* thread = os_thread->thread();
+    if (thread != NULL) {
+      Profiler::SampleThread(thread, its);
+    }
+  }
+
+ private:
+  static const char* ThreadStateGetString(uint32_t state) {
+    switch (state) {
+      case MX_THREAD_STATE_NEW:
+        return "MX_THREAD_STATE_NEW";
+      case MX_THREAD_STATE_RUNNING:
+        return "MX_THREAD_STATE_RUNNING";
+      case MX_THREAD_STATE_SUSPENDED:
+        return "MX_THREAD_STATE_SUSPENDED";
+      case MX_THREAD_STATE_BLOCKED:
+        return "MX_THREAD_STATE_BLOCKED";
+      case MX_THREAD_STATE_DYING:
+        return "MX_THREAD_STATE_DYING";
+      case MX_THREAD_STATE_DEAD:
+        return "MX_THREAD_STATE_DEAD";
+      default:
+        return "<Unknown>";
     }
+  }
 
-    // TODO(zra): Enable this when mx_thread_read_state() works on suspended
-    // threads.
-    while (false) {
-      // Grab the target thread's registers.
-      InterruptedThreadState its;
-      if (!GrabRegisters(target_thread_handle, &its)) {
-        // Failed to get thread registers.
-        status = mx_task_resume(target_thread_handle, 0);
-        if (status != NO_ERROR) {
-          FATAL1("mx_task_resume failed: %s", mx_status_get_string(status));
+  static mx_status_t PollThreadUntilSuspended(mx_handle_t thread_handle) {
+    while (true) {
+      mx_info_thread_t thread_info;
+      mx_status_t status = mx_object_get_info(thread_handle,
+                                              MX_INFO_THREAD,
+                                             &thread_info,
+                                             sizeof(thread_info),
+                                             NULL,
+                                             NULL);
+      if (status != NO_ERROR) {
+        if (FLAG_trace_thread_interrupter) {
+          OS::PrintErr("ThreadInterrupter: mx_object_get_info failed: %s\n",
+              mx_status_get_string(status));
         }
-        mx_handle_close(target_thread_handle);
-        return;
+        return status;
       }
-      // Currently we sample only threads that are associated
-      // with an isolate. It is safe to call 'os_thread->thread()'
-      // here as the thread which is being queried is suspended.
-      Thread* thread = os_thread->thread();
-      if (thread != NULL) {
-        Profiler::SampleThread(thread, its);
+      if (thread_info.state == MX_THREAD_STATE_SUSPENDED) {
+        // Success.
+        return NO_ERROR;
       }
+      if (thread_info.state == MX_THREAD_STATE_RUNNING) {
+        // Poll.
+        continue;
+      }
+      if (FLAG_trace_thread_interrupter) {
+        OS::PrintErr("ThreadInterrupter: Thread is not suspended: %s\n",
+            ThreadStateGetString(thread_info.state));
+      }
+      return ERR_BAD_STATE;
     }
-
-    // Resume the target thread.
-    status = mx_task_resume(target_thread_handle, 0);
-    if (status != NO_ERROR) {
-      FATAL1("mx_task_resume failed: %s", mx_status_get_string(status));
-    }
-    mx_handle_close(target_thread_handle);
   }
 };
 
@@ -149,12 +225,12 @@ bool ThreadInterrupter::IsDebuggerAttached() {
 
 void ThreadInterrupter::InterruptThread(OSThread* thread) {
   if (FLAG_trace_thread_interrupter) {
-    OS::Print("ThreadInterrupter suspending %p\n",
+    OS::PrintErr("ThreadInterrupter suspending %p\n",
               reinterpret_cast<void*>(thread->id()));
   }
   ThreadInterrupterFuchsia::Interrupt(thread);
   if (FLAG_trace_thread_interrupter) {
-    OS::Print("ThreadInterrupter resuming %p\n",
+    OS::PrintErr("ThreadInterrupter resuming %p\n",
               reinterpret_cast<void*>(thread->id()));
   }
 }