Bug 818793 - Add a |aMaxFrames| parameter to NS_StackWalk. r=jlebar,g…

…landium; sr=dbaron.

memory/replace/dmd/DMD.cpp

@@ -707,10 +707,10 @@ class LocationService
 
 class StackTrace
 {
-  static const uint32_t MaxDepth = 24;
+  static const uint32_t MaxFrames = 24;
 
   uint32_t mLength;             // The number of PCs.
-  void* mPcs[MaxDepth];         // The PCs themselves.
+  void* mPcs[MaxFrames];        // The PCs themselves.
 
 public:
   StackTrace() : mLength(0) {}
@@ -751,13 +751,9 @@ class StackTrace
   static void StackWalkCallback(void* aPc, void* aSp, void* aClosure)
   {
     StackTrace* st = (StackTrace*) aClosure;
-
-    // Only fill to MaxDepth.
-    // XXX: bug 818793 will allow early bailouts.
-    if (st->mLength < MaxDepth) {
-      st->mPcs[st->mLength] = aPc;
-      st->mLength++;
-    }
+    MOZ_ASSERT(st->mLength < MaxFrames);
+    st->mPcs[st->mLength] = aPc;
+    st->mLength++;
   }
 
   static int QsortCmp(const void* aA, const void* aB)
@@ -778,7 +774,7 @@ void
 StackTrace::Print(const Writer& aWriter, LocationService* aLocService) const
 {
   if (mLength == 0) {
-    W("   (empty)\n");
+    W("   (empty)\n");  // StackTrace::Get() must have failed
     return;
   }
 
@@ -800,17 +796,33 @@ StackTrace::Get(Thread* aT)
   // https://bugzilla.mozilla.org/show_bug.cgi?id=374829#c8
   // On Linux, something similar can happen;  see bug 824340.
   // So let's just release it on all platforms.
+  nsresult rv;
   StackTrace tmp;
   {
     AutoUnlockState unlock;
-    // In normal operation, skip=3 gets us past various malloc wrappers into
-    // more interesting stuff.  But in test mode we need to skip a bit less to
-    // sufficiently differentiate some similar stacks.
-    uint32_t skip = 2;
-    nsresult rv = NS_StackWalk(StackWalkCallback, skip, &tmp, 0, nullptr);
-    if (NS_FAILED(rv) || tmp.mLength == 0) {
-      tmp.mLength = 0;
-    }
+    uint32_t skipFrames = 2;
+    rv = NS_StackWalk(StackWalkCallback, skipFrames, MaxFrames, &tmp, 0,
+                      nullptr);
+  }
+
+  if (rv == NS_OK) {
+    // Handle the common case first.  All is ok.  Nothing to do.
+  } else if (rv == NS_ERROR_NOT_IMPLEMENTED || rv == NS_ERROR_FAILURE) {
+    tmp.mLength = 0;
+  } else if (rv == NS_ERROR_UNEXPECTED) {
+    // XXX: This |rv| only happens on Mac, and it indicates that we're handling
+    // a call to malloc that happened inside a mutex-handling function.  Any
+    // attempt to create a semaphore (which can happen in printf) could
+    // deadlock.
+    //
+    // However, the most complex thing DMD does after Get() returns is to put
+    // something in a hash table, which might call
+    // InfallibleAllocPolicy::malloc_.  I'm not yet sure if this needs special
+    // handling, hence the forced abort.  Sorry.  If you hit this, please file
+    // a bug and CC nnethercote.
+    MOZ_CRASH();
+  } else {
+    MOZ_CRASH();  // should be impossible
   }
 
   StackTraceTable::AddPtr p = gStackTraceTable->lookupForAdd(&tmp);
@@ -1670,7 +1682,8 @@ Init(const malloc_table_t* aMallocTable)
   // StackWalkInitCriticalAddress() isn't exported from xpcom/, so instead we
   // just call NS_StackWalk, because that calls StackWalkInitCriticalAddress().
   // See the comment above StackWalkInitCriticalAddress() for more details.
-  (void)NS_StackWalk(NopStackWalkCallback, 0, nullptr, 0, nullptr);
+  (void)NS_StackWalk(NopStackWalkCallback, /* skipFrames */ 0,
+                     /* maxFrames */ 1, nullptr, 0, nullptr);
 #endif
 
   gStateLock = InfallibleAllocPolicy::new_<Mutex>();

toolkit/xre/nsSigHandlers.cpp

@@ -77,7 +77,8 @@ ah_crap_handler(int signum)
          signum);
 
   printf("Stack:\n");
-  NS_StackWalk(PrintStackFrame, 2, nullptr, 0, nullptr);
+  NS_StackWalk(PrintStackFrame, /* skipFrames */ 2, /* maxFrames */ 0,
+               nullptr, 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/TableTicker.cpp

@@ -796,10 +796,7 @@ static
 void StackWalkCallback(void* aPC, void* aSP, void* aClosure)
 {
   PCArray* array = static_cast<PCArray*>(aClosure);
-  if (array->count >= array->size) {
-    // too many frames, ignore
-    return;
-  }
+  MOZ_ASSERT(array->count < array->size);
   array->sp_array[array->count] = aSP;
   array->array[array->count] = aPC;
   array->count++;
@@ -828,16 +825,20 @@ void TableTicker::doBacktrace(ThreadProfile &aProfile, TickSample* aSample)
   platformData = aSample->context;
 #endif
 
+  uint32_t maxFrames = array.size - array.count;
 #ifdef XP_MACOSX
   pthread_t pt = GetProfiledThread(platform_data());
   void *stackEnd = reinterpret_cast<void*>(-1);
   if (pt)
     stackEnd = static_cast<char*>(pthread_get_stackaddr_np(pt));
   nsresult rv = NS_OK;
   if (aSample->fp >= aSample->sp && aSample->fp <= stackEnd)
-    rv = FramePointerStackWalk(StackWalkCallback, 0, &array, reinterpret_cast<void**>(aSample->fp), stackEnd);
+    rv = FramePointerStackWalk(StackWalkCallback, /* skipFrames */ 0,
+                               maxFrames, &array,
+                               reinterpret_cast<void**>(aSample->fp), stackEnd);
 #else
-  nsresult rv = NS_StackWalk(StackWalkCallback, 0, &array, thread, platformData);
+  nsresult rv = NS_StackWalk(StackWalkCallback, /* skipFrames */ 0, maxFrames,
+                             &array, thread, platformData);
 #endif
   if (NS_SUCCEEDED(rv)) {
     aProfile.addTag(ProfileEntry('s', "(root)"));

tools/trace-malloc/lib/nsTraceMalloc.c

@@ -914,7 +914,7 @@ stack_callback(void *pc, void *sp, void *closure)
  * without doing anything (such as acquiring locks).
  */
 static callsite *
-backtrace(tm_thread *t, int skip, int *immediate_abort)
+backtrace(tm_thread *t, int skipFrames, int *immediate_abort)
 {
     callsite *site;
     stack_buffer_info *info = &t->backtrace_buf;
@@ -929,7 +929,8 @@ backtrace(tm_thread *t, int skip, int *immediate_abort)
         /* Walk the stack, even if stacks_enabled is false. We do this to
            check if we must set immediate_abort. */
         info->entries = 0;
-        rv = NS_StackWalk(stack_callback, skip, info, 0, NULL);
+        rv = NS_StackWalk(stack_callback, skipFrames, /* maxFrames */ 0, info,
+                          0, NULL);
         *immediate_abort = rv == NS_ERROR_UNEXPECTED;
         if (rv == NS_ERROR_UNEXPECTED || info->entries == 0) {
             t->suppress_tracing--;
@@ -961,10 +962,14 @@ backtrace(tm_thread *t, int skip, int *immediate_abort)
          * https://bugzilla.mozilla.org/show_bug.cgi?id=374829#c8
          */
 
-        /* skip == 0 means |backtrace| should show up, so don't use skip + 1 */
-        /* NB: this call is repeated below if the buffer is too small */
+        /*
+         * skipFrames == 0 means |backtrace| should show up, so don't use
+         * skipFrames + 1.
+         * NB: this call is repeated below if the buffer is too small.
+         */
         info->entries = 0;
-        rv = NS_StackWalk(stack_callback, skip, info, 0, NULL);
+        rv = NS_StackWalk(stack_callback, skipFrames, /* maxFrames */ 0, info,
+                          0, NULL);
         *immediate_abort = rv == NS_ERROR_UNEXPECTED;
         if (rv == NS_ERROR_UNEXPECTED || info->entries == 0) {
             t->suppress_tracing--;
@@ -988,7 +993,8 @@ backtrace(tm_thread *t, int skip, int *immediate_abort)
 
             /* and call NS_StackWalk again */
             info->entries = 0;
-            NS_StackWalk(stack_callback, skip, info, 0, NULL);
+            NS_StackWalk(stack_callback, skipFrames, /* maxFrames */ 0, info,
+                         0, NULL);
 
             /* same stack */
             PR_ASSERT(info->entries * 2 == new_stack_buffer_size);

xpcom/base/StackWalk.h

@@ -15,7 +15,8 @@ namespace mozilla {
 
 nsresult
 FramePointerStackWalk(NS_WalkStackCallback aCallback, uint32_t aSkipFrames,
-                      void *aClosure, void **bp, void *stackEnd);
+                      uint32_t aMaxFrames, void *aClosure, void **bp,
+                      void *stackEnd);
 
 }