From 2684b4dc196ca2da9466aa5391f5c0090739d2f6 Mon Sep 17 00:00:00 2001 From: jimblandy Date: Thu, 14 Jan 2010 19:17:36 +0000 Subject: Breakpad processor: Don't pass Windows stack walking information to all walkers. At the moment, the StackWalker GetCallerFrame member function expects a vector of WindowsFrameInfo structures, even though WindowsFrameInfo is only used or useful on one one implementation (StackWalkerX86). This patch changes StackWalker::GetCallerFrame to no longer expect the WindowsFrameInfo structures, and changes all implementations to match. In particular, StackWalkerX86 is changed to find the WindowsFrameInfo data itself, and store a pointer to whatever it got in the StackFrame object itself (which is really a StackFrameX86). To allow GetCallerFrame implementations to look up stack walking data, StackWalker::resolver_ needs to be made protected, not private. a=jimblandy, r=mmentovai git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@491 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/google_breakpad/processor/stack_frame_cpu.h | 11 ++++++++- src/google_breakpad/processor/stackwalker.h | 13 +++++------ src/processor/stackwalker.cc | 19 +++------------- src/processor/stackwalker_amd64.cc | 4 +--- src/processor/stackwalker_amd64.h | 4 +--- src/processor/stackwalker_arm.cc | 4 +--- src/processor/stackwalker_arm.h | 4 +--- src/processor/stackwalker_ppc.cc | 4 +--- src/processor/stackwalker_ppc.h | 4 +--- src/processor/stackwalker_sparc.cc | 4 +--- src/processor/stackwalker_sparc.h | 12 ++-------- src/processor/stackwalker_x86.cc | 30 +++++++++++++++++-------- src/processor/stackwalker_x86.h | 7 +++--- 13 files changed, 51 insertions(+), 69 deletions(-) (limited to 'src') diff --git a/src/google_breakpad/processor/stack_frame_cpu.h b/src/google_breakpad/processor/stack_frame_cpu.h index 1e414638..8963f6ff 100644 --- a/src/google_breakpad/processor/stack_frame_cpu.h +++ b/src/google_breakpad/processor/stack_frame_cpu.h @@ -44,6 +44,8 @@ namespace google_breakpad { +struct WindowsFrameInfo; + struct StackFrameX86 : public StackFrame { // ContextValidity has one entry for each relevant hardware pointer register // (%eip and %esp) and one entry for each nonvolatile (callee-save) register. @@ -74,7 +76,9 @@ struct StackFrameX86 : public StackFrame { StackFrameX86() : context(), context_validity(CONTEXT_VALID_NONE), - trust(FRAME_TRUST_NONE) {} + trust(FRAME_TRUST_NONE), + windows_frame_info(NULL) {} + ~StackFrameX86(); // Register state. This is only fully valid for the topmost frame in a // stack. In other frames, the values of nonvolatile registers may be @@ -90,6 +94,11 @@ struct StackFrameX86 : public StackFrame { // Amount of trust the stack walker has in the instruction pointer // of this frame. FrameTrust trust; + + // Any stack walking information we found describing + // this.instruction. These may be NULL if we couldn't find the + // appropriate information. + WindowsFrameInfo *windows_frame_info; }; struct StackFramePPC : public StackFrame { diff --git a/src/google_breakpad/processor/stackwalker.h b/src/google_breakpad/processor/stackwalker.h index 27085585..3b62eacc 100644 --- a/src/google_breakpad/processor/stackwalker.h +++ b/src/google_breakpad/processor/stackwalker.h @@ -48,12 +48,10 @@ namespace google_breakpad { class CallStack; class CodeModules; -template class linked_ptr; class MemoryRegion; class MinidumpContext; class SourceLineResolverInterface; struct StackFrame; -struct WindowsFrameInfo; class SymbolSupplier; class SystemInfo; @@ -118,6 +116,10 @@ class Stackwalker { // This field is optional and may be NULL. const CodeModules *modules_; + protected: + // The SourceLineResolver implementation. + SourceLineResolverInterface *resolver_; + private: // Obtains the context frame, the innermost called procedure in a stack // trace. Returns NULL on failure. GetContextFrame allocates a new @@ -133,15 +135,10 @@ 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, - const vector< linked_ptr > &stack_frame_info) = 0; + virtual StackFrame* GetCallerFrame(const CallStack *stack) = 0; // The optional SymbolSupplier for resolving source line info. SymbolSupplier *supplier_; - - // The SourceLineResolver implementation - SourceLineResolverInterface *resolver_; }; diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index b40ed0c6..ed3da7cf 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc @@ -47,7 +47,6 @@ #include "processor/linked_ptr.h" #include "processor/logging.h" #include "processor/scoped_ptr.h" -#include "processor/windows_frame_info.h" #include "processor/stackwalker_ppc.h" #include "processor/stackwalker_sparc.h" #include "processor/stackwalker_x86.h" @@ -65,8 +64,8 @@ Stackwalker::Stackwalker(const SystemInfo *system_info, : system_info_(system_info), memory_(memory), modules_(modules), - supplier_(supplier), - resolver_(resolver) { + resolver_(resolver), + supplier_(supplier) { } @@ -75,11 +74,6 @@ bool Stackwalker::Walk(CallStack *stack) { assert(stack); stack->Clear(); - // stack_frame_info parallels the CallStack. The vector is passed to the - // GetCallerFrame function. It contains information that may be helpful - // for stackwalking. - vector< linked_ptr > stack_frame_info; - // Begin with the context frame, and keep getting callers until there are // no more. @@ -91,8 +85,6 @@ bool Stackwalker::Walk(CallStack *stack) { // frame_pointer fields. The frame structure comes from either the // context frame (above) or a caller frame (below). - linked_ptr frame_info; - // Resolve the module information, if a module map was provided. if (modules_) { const CodeModule *module = @@ -119,7 +111,6 @@ bool Stackwalker::Walk(CallStack *stack) { } } resolver_->FillSourceLineInfo(frame.get()); - frame_info.reset(resolver_->FindWindowsFrameInfo(frame.get())); } } @@ -127,12 +118,8 @@ bool Stackwalker::Walk(CallStack *stack) { // over the frame, because the stack now owns it. stack->frames_.push_back(frame.release()); - // Add the frame info to the parallel stack. - stack_frame_info.push_back(frame_info); - frame_info.reset(NULL); - // Get the next frame and take ownership. - frame.reset(GetCallerFrame(stack, stack_frame_info)); + frame.reset(GetCallerFrame(stack)); } return true; diff --git a/src/processor/stackwalker_amd64.cc b/src/processor/stackwalker_amd64.cc index 8b43933a..ccdb80f4 100644 --- a/src/processor/stackwalker_amd64.cc +++ b/src/processor/stackwalker_amd64.cc @@ -72,9 +72,7 @@ StackFrame* StackwalkerAMD64::GetContextFrame() { } -StackFrame* StackwalkerAMD64::GetCallerFrame( - const CallStack *stack, - const vector< linked_ptr > &stack_frame_info) { +StackFrame* StackwalkerAMD64::GetCallerFrame(const CallStack *stack) { if (!memory_ || !stack) { BPLOG(ERROR) << "Can't get caller frame without memory or stack"; return NULL; diff --git a/src/processor/stackwalker_amd64.h b/src/processor/stackwalker_amd64.h index 4972d4fd..d70dd4bc 100644 --- a/src/processor/stackwalker_amd64.h +++ b/src/processor/stackwalker_amd64.h @@ -64,9 +64,7 @@ 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, - const vector< linked_ptr > &stack_frame_info); + virtual StackFrame* GetCallerFrame(const CallStack *stack); // Stores the CPU context corresponding to the innermost stack frame to // be returned by GetContextFrame. diff --git a/src/processor/stackwalker_arm.cc b/src/processor/stackwalker_arm.cc index a26e1e85..4bd49e29 100644 --- a/src/processor/stackwalker_arm.cc +++ b/src/processor/stackwalker_arm.cc @@ -72,9 +72,7 @@ StackFrame* StackwalkerARM::GetContextFrame() { } -StackFrame* StackwalkerARM::GetCallerFrame( - const CallStack *stack, - const vector< linked_ptr > &stack_frame_info) { +StackFrame* StackwalkerARM::GetCallerFrame(const CallStack *stack) { if (!memory_ || !stack) { BPLOG(ERROR) << "Can't get caller frame without memory or stack"; return NULL; diff --git a/src/processor/stackwalker_arm.h b/src/processor/stackwalker_arm.h index b9399dd7..b149bfb7 100644 --- a/src/processor/stackwalker_arm.h +++ b/src/processor/stackwalker_arm.h @@ -64,9 +64,7 @@ class StackwalkerARM : public Stackwalker { // Implementation of Stackwalker, using arm context and stack conventions. // TODO: currently stubbed out, needs CFI symbol dumper support virtual StackFrame* GetContextFrame(); - virtual StackFrame* GetCallerFrame( - const CallStack *stack, - const vector< linked_ptr > &stack_frame_info); + virtual StackFrame* GetCallerFrame(const CallStack *stack); // Stores the CPU context corresponding to the innermost stack frame to // be returned by GetContextFrame. diff --git a/src/processor/stackwalker_ppc.cc b/src/processor/stackwalker_ppc.cc index 3e37012c..da6edc58 100644 --- a/src/processor/stackwalker_ppc.cc +++ b/src/processor/stackwalker_ppc.cc @@ -81,9 +81,7 @@ StackFrame* StackwalkerPPC::GetContextFrame() { } -StackFrame* StackwalkerPPC::GetCallerFrame( - const CallStack *stack, - const vector< linked_ptr > &stack_frame_info) { +StackFrame* StackwalkerPPC::GetCallerFrame(const CallStack *stack) { if (!memory_ || !stack) { BPLOG(ERROR) << "Can't get caller frame without memory or stack"; return NULL; diff --git a/src/processor/stackwalker_ppc.h b/src/processor/stackwalker_ppc.h index 5dea99fa..e9acc405 100644 --- a/src/processor/stackwalker_ppc.h +++ b/src/processor/stackwalker_ppc.h @@ -65,9 +65,7 @@ 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, - const vector< linked_ptr > &stack_frame_info); + virtual StackFrame* GetCallerFrame(const CallStack *stack); // Stores the CPU context corresponding to the innermost stack frame to // be returned by GetContextFrame. diff --git a/src/processor/stackwalker_sparc.cc b/src/processor/stackwalker_sparc.cc index e0d5cfa3..206ea0bc 100644 --- a/src/processor/stackwalker_sparc.cc +++ b/src/processor/stackwalker_sparc.cc @@ -72,9 +72,7 @@ StackFrame* StackwalkerSPARC::GetContextFrame() { } -StackFrame* StackwalkerSPARC::GetCallerFrame( - const CallStack *stack, - const vector< linked_ptr > &stack_frame_info) { +StackFrame* StackwalkerSPARC::GetCallerFrame(const CallStack *stack) { if (!memory_ || !stack) { BPLOG(ERROR) << "Can't get caller frame without memory or stack"; return NULL; diff --git a/src/processor/stackwalker_sparc.h b/src/processor/stackwalker_sparc.h index 2e5e1cee..af96844c 100644 --- a/src/processor/stackwalker_sparc.h +++ b/src/processor/stackwalker_sparc.h @@ -61,18 +61,10 @@ class StackwalkerSPARC : public Stackwalker { SourceLineResolverInterface *resolver); private: - // Implementation of Stackwalker, using x86 context (%ebp, %esp, %eip) and - // stack conventions (saved %ebp at [%ebp], saved %eip at 4[%ebp], or - // alternate conventions as guided by stack_frame_info_). - // Implementation of Stackwalker, using ppc context (stack pointer in %r1, - // saved program counter in %srr0) and stack conventions (saved stack - // pointer at 0(%r1), return address at 8(0(%r1)). // Implementation of Stackwalker, using sparc context (%fp, %sp, %pc) and - // stack conventions (saved %sp at) + // stack conventions virtual StackFrame* GetContextFrame(); - virtual StackFrame* GetCallerFrame( - const CallStack *stack, - const vector< linked_ptr > &stack_frame_info); + virtual StackFrame* GetCallerFrame(const CallStack *stack); // Stores the CPU context corresponding to the innermost stack frame to // be returned by GetContextFrame. diff --git a/src/processor/stackwalker_x86.cc b/src/processor/stackwalker_x86.cc index 49e25b3e..27fdc09c 100644 --- a/src/processor/stackwalker_x86.cc +++ b/src/processor/stackwalker_x86.cc @@ -41,7 +41,7 @@ #include "google_breakpad/processor/code_modules.h" #include "google_breakpad/processor/memory_region.h" #include "google_breakpad/processor/stack_frame_cpu.h" -#include "processor/linked_ptr.h" +#include "google_breakpad/processor/source_line_resolver_interface.h" #include "processor/logging.h" #include "processor/windows_frame_info.h" @@ -66,6 +66,11 @@ StackwalkerX86::StackwalkerX86(const SystemInfo *system_info, } } +StackFrameX86::~StackFrameX86() { + if (windows_frame_info) + delete windows_frame_info; + windows_frame_info = NULL; +} StackFrame* StackwalkerX86::GetContextFrame() { if (!context_ || !memory_) { @@ -86,9 +91,7 @@ StackFrame* StackwalkerX86::GetContextFrame() { } -StackFrame* StackwalkerX86::GetCallerFrame( - const CallStack *stack, - const vector< linked_ptr > &stack_frame_info) { +StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) { if (!memory_ || !stack) { BPLOG(ERROR) << "Can't get caller frame without memory or stack"; return NULL; @@ -96,7 +99,9 @@ StackFrame* StackwalkerX86::GetCallerFrame( StackFrameX86::FrameTrust trust = StackFrameX86::FRAME_TRUST_NONE; StackFrameX86 *last_frame = static_cast( stack->frames()->back()); - WindowsFrameInfo *last_frame_info = stack_frame_info.back().get(); + + WindowsFrameInfo *last_frame_info + = resolver_->FindWindowsFrameInfo(last_frame); // This stackwalker sets each frame's %esp to its value immediately prior // to the CALL into the callee. This means that %esp points to the last @@ -130,13 +135,17 @@ StackFrame* StackwalkerX86::GetCallerFrame( // are unknown, 0 is also used in that case. When that happens, it should // be possible to walk to the next frame without reference to %esp. - int frames_already_walked = stack_frame_info.size(); + int frames_already_walked = stack->frames()->size(); u_int32_t last_frame_callee_parameter_size = 0; if (frames_already_walked >= 2) { - WindowsFrameInfo *last_frame_callee_info = - stack_frame_info[frames_already_walked - 2].get(); + StackFrameX86 *last_frame_callee + = static_cast((*stack->frames()) + [frames_already_walked - 2]); + WindowsFrameInfo *last_frame_callee_info + = last_frame_callee->windows_frame_info; if (last_frame_callee_info && - last_frame_callee_info->valid & WindowsFrameInfo::VALID_PARAMETER_SIZE) { + (last_frame_callee_info->valid + & WindowsFrameInfo::VALID_PARAMETER_SIZE)) { last_frame_callee_parameter_size = last_frame_callee_info->parameter_size; } @@ -433,6 +442,9 @@ StackFrame* StackwalkerX86::GetCallerFrame( // StackFrameX86. frame->instruction = frame->context.eip - 1; + // Save the stack walking info we found for the callee. + last_frame->windows_frame_info = last_frame_info; + return frame; } diff --git a/src/processor/stackwalker_x86.h b/src/processor/stackwalker_x86.h index bd6df1fa..f77c6db8 100644 --- a/src/processor/stackwalker_x86.h +++ b/src/processor/stackwalker_x86.h @@ -64,11 +64,10 @@ class StackwalkerX86 : public Stackwalker { private: // Implementation of Stackwalker, using x86 context (%ebp, %esp, %eip) and // stack conventions (saved %ebp at [%ebp], saved %eip at 4[%ebp], or - // alternate conventions as guided by stack_frame_info_). + // alternate conventions as guided by any WindowsFrameInfo available for the + // code in question.). virtual StackFrame* GetContextFrame(); - virtual StackFrame* GetCallerFrame( - const CallStack *stack, - const vector< linked_ptr > &stack_frame_info); + virtual StackFrame* GetCallerFrame(const CallStack *stack); // Scan the stack starting at location_start, looking for an address // that looks like a valid instruction pointer. Addresses must -- cgit v1.2.1