Fix Win64 flakey crash related to reading from stack guard page.

runtime/vm/profiler.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "platform/address_sanitizer.h"
 #include "platform/memory_sanitizer.h"
 #include "platform/utils.h"
 
 #include "vm/allocation.h"
 #include "vm/atomic.h"
@@ skipping 854 matching lines @@
   const uword original_pc_;
   const uword original_fp_;
   const uword original_sp_;
   uword lower_bound_;
 };
 
 
 static void CopyPCMarkerIfSafe(Sample* sample) {
   ASSERT(sample != NULL);
 
+  if (sample->vm_tag() != VMTag::kDartTagId) {
+    // We can only trust the stack pointer if we are executing Dart code.
+    // See http://dartbug.com/20421 for details.
+    return;
+  }
   uword* fp = reinterpret_cast<uword*>(sample->fp());
   uword* sp = reinterpret_cast<uword*>(sample->sp());
 
   // If FP == SP, the pc marker hasn't been pushed.
   if (fp > sp) {
 #if defined(TARGET_OS_WINDOWS)
     COMPILE_ASSERT(kPcMarkerSlotFromFp < 0);
     // 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 marker is below fp and the previous page may be a guard
     // page.
     const intptr_t kPageMask = VirtualMemory::PageSize() - 1;
     if ((sample->fp() & kPageMask) == 0) {
       return;
     }
 #endif
     uword* pc_marker_ptr = fp + kPcMarkerSlotFromFp;
     // MSan/ASan are unaware of frames initialized by generated code.
     MSAN_UNPOISON(pc_marker_ptr, kWordSize);
     ASAN_UNPOISON(pc_marker_ptr, kWordSize);
     sample->set_pc_marker(*pc_marker_ptr);
   }
 }
 
 
 static void CopyStackBuffer(Sample* sample) {
   ASSERT(sample != NULL);
+  if (sample->vm_tag() != VMTag::kDartTagId) {
+    // We can only trust the stack pointer if we are executing Dart code.
+    // See http://dartbug.com/20421 for details.
+    return;
+  }
   uword* sp = reinterpret_cast<uword*>(sample->sp());
   uword* buffer = sample->GetStackBuffer();
   if (sp != NULL) {
     for (intptr_t i = 0; i < Sample::kStackBufferSizeInWords; i++) {
       MSAN_UNPOISON(sp, kWordSize);
       ASAN_UNPOISON(sp, kWordSize);
       buffer[i] = *sp;
       sp++;
     }
   }
@@ skipping 98 matching lines @@
 #endif
   // Increment counter for vm tag.
   VMTagCounters* counters = isolate->vm_tag_counters();
   ASSERT(counters != NULL);
   counters->Increment(vm_tag);
   sample->set_vm_tag(vm_tag);
   sample->set_user_tag(isolate->user_tag());
   sample->set_sp(sp);
   sample->set_fp(state.fp);
   sample->set_lr(state.lr);
   CopyStackBuffer(sample);

[koda, 2015/02/26 21:05:31]

I would prefer to have the "if" here instead, and then ASSERT inside the static
helper functions, since this is part of the normal control flow rather than an
exceptional condition.

-#if !(defined(TARGET_OS_WINDOWS) && defined(TARGET_ARCH_X64))
-  // It is never safe to read other thread's stack unless on Win64
-  // other thread is inside Dart code.
-  if (vm_tag != VMTag::kDartTagId) {
-    CopyPCMarkerIfSafe(sample);
-  }
-#endif
+  CopyPCMarkerIfSafe(sample);
 
   // Walk the call stack.
   if (FLAG_profile_vm) {
     // Always walk the native stack collecting both native and Dart frames.
     ProfilerNativeStackWalker stackWalker(sample,
                                           stack_lower,
                                           stack_upper,
                                           state.pc,
                                           state.fp,
                                           sp);
@@ skipping 32 matching lines @@
                                             state.pc,
                                             state.fp,
                                             sp);
       stackWalker.walk();
 #endif
     }
   }
 }
 
 }  // namespace dart