Bug 894264 - Breakpad Stack scan: don't generate frames we aren't going to use. r=ted.

This commit is contained in:
Julian Seward 2013-08-16 21:03:54 +02:00
parent b3b751a66e
commit f3d2bd7154
13 changed files with 82 additions and 49 deletions

View File

@ -94,6 +94,10 @@ class Stackwalker {
}
static uint32_t max_frames() { return max_frames_; }
static void set_max_frames_scanned(uint32_t max_frames_scanned) {
max_frames_scanned_ = max_frames_scanned;
}
protected:
// system_info identifies the operating system, NULL or empty if unknown.
// memory identifies a MemoryRegion that provides the stack memory
@ -193,8 +197,11 @@ class Stackwalker {
// return NULL on failure or when there are no more caller frames (when
// 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 caller. |stack_scan_allowed| controls whether stack scanning is
// an allowable frame-recovery method, since it is desirable to be able to
// disable stack scanning in performance-critical use cases.
virtual StackFrame* GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed) = 0;
// The maximum number of frames Stackwalker will walk through.
// This defaults to 1024 to prevent infinite loops.
@ -204,6 +211,12 @@ class Stackwalker {
// 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_;
// The maximum number of stack-scanned and otherwise untrustworthy
// frames allowed. Stack-scanning can be expensive, so the option to
// disable or limit it is helpful in cases where unwind performance is
// important. This defaults to 1024, the same as max_frames_.
static uint32_t max_frames_scanned_;
};
} // namespace google_breakpad

View File

@ -56,9 +56,12 @@
namespace google_breakpad {
const int Stackwalker::kRASearchWords = 30;
uint32_t Stackwalker::max_frames_ = 1024;
bool Stackwalker::max_frames_set_ = false;
uint32_t Stackwalker::max_frames_scanned_ = 1024;
Stackwalker::Stackwalker(const SystemInfo* system_info,
MemoryRegion* memory,
const CodeModules* modules,
@ -84,6 +87,10 @@ bool Stackwalker::Walk(CallStack* stack,
// Begin with the context frame, and keep getting callers until there are
// no more.
// Keep track of the number of scanned or otherwise dubious frames seen
// so far, as the caller may have set a limit.
uint32_t n_scanned_frames = 0;
// Take ownership of the pointer returned by GetContextFrame.
scoped_ptr<StackFrame> frame(GetContextFrame());
@ -122,6 +129,17 @@ bool Stackwalker::Walk(CallStack* stack,
}
}
// Keep track of the number of dubious frames so far.
switch (frame.get()->trust) {
case StackFrame::FRAME_TRUST_NONE:
case StackFrame::FRAME_TRUST_SCAN:
case StackFrame::FRAME_TRUST_CFI_SCAN:
n_scanned_frames++;
break;
default:
break;
}
// 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());
@ -134,7 +152,8 @@ bool Stackwalker::Walk(CallStack* stack,
}
// Get the next frame and take ownership.
frame.reset(GetCallerFrame(stack));
bool stack_scan_allowed = n_scanned_frames < max_frames_scanned_;
frame.reset(GetCallerFrame(stack, stack_scan_allowed));
}
return true;

View File

@ -196,7 +196,8 @@ StackFrameAMD64* StackwalkerAMD64::GetCallerByStackScan(
return frame;
}
StackFrame* StackwalkerAMD64::GetCallerFrame(const CallStack* stack) {
StackFrame* StackwalkerAMD64::GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed) {
if (!memory_ || !stack) {
BPLOG(ERROR) << "Can't get caller frame without memory or stack";
return NULL;
@ -214,7 +215,7 @@ StackFrame* StackwalkerAMD64::GetCallerFrame(const CallStack* stack) {
// If CFI failed, or there wasn't CFI available, fall back
// to stack scanning.
if (!new_frame.get()) {
if (stack_scan_allowed && !new_frame.get()) {
new_frame.reset(GetCallerByStackScan(frames));
}

View File

@ -69,7 +69,8 @@ class StackwalkerAMD64 : public Stackwalker {
// Implementation of Stackwalker, using amd64 context (stack pointer in %rsp,
// stack base in %rbp) and stack conventions (saved stack pointer at 0(%rbp))
virtual StackFrame* GetContextFrame();
virtual StackFrame* GetCallerFrame(const CallStack* stack);
virtual StackFrame* GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed);
// Use cfi_frame_info (derived from STACK CFI records) to construct
// the frame that called frames.back(). The caller takes ownership

View File

@ -252,7 +252,8 @@ StackFrameARM* StackwalkerARM::GetCallerByFramePointer(
return frame;
}
StackFrame* StackwalkerARM::GetCallerFrame(const CallStack* stack) {
StackFrame* StackwalkerARM::GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed) {
if (!memory_ || !stack) {
BPLOG(ERROR) << "Can't get caller frame without memory or stack";
return NULL;
@ -274,7 +275,7 @@ StackFrame* StackwalkerARM::GetCallerFrame(const CallStack* stack) {
frame.reset(GetCallerByFramePointer(frames));
// If everuthing failed, fall back to stack scanning.
if (!frame.get())
if (stack_scan_allowed && !frame.get())
frame.reset(GetCallerByStackScan(frames));
// If nothing worked, tell the caller.

View File

@ -69,7 +69,8 @@ class StackwalkerARM : public Stackwalker {
private:
// Implementation of Stackwalker, using arm context and stack conventions.
virtual StackFrame* GetContextFrame();
virtual StackFrame* GetCallerFrame(const CallStack* stack);
virtual StackFrame* GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed);
// Use cfi_frame_info (derived from STACK CFI records) to construct
// the frame that called frames.back(). The caller takes ownership

View File

@ -81,7 +81,8 @@ StackFrame* StackwalkerPPC::GetContextFrame() {
}
StackFrame* StackwalkerPPC::GetCallerFrame(const CallStack* stack) {
StackFrame* StackwalkerPPC::GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed) {
if (!memory_ || !stack) {
BPLOG(ERROR) << "Can't get caller frame without memory or stack";
return NULL;

View File

@ -64,7 +64,8 @@ class StackwalkerPPC : public Stackwalker {
// saved program counter in %srr0) and stack conventions (saved stack
// pointer at 0(%r1), return address at 8(0(%r1)).
virtual StackFrame* GetContextFrame();
virtual StackFrame* GetCallerFrame(const CallStack* stack);
virtual StackFrame* GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed);
// Stores the CPU context corresponding to the innermost stack frame to
// be returned by GetContextFrame.

View File

@ -72,7 +72,8 @@ StackFrame* StackwalkerSPARC::GetContextFrame() {
}
StackFrame* StackwalkerSPARC::GetCallerFrame(const CallStack* stack) {
StackFrame* StackwalkerSPARC::GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed) {
if (!memory_ || !stack) {
BPLOG(ERROR) << "Can't get caller frame without memory or stack";
return NULL;

View File

@ -63,7 +63,8 @@ class StackwalkerSPARC : public Stackwalker {
// Implementation of Stackwalker, using sparc context (%fp, %sp, %pc) and
// stack conventions
virtual StackFrame* GetContextFrame();
virtual StackFrame* GetCallerFrame(const CallStack* stack);
virtual StackFrame* GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed);
// Stores the CPU context corresponding to the innermost stack frame to
// be returned by GetContextFrame.

View File

@ -132,7 +132,8 @@ StackFrame* StackwalkerX86::GetContextFrame() {
StackFrameX86* StackwalkerX86::GetCallerByWindowsFrameInfo(
const vector<StackFrame*> &frames,
WindowsFrameInfo* last_frame_info) {
WindowsFrameInfo* last_frame_info,
bool stack_scan_allowed) {
StackFrame::FrameTrust trust = StackFrame::FRAME_TRUST_NONE;
StackFrameX86* last_frame = static_cast<StackFrameX86*>(frames.back());
@ -340,7 +341,8 @@ StackFrameX86* StackwalkerX86::GetCallerByWindowsFrameInfo(
// frame pointer.
uint32_t location_start = last_frame->context.esp;
uint32_t location, eip;
if (!ScanForReturnAddress(location_start, &location, &eip)) {
if (!stack_scan_allowed
|| !ScanForReturnAddress(location_start, &location, &eip)) {
// if we can't find an instruction pointer even with stack scanning,
// give up.
return NULL;
@ -383,7 +385,8 @@ StackFrameX86* StackwalkerX86::GetCallerByWindowsFrameInfo(
// looking one 32-bit word above that location.
uint32_t location_start = dictionary.get(ustr__ZDraSearchStart()) + 4;
uint32_t location;
if (ScanForReturnAddress(location_start, &location, &eip)) {
if (stack_scan_allowed
&& ScanForReturnAddress(location_start, &location, &eip)) {
// This is a better return address that what program string
// evaluation found. Use it, and set %esp to the location above the
// one where the return address was found.
@ -491,7 +494,8 @@ StackFrameX86* StackwalkerX86::GetCallerByCFIFrameInfo(
}
StackFrameX86* StackwalkerX86::GetCallerByEBPAtBase(
const vector<StackFrame*> &frames) {
const vector<StackFrame*> &frames,
bool stack_scan_allowed) {
StackFrame::FrameTrust trust;
StackFrameX86* last_frame = static_cast<StackFrameX86*>(frames.back());
uint32_t last_esp = last_frame->context.esp;
@ -532,7 +536,8 @@ StackFrameX86* StackwalkerX86::GetCallerByEBPAtBase(
// return address. This can happen if last_frame is executing code
// for a module for which we don't have symbols, and that module
// is compiled without a frame pointer.
if (!ScanForReturnAddress(last_esp, &caller_esp, &caller_eip)) {
if (!stack_scan_allowed
|| !ScanForReturnAddress(last_esp, &caller_esp, &caller_eip)) {
// if we can't find an instruction pointer even with stack scanning,
// give up.
return NULL;
@ -563,7 +568,8 @@ StackFrameX86* StackwalkerX86::GetCallerByEBPAtBase(
return frame;
}
StackFrame* StackwalkerX86::GetCallerFrame(const CallStack* stack) {
StackFrame* StackwalkerX86::GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed) {
if (!memory_ || !stack) {
BPLOG(ERROR) << "Can't get caller frame without memory or stack";
return NULL;
@ -577,7 +583,8 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack* stack) {
WindowsFrameInfo* windows_frame_info
= frame_symbolizer_->FindWindowsFrameInfo(last_frame);
if (windows_frame_info)
new_frame.reset(GetCallerByWindowsFrameInfo(frames, windows_frame_info));
new_frame.reset(GetCallerByWindowsFrameInfo(frames, windows_frame_info,
stack_scan_allowed));
// If the resolver has DWARF CFI information, use that.
if (!new_frame.get()) {
@ -589,7 +596,7 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack* stack) {
// Otherwise, hope that the program was using a traditional frame structure.
if (!new_frame.get())
new_frame.reset(GetCallerByEBPAtBase(frames));
new_frame.reset(GetCallerByEBPAtBase(frames, stack_scan_allowed));
// If nothing worked, tell the caller.
if (!new_frame.get())

View File

@ -74,14 +74,16 @@ class StackwalkerX86 : public Stackwalker {
// alternate conventions as guided by any WindowsFrameInfo available for the
// code in question.).
virtual StackFrame* GetContextFrame();
virtual StackFrame* GetCallerFrame(const CallStack* stack);
virtual StackFrame* GetCallerFrame(const CallStack* stack,
bool stack_scan_allowed);
// Use windows_frame_info (derived from STACK WIN and FUNC records)
// to construct the frame that called frames.back(). The caller
// takes ownership of the returned frame. Return NULL on failure.
StackFrameX86* GetCallerByWindowsFrameInfo(
const vector<StackFrame*> &frames,
WindowsFrameInfo* windows_frame_info);
WindowsFrameInfo* windows_frame_info,
bool stack_scan_allowed);
// Use cfi_frame_info (derived from STACK CFI records) to construct
// the frame that called frames.back(). The caller takes ownership
@ -94,7 +96,8 @@ class StackwalkerX86 : public Stackwalker {
// %ebp points to the saved %ebp --- construct the frame that called
// frames.back(). The caller takes ownership of the returned frame.
// Return NULL on failure.
StackFrameX86* GetCallerByEBPAtBase(const vector<StackFrame*> &frames);
StackFrameX86* GetCallerByEBPAtBase(const vector<StackFrame*> &frames,
bool stack_scan_allowed);
// Stores the CPU context corresponding to the innermost stack frame to
// be returned by GetContextFrame.

View File

@ -1936,13 +1936,17 @@ void do_breakpad_unwind_Buffer(/*OUT*/PCandSP** pairs,
// spending a lot of time looping on corrupted stacks.
sw->set_max_frames(256);
// Set the max number of scanned or otherwise dubious frames
// to the user specified limit
sw->set_max_frames_scanned((sUnwindStackScan > 256) ? 256
: (sUnwindStackScan < 0) ? 0
: sUnwindStackScan);
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;
*pairs = (PCandSP*)calloc(n_frames, sizeof(PCandSP));
*nPairs = n_frames;
@ -1956,26 +1960,6 @@ void do_breakpad_unwind_Buffer(/*OUT*/PCandSP** pairs,
frame_index < n_frames; ++frame_index) {
google_breakpad::StackFrame *frame = stack->frames()->at(frame_index);
bool dubious
= frame->trust == google_breakpad::StackFrame::FRAME_TRUST_SCAN
|| frame->trust == google_breakpad::StackFrame::FRAME_TRUST_CFI_SCAN
|| frame->trust == google_breakpad::StackFrame::FRAME_TRUST_NONE;
if (dubious) {
n_frames_dubious++;
} else {
n_frames_good++;
}
/* Once we've seen more than some threshhold number of dubious
frames, give up. Doing that gives better results than
polluting the profiling results with junk frames. Because
the entries are put into the pairs array starting at the end,
this will leave some initial section of pairs containing
(0,0) values, which correspond to the skipped frames. */
if (n_frames_dubious > (unsigned int)sUnwindStackScan)
break;
if (LOGLEVEL >= 2)
stats_notify_frame(frame->trust);
@ -2025,9 +2009,8 @@ void do_breakpad_unwind_Buffer(/*OUT*/PCandSP** pairs,
}
if (LOGLEVEL >= 3) {
LOGF("BPUnw: unwinder: seqNo %llu, buf %d: got %u frames "
"(%u trustworthy)",
(unsigned long long int)buff->seqNo, buffNo, n_frames, n_frames_good);
LOGF("BPUnw: unwinder: seqNo %llu, buf %d: got %u frames",
(unsigned long long int)buff->seqNo, buffNo, n_frames);
}
if (LOGLEVEL >= 2) {