Don't try to read return address from the stack of another thread on Win64.

runtime/vm/profiler.cc

Index: runtime/vm/profiler.cc
diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc
index 7c1980172931214b3a974397dcb171f4c6bf2244..a76222485bc0b58fd0e02cbc712570667c626715 100644
--- a/runtime/vm/profiler.cc
+++ b/runtime/vm/profiler.cc
@@ -1676,15 +1676,6 @@ static void SetPCMarkerIfSafe(Sample* sample) {
 
   // If FP == SP, the pc marker hasn't been pushed.
   if (fp > sp) {
-#if defined(TARGET_OS_WINDOWS)

[Vyacheslav Egorov (Google), 2014/08/08 12:47:54]

I don't think this code was doing anything.

Stack is growing downwards hence GUARD page can only be reasoned to be below
last accessed SP (and thus FP --- if FP is really FP not some random stack
pointer).

If FP is at the beggining of the page FP+8 is on the same page not on the next
page. Hence if FP is really FP then FP+8 can't be on the guard page (it was
already written to).

Overall logic behind this check is unclear to me.

I would understand this guard if we were reading [FP-8] here. In any case this
is not sufficient to protect us from hitting guard page given how freely Win64
compiler uses RBP.

[Cutch, 2014/08/08 13:24:53]

Your analysis is correct, this logic isn't entirely correct- but, this code was
successfully avoiding a similar flaky crash on Win32. 

-    // If the fp is at the beginning of a page, it may be unsafe to access
-    // the pc marker, because we are reading it from a different thread on
-    // Windows. The next page may be a guard page.
-    const intptr_t kPageMask = VirtualMemory::PageSize() - 1;
-    if ((sample->fp() & kPageMask) == 0) {
-      return;
-    }
-#endif
     const uword pc_marker = *(fp + kPcMarkerSlotFromFp);
     sample->set_pc_marker(pc_marker);
   }
@@ -2047,7 +2038,11 @@ void Profiler::RecordSampleInterruptCallback(
   sample->set_user_tag(isolate->user_tag());
   sample->set_sp(state.sp);
   sample->set_fp(state.fp);
+#if !(defined(TARGET_OS_WINDOWS) && defined(HOST_ARCH_X64))
+  // It is never safe to read other thread's stack unless
+  // other thread is inside Dart code.
   SetPCMarkerIfSafe(sample);

[Cutch, 2014/08/08 13:24:53]

This should either be:

#if !defined(TARGET_OS_WINDOWS)
#endif

or keep the check in SetPCMarkerIfSafe

+#endif
 
   // Walk the call stack.
   if (FLAG_profile_vm) {