Bug 1384819 (part 1) - Split MozStackWalk(). r=glandium.

MozStackWalk() is different on Windows to the other platforms. It has two extra
arguments, which can be used to walk the stack of a different thread.

This patch makes those differences clearer. Instead of having a single function
and forbidding those two arguments on non-Windows, it removes those arguments
from MozStackWalk, and splits off MozStackWalkThread() which retains them. This
also allows those arguments to have more appropriate types (HANDLE instead of
uintptr_t; CONTEXT* instead of than void*) and names (aContext instead of
aPlatformData).

The patch also removes unnecessary reinterpret_casts for the aClosure argument
at a couple of MozStackWalk() callsites.

--HG--
extra : rebase_source : 111ab7d6426d7be921facc2264f6db86c501d127

Author: nnethercote

memory/replace/dmd/DMD.cpp

@@ -811,8 +811,7 @@ StackTrace::Get(Thread* aT)
 #else
     int skipFrames = 2;
 #endif
-    bool ok = MozStackWalk(StackWalkCallback, skipFrames, MaxFrames, &tmp, 0,
-                           nullptr);
+    bool ok = MozStackWalk(StackWalkCallback, skipFrames, MaxFrames, &tmp);
 #endif
     if (!ok) {
       tmp.mLength = 0; // re-zero in case the stack walk function changed it
@@ -1631,7 +1630,7 @@ Init(const malloc_table_t* aMallocTable)
   // just call MozStackWalk, because that calls StackWalkInitCriticalAddress().
   // See the comment above StackWalkInitCriticalAddress() for more details.
   (void)MozStackWalk(NopStackWalkCallback, /* skipFrames */ 0,
-                     /* maxFrames */ 1, nullptr, 0, nullptr);
+                     /* maxFrames */ 1, nullptr);
 #endif
 
   gStateLock = InfallibleAllocPolicy::new_<Mutex>();

mozglue/misc/StackWalk.cpp

@@ -110,7 +110,7 @@ my_malloc_logger(uint32_t aType,
   // stack shows up as having two pthread_cond_wait$UNIX2003 frames.
   const char* name = "new_sem_from_pool";
   MozStackWalk(stack_callback, /* skipFrames */ 0, /* maxFrames */ 0,
-               const_cast<char*>(name), 0, nullptr);
+               const_cast<char*>(name));
 }
 
 // This is called from NS_LogInit() and from the stack walking functions, but
@@ -186,7 +186,7 @@ StackWalkInitCriticalAddress()
 }
 #endif
 
-#if defined(_WIN32) && (defined(_M_IX86) || defined(_M_AMD64) || defined(_M_IA64)) // WIN32 x86 stack walking code
+#if MOZ_STACKWALK_SUPPORTS_WINDOWS
 
 #include <windows.h>
 #include <process.h>
@@ -222,7 +222,7 @@ struct WalkStackData
   void** sps;
   uint32_t sp_size;
   uint32_t sp_count;
-  void* platformData;
+  CONTEXT* context;
 };
 
 DWORD gStackWalkThread;
@@ -378,32 +378,34 @@ static void
 WalkStackMain64(struct WalkStackData* aData)
 {
   // Get a context for the specified thread.
-  CONTEXT context;
-  if (!aData->platformData) {
-    memset(&context, 0, sizeof(CONTEXT));
-    context.ContextFlags = CONTEXT_FULL;
-    if (!GetThreadContext(aData->thread, &context)) {
+  CONTEXT context_buf;
+  CONTEXT* context;
+  if (!aData->context) {
+    context = &context_buf;
+    memset(context, 0, sizeof(CONTEXT));
+    context->ContextFlags = CONTEXT_FULL;
+    if (!GetThreadContext(aData->thread, context)) {
       if (aData->walkCallingThread) {
         PrintError("GetThreadContext");
       }
       return;
     }
   } else {
-    context = *static_cast<CONTEXT*>(aData->platformData);
+    context = aData->context;
   }
 
 #if defined(_M_IX86) || defined(_M_IA64)
   // Setup initial stack frame to walk from.
   STACKFRAME64 frame64;
   memset(&frame64, 0, sizeof(frame64));
 #ifdef _M_IX86
-  frame64.AddrPC.Offset    = context.Eip;
-  frame64.AddrStack.Offset = context.Esp;
-  frame64.AddrFrame.Offset = context.Ebp;
+  frame64.AddrPC.Offset    = context->Eip;
+  frame64.AddrStack.Offset = context->Esp;
+  frame64.AddrFrame.Offset = context->Ebp;
 #elif defined _M_IA64
-  frame64.AddrPC.Offset    = context.StIIP;
-  frame64.AddrStack.Offset = context.SP;
-  frame64.AddrFrame.Offset = context.RsBSP;
+  frame64.AddrPC.Offset    = context->StIIP;
+  frame64.AddrStack.Offset = context->SP;
+  frame64.AddrFrame.Offset = context->RsBSP;
 #endif
   frame64.AddrPC.Mode      = AddrModeFlat;
   frame64.AddrStack.Mode   = AddrModeFlat;
@@ -452,7 +454,7 @@ WalkStackMain64(struct WalkStackData* aData)
       aData->process,
       aData->thread,
       &frame64,
-      &context,
+      context,
       nullptr,
       SymFunctionTableAccess64, // function table access routine
       SymGetModuleBase64,       // module base routine
@@ -479,48 +481,48 @@ WalkStackMain64(struct WalkStackData* aData)
     // If we reach a frame in JIT code, we don't have enough information to
     // unwind, so we have to give up.
     if (sJitCodeRegionStart &&
-        (uint8_t*)context.Rip >= sJitCodeRegionStart &&
-        (uint8_t*)context.Rip < sJitCodeRegionStart + sJitCodeRegionSize) {
+        (uint8_t*)context->Rip >= sJitCodeRegionStart &&
+        (uint8_t*)context->Rip < sJitCodeRegionStart + sJitCodeRegionSize) {
       break;
     }
 
     // We must also avoid msmpeg2vdec.dll's JIT region: they don't generate
     // unwind data, so their JIT unwind callback just throws up its hands and
     // terminates the process.
     if (sMsMpegJitCodeRegionStart &&
-        (uint8_t*)context.Rip >= sMsMpegJitCodeRegionStart &&
-        (uint8_t*)context.Rip < sMsMpegJitCodeRegionStart + sMsMpegJitCodeRegionSize) {
+        (uint8_t*)context->Rip >= sMsMpegJitCodeRegionStart &&
+        (uint8_t*)context->Rip < sMsMpegJitCodeRegionStart + sMsMpegJitCodeRegionSize) {
       break;
     }
 
     // 64-bit frame unwinding.
     // Try to look up unwind metadata for the current function.
     ULONG64 imageBase;
     PRUNTIME_FUNCTION runtimeFunction =
-      RtlLookupFunctionEntry(context.Rip, &imageBase, NULL);
+      RtlLookupFunctionEntry(context->Rip, &imageBase, NULL);
 
     if (runtimeFunction) {
       PVOID dummyHandlerData;
       ULONG64 dummyEstablisherFrame;
       RtlVirtualUnwind(UNW_FLAG_NHANDLER,
                        imageBase,
-                       context.Rip,
+                       context->Rip,
                        runtimeFunction,
-                       &context,
+                       context,
                        &dummyHandlerData,
                        &dummyEstablisherFrame,
                        nullptr);
     } else if (firstFrame) {
       // Leaf functions can be unwound by hand.
-      context.Rip = *reinterpret_cast<DWORD64*>(context.Rsp);
-      context.Rsp += sizeof(void*);
+      context->Rip = *reinterpret_cast<DWORD64*>(context->Rsp);
+      context->Rsp += sizeof(void*);
     } else {
       // Something went wrong.
       break;
     }
 
-    addr = context.Rip;
-    spaddr = context.Rsp;
+    addr = context->Rip;
+    spaddr = context->Rsp;
     firstFrame = false;
 #else
 #error "unknown platform"
@@ -618,9 +620,9 @@ WalkStackThread(void* aData)
  */
 
 MFBT_API bool
-MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
-             uint32_t aMaxFrames, void* aClosure, uintptr_t aThread,
-             void* aPlatformData)
+MozStackWalkThread(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
+                   uint32_t aMaxFrames, void* aClosure,
+                   HANDLE aThread, CONTEXT* aContext)
 {
   StackWalkInitCriticalAddress();
   static HANDLE myProcess = nullptr;
@@ -637,8 +639,7 @@ MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
   }
 
   HANDLE currentThread = ::GetCurrentThread();
-  HANDLE targetThread =
-    aThread ? reinterpret_cast<HANDLE>(aThread) : currentThread;
+  HANDLE targetThread = aThread ? aThread : currentThread;
   data.walkCallingThread = (targetThread == currentThread);
 
   // Have to duplicate handle to get a real handle.
@@ -677,7 +678,7 @@ MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
   data.sps = local_sps;
   data.sp_count = 0;
   data.sp_size = ArrayLength(local_sps);
-  data.platformData = aPlatformData;
+  data.context = aContext;
 
   if (aThread) {
     // If we're walking the stack of another thread, we don't need to
@@ -734,6 +735,13 @@ MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
   return data.pc_count != 0;
 }
 
+MFBT_API bool
+MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
+             uint32_t aMaxFrames, void* aClosure)
+{
+  return MozStackWalkThread(aCallback, aSkipFrames, aMaxFrames, aClosure,
+                            nullptr, nullptr);
+}
 
 static BOOL CALLBACK
 callbackEspecial64(
@@ -989,11 +997,8 @@ void DemangleSymbol(const char* aSymbol,
 
 MFBT_API bool
 MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
-             uint32_t aMaxFrames, void* aClosure, uintptr_t aThread,
-             void* aPlatformData)
+             uint32_t aMaxFrames, void* aClosure)
 {
-  MOZ_ASSERT(!aThread);
-  MOZ_ASSERT(!aPlatformData);
   StackWalkInitCriticalAddress();
 
   // Get the frame pointer
@@ -1077,11 +1082,8 @@ unwind_callback(struct _Unwind_Context* context, void* closure)
 
 MFBT_API bool
 MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
-             uint32_t aMaxFrames, void* aClosure, uintptr_t aThread,
-             void* aPlatformData)
+             uint32_t aMaxFrames, void* aClosure)
 {
-  MOZ_ASSERT(!aThread);
-  MOZ_ASSERT(!aPlatformData);
   StackWalkInitCriticalAddress();
   unwind_info info;
   info.callback = aCallback;
@@ -1150,11 +1152,8 @@ MozDescribeCodeAddress(void* aPC, MozCodeAddressDetails* aDetails)
 
 MFBT_API bool
 MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
-             uint32_t aMaxFrames, void* aClosure, uintptr_t aThread,
-             void* aPlatformData)
+             uint32_t aMaxFrames, void* aClosure)
 {
-  MOZ_ASSERT(!aThread);
-  MOZ_ASSERT(!aPlatformData);
   return false;
 }
 

mozglue/misc/StackWalk.h

@@ -15,15 +15,16 @@
 #include <stdint.h>
 
 /**
- * The callback for MozStackWalk.
+ * The callback for MozStackWalk and MozStackWalkThread.
  *
  * @param aFrameNumber  The frame number (starts at 1, not 0).
  * @param aPC           The program counter value.
  * @param aSP           The best approximation possible of what the stack
  *                      pointer will be pointing to when the execution returns
  *                      to executing that at aPC. If no approximation can
  *                      be made it will be nullptr.
- * @param aClosure      Extra data passed in via MozStackWalk().
+ * @param aClosure      Extra data passed in from MozStackWalk() or
+ *                      MozStackWalkThread().
  */
 typedef void
 (*MozWalkStackCallback)(uint32_t aFrameNumber, void* aPC, void* aSP,
@@ -39,24 +40,46 @@ typedef void
  *                     MozStackWalk.
  * @param aMaxFrames   Maximum number of frames to trace.  0 means no limit.
  * @param aClosure     Caller-supplied data passed through to aCallback.
- * @param aThread      The thread for which the stack is to be retrieved.
- *                     Passing null causes us to walk the stack of the
- *                     current thread. On Windows, this is a thread HANDLE.
- *                     It is currently not supported on any other platform.
- * @param aPlatformData Platform specific data that can help in walking the
- *                      stack, this should be nullptr unless you really know
- *                      what you're doing! This needs to be a pointer to a
- *                      CONTEXT on Windows and should not be passed on other
- *                      platforms.
  *
  * May skip some stack frames due to compiler optimizations or code
  * generation.
- *
  */
 MFBT_API bool
 MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
-             uint32_t aMaxFrames, void* aClosure, uintptr_t aThread,
-             void* aPlatformData);
+             uint32_t aMaxFrames, void* aClosure);
+
+#if defined(_WIN32) && \
+    (defined(_M_IX86) || defined(_M_AMD64) || defined(_M_IA64))
+
+#include <windows.h>
+
+#define MOZ_STACKWALK_SUPPORTS_WINDOWS 1
+
+/**
+ * Like MozStackWalk, but walks the stack for another thread.
+ * Call aCallback for the C/C++ stack frames on the current thread, from
+ * the caller of MozStackWalk to main (or above).
+ *
+ * @param aCallback    Same as for MozStackWalk().
+ * @param aSkipFrames  Same as for MozStackWalk().
+ * @param aMaxFrames   Same as for MozStackWalk().
+ * @param aClosure     Same as for MozStackWalk().
+ * @param aThread      The handle of the thread whose stack is to be walked.
+ *                     If 0, walks the current thread.
+ * @param aContext     A CONTEXT, presumably obtained with GetThreadContext()
+ *                     after suspending the thread with SuspendThread(). If
+ *                     null, the CONTEXT will be re-obtained.
+ */
+MFBT_API bool
+MozStackWalkThread(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
+                   uint32_t aMaxFrames, void* aClosure,
+                   HANDLE aThread, CONTEXT* aContext);
+
+#else
+
+#define MOZ_STACKWALK_SUPPORTS_WINDOWS 0
+
+#endif
 
 typedef struct
 {

security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h

@@ -59,7 +59,7 @@ Log(const char* aMessageType,
     if (sStackTraceDepth) {
       msgStream << std::endl << "Stack Trace:";
       MozStackWalk(StackFrameToOStringStream, aFramesToSkip, sStackTraceDepth,
-                   &msgStream, 0, nullptr);
+                   &msgStream);
     }
   }
 

security/sandbox/linux/glue/SandboxCrash.cpp

@@ -98,8 +98,7 @@ SandboxLogCStack()
   // can't walk past the signal trampoline on ARM (bug 968531), and
   // x86 frame pointer walking may or may not work (bug 1082276).
 
-  MozStackWalk(SandboxPrintStackFrame, /* skip */ 3, /* max */ 0,
-               nullptr, 0, nullptr);
+  MozStackWalk(SandboxPrintStackFrame, /* skip */ 3, /* max */ 0, nullptr);
   SANDBOX_LOG_ERROR("end of stack.");
 }
 

toolkit/components/telemetry/KeyedStackCapturer.cpp

@@ -98,8 +98,7 @@ void KeyedStackCapturer::Capture(const nsACString& aKey) {
       static_cast<std::vector<uintptr_t>*>(aClosure);
     stack->push_back(reinterpret_cast<uintptr_t>(aPC));
   };
-  MozStackWalk(callback, /* skipFrames */ 0,
-              /* maxFrames */ 0, reinterpret_cast<void*>(&rawStack), 0, nullptr);
+  MozStackWalk(callback, /* skipFrames */ 0, /* maxFrames */ 0, &rawStack);
   ProcessedStack stack = GetStackAndModules(rawStack);
 
   // Store the new stack info.

toolkit/xre/nsSigHandlers.cpp

@@ -89,8 +89,7 @@ ah_crap_handler(int signum)
          signum);
 
   printf("Stack:\n");
-  MozStackWalk(PrintStackFrame, /* skipFrames */ 2, /* maxFrames */ 0,
-               nullptr, 0, nullptr);
+  MozStackWalk(PrintStackFrame, /* skipFrames */ 2, /* maxFrames */ 0, nullptr);
 
   printf("Sleeping for %d seconds.\n",_gdb_sleep_duration);
   printf("Type 'gdb %s %d' to attach your debugger to this thread.\n",

tools/profiler/core/platform-win32.cpp

@@ -91,10 +91,10 @@ class PlatformData
   HANDLE mProfiledThread;
 };
 
-uintptr_t
+HANDLE
 GetThreadHandle(PlatformData* aData)
 {
-  return (uintptr_t) aData->ProfiledThread();
+  return aData->ProfiledThread();
 }
 
 static const HANDLE kNoThread = INVALID_HANDLE_VALUE;