Bug 859745 - Install sane unwinding limit for SPS/breakpad. r=ted
authorJulian Seward <jseward@acm.org>
Mon, 22 Apr 2013 17:10:54 +0200
changeset 129477 b2d2b365e2e5c3e821c06e3a955a3e6adfadf04f
parent 129476 6c9af867443cab152d6b1997f769e652e6567fd1
child 129479 675cf08dc15415871de164ba7ace3052993d0e35
push id26844
push userjseward@mozilla.com
push dateMon, 22 Apr 2013 15:12:01 +0000
treeherdermozilla-inbound@b2d2b365e2e5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersted
bugs859745
milestone23.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 859745 - Install sane unwinding limit for SPS/breakpad. r=ted
toolkit/crashreporter/google-breakpad/src/google_breakpad/processor/stackwalker.h
toolkit/crashreporter/google-breakpad/src/processor/stackwalker.cc
tools/profiler/UnwinderThread2.cpp
--- a/toolkit/crashreporter/google-breakpad/src/google_breakpad/processor/stackwalker.h
+++ b/toolkit/crashreporter/google-breakpad/src/google_breakpad/processor/stackwalker.h
@@ -83,17 +83,20 @@ class Stackwalker {
   // argument.  If no suitable concrete subclass exists, returns NULL.
   static Stackwalker* StackwalkerForCPU(
      const SystemInfo* system_info,
      MinidumpContext* context,
      MemoryRegion* memory,
      const CodeModules* modules,
      StackFrameSymbolizer* resolver_helper);
 
-  static void set_max_frames(uint32_t max_frames) { max_frames_ = max_frames; }
+  static void set_max_frames(uint32_t max_frames) {
+    max_frames_ = max_frames;
+    max_frames_set_ = true;
+  }
   static uint32_t max_frames() { return max_frames_; }
 
  protected:
   // system_info identifies the operating system, NULL or empty if unknown.
   // memory identifies a MemoryRegion that provides the stack memory
   // for the stack to walk.  modules, if non-NULL, is a CodeModules
   // object that is used to look up which code module each stack frame is
   // associated with.  frame_symbolizer is a StackFrameSymbolizer object that
@@ -191,14 +194,19 @@ class Stackwalker {
   // the end of the stack has been reached).  GetCallerFrame allocates a new
   // StackFrame (or StackFrame subclass), ownership of which is taken by
   // the caller.
   virtual StackFrame* GetCallerFrame(const CallStack* stack) = 0;
 
   // The maximum number of frames Stackwalker will walk through.
   // This defaults to 1024 to prevent infinite loops.
   static uint32_t max_frames_;
+
+  // Keep track of whether max_frames_ has been set by the user, since
+  // it affects whether or not an error message is printed in the case
+  // where an unwind got stopped by the limit.
+  static bool max_frames_set_;
 };
 
 }  // namespace google_breakpad
 
 
 #endif  // GOOGLE_BREAKPAD_PROCESSOR_STACKWALKER_H__
--- a/toolkit/crashreporter/google-breakpad/src/processor/stackwalker.cc
+++ b/toolkit/crashreporter/google-breakpad/src/processor/stackwalker.cc
@@ -52,16 +52,17 @@
 #include "processor/stackwalker_x86.h"
 #include "processor/stackwalker_amd64.h"
 #include "processor/stackwalker_arm.h"
 
 namespace google_breakpad {
 
 const int Stackwalker::kRASearchWords = 30;
 uint32_t Stackwalker::max_frames_ = 1024;
+bool Stackwalker::max_frames_set_ = false;
 
 Stackwalker::Stackwalker(const SystemInfo* system_info,
                          MemoryRegion* memory,
                          const CodeModules* modules,
                          StackFrameSymbolizer* frame_symbolizer)
     : system_info_(system_info),
       memory_(memory),
       modules_(modules),
@@ -120,17 +121,20 @@ bool Stackwalker::Walk(CallStack* stack,
         modules_without_symbols->push_back(frame->module);
       }
     }
 
     // Add the frame to the call stack.  Relinquish the ownership claim
     // over the frame, because the stack now owns it.
     stack->frames_.push_back(frame.release());
     if (stack->frames_.size() > max_frames_) {
-      BPLOG(ERROR) << "The stack is over " << max_frames_ << " frames.";
+      // Only emit an error message in the case where the limit that we
+      // reached is the default limit, not set by the user.
+      if (!max_frames_set_)
+        BPLOG(ERROR) << "The stack is over " << max_frames_ << " frames.";
       break;
     }
 
     // Get the next frame and take ownership.
     frame.reset(GetCallerFrame(stack));
   }
 
   return true;
--- a/tools/profiler/UnwinderThread2.cpp
+++ b/tools/profiler/UnwinderThread2.cpp
@@ -1641,16 +1641,22 @@ void do_breakpad_unwind_Buffer(/*OUT*/PC
 # else
 #   error "Unknown plat"
 # endif
 
   google_breakpad::CallStack* stack = new google_breakpad::CallStack();
 
   std::vector<const google_breakpad::CodeModule*>* modules_without_symbols
     = new std::vector<const google_breakpad::CodeModule*>();
+
+  // Set the max number of frames to a reasonably low level.  By
+  // default Breakpad's limit is 1024, which means it can wind up
+  // spending a lot of time looping on corrupted stacks.
+  sw->set_max_frames(256);
+
   bool b = sw->Walk(stack, modules_without_symbols);
   (void)b;
   delete modules_without_symbols;
 
   unsigned int n_frames = stack->frames()->size();
   unsigned int n_frames_good = 0;
   unsigned int n_frames_dubious = 0;