Bug 1346415 - Use FramePointerStackWalk for less deadlocking when stackwalking on x86, r=njn

MozReview-Commit-ID: CAHarvGSuTY

tools/profiler/core/platform.cpp

@@ -3069,5 +3069,16 @@ profiler_call_exit(void* aHandle)
   pseudoStack->pop();
 }
 
+void*
+profiler_get_stack_top()
+{
+  PSAutoLock lock(gPSMutex);
+  ThreadInfo* threadInfo = FindLiveThreadInfo(lock);
+  if (threadInfo) {
+    return threadInfo->StackTop();
+  }
+  return nullptr;
+}
+
 // END externally visible functions
 ////////////////////////////////////////////////////////////////////////

tools/profiler/public/GeckoProfiler.h

@@ -315,6 +315,12 @@ PROFILER_FUNC(double profiler_time(), 0)
 
 PROFILER_FUNC_VOID(profiler_log(const char *str))
 
+// Gets the stack top of the current thread.
+//
+// The thread must have been previously registered with the profiler, otherwise
+// this method will return nullptr.
+PROFILER_FUNC(void* profiler_get_stack_top(), nullptr)
+
 // End of the functions defined whether the profiler is enabled or not.
 
 #if defined(MOZ_GECKO_PROFILER)

xpcom/threads/ThreadStackHelper.cpp

@@ -134,6 +134,7 @@ ThreadStackHelper::ThreadStackHelper()
     | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION
 #endif
     , FALSE, 0);
+  mStackTop = profiler_get_stack_top();
   MOZ_ASSERT(mInitialized);
 #elif defined(XP_MACOSX)
   mThreadID = mach_thread_self();
@@ -205,9 +206,14 @@ ThreadStackHelper::GetStackInternal(Stack& aStack, bool aAppendNativeStack)
     return;
   }
 
+  // NOTE: We can only perform frame pointer stack walking on non win64
+  // platforms, because Win64 always omits frame pointers. We don't want to use
+  // MozStackWalk here, so we just skip collecting stacks entirely.
+#ifndef MOZ_THREADSTACKHELPER_X64
   if (aAppendNativeStack) {
     aStack.EnsureNativeFrameCapacity(Telemetry::HangStack::sMaxNativeFrames);
   }
+#endif
 
   if (::SuspendThread(mThreadID) == DWORD(-1)) {
     MOZ_ASSERT(false);
@@ -218,22 +224,48 @@ ThreadStackHelper::GetStackInternal(Stack& aStack, bool aAppendNativeStack)
   // GetThreadContext to ensure it's really suspended.
   // See https://blogs.msdn.microsoft.com/oldnewthing/20150205-00/?p=44743.
   CONTEXT context;
+  memset(&context, 0, sizeof(context));
   context.ContextFlags = CONTEXT_CONTROL;
   if (::GetThreadContext(mThreadID, &context)) {
     FillStackBuffer();
   }
 
+#ifndef MOZ_THREADSTACKHELPER_X64
   if (aAppendNativeStack) {
     auto callback = [](uint32_t, void* aPC, void*, void* aClosure) {
       Stack* stack = static_cast<Stack*>(aClosure);
       stack->AppendNativeFrame(reinterpret_cast<uintptr_t>(aPC));
     };
 
-    MozStackWalk(callback, /* skipFrames */ 0,
-                 /* maxFrames */ Telemetry::HangStack::sMaxNativeFrames,
-                 reinterpret_cast<void*>(&aStack),
-                 reinterpret_cast<uintptr_t>(mThreadID), nullptr);
+    // Now we need to get our frame pointer, our stack pointer, and our stack
+    // top. Rather than registering and storing the stack tops ourselves, we use
+    // the gecko profiler to look it up.
+    void** framePointer = reinterpret_cast<void**>(context.Ebp);
+    void** stackPointer = reinterpret_cast<void**>(context.Esp);
+
+    MOZ_ASSERT(mStackTop, "The thread should be registered by the profiler");
+
+    // Double check that the values we pulled for the thread make sense before
+    // walking the stack.
+    if (mStackTop && framePointer >= stackPointer && framePointer < mStackTop) {
+      // NOTE: In bug 1346415 this was changed to use FramePointerStackWalk.
+      // This was done because lowering the background hang timer threshold
+      // would cause it to fire on infra early during the boot process, causing
+      // a deadlock in MozStackWalk when the target thread was holding the
+      // windows-internal lock on the function table, as it would be suspended
+      // before we tried to grab the lock to walk its stack.
+      //
+      // FramePointerStackWalk is implemented entirely in userspace and thus
+      // doesn't have the same issues with deadlocking. Unfortunately as 64-bit
+      // windows is not guaranteed to have frame pointers, the stack walking
+      // code is only enabled on 32-bit windows builds (bug 1357829).
+      FramePointerStackWalk(callback, /* skipFrames */ 0,
+                            /* maxFrames */ Telemetry::HangStack::sMaxNativeFrames,
+                            reinterpret_cast<void*>(&aStack), framePointer,
+                            mStackTop);
+    }
   }
+#endif
 
   MOZ_ALWAYS_TRUE(::ResumeThread(mThreadID) != DWORD(-1));
 

xpcom/threads/ThreadStackHelper.h

@@ -125,6 +125,7 @@ private:
 private:
   bool mInitialized;
   HANDLE mThreadID;
+  void* mStackTop;
 
 #elif defined(XP_MACOSX)
 private: