From d119a921ea611dc38cfcb7411759ddf2c688603f Mon Sep 17 00:00:00 2001 From: mmentovai Date: Mon, 23 Oct 2006 19:24:58 +0000 Subject: [PATCH] Make stack_frame_info vector hold linked_ptrs instead of objects; make Stackwalker::Walk create and return a CallStack instead of filling a caller-supplied one (#54). r=bryner Interface change: Stackwalker::Walk and MinidumpProcessor::Process now return a new CallStack*. http://groups.google.com/group/airbag-dev/browse_thread/thread/d2bad5d7c115c3fe git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@45 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/google/minidump_processor.h | 6 +- src/processor/minidump_processor.cc | 18 ++-- src/processor/minidump_processor_unittest.cc | 48 ++++++----- src/processor/minidump_stackwalk.cc | 7 +- src/processor/source_line_resolver.cc | 85 ++++++++++--------- src/processor/source_line_resolver.h | 7 +- .../source_line_resolver_unittest.cc | 50 ++++++----- src/processor/stack_frame_info.h | 13 +++ src/processor/stackwalker.cc | 29 ++++--- src/processor/stackwalker.h | 22 ++--- src/processor/stackwalker_ppc.cc | 4 +- src/processor/stackwalker_ppc.h | 4 +- src/processor/stackwalker_selftest.cc | 15 ++-- src/processor/stackwalker_x86.cc | 22 +++-- src/processor/stackwalker_x86.h | 4 +- 15 files changed, 192 insertions(+), 142 deletions(-) diff --git a/src/google/minidump_processor.h b/src/google/minidump_processor.h index 925ea90a..6a126022 100644 --- a/src/google/minidump_processor.h +++ b/src/google/minidump_processor.h @@ -46,9 +46,9 @@ class MinidumpProcessor { MinidumpProcessor(SymbolSupplier *supplier); ~MinidumpProcessor(); - // Fills in the given CallStack by processing the minidump file. Returns - // true on success. - bool Process(const string &minidump_file, CallStack *stack); + // Returns a new CallStack produced by processing the minidump file. The + // caller takes ownership of the CallStack. Returns NULL on failure. + CallStack* Process(const string &minidump_file); private: SymbolSupplier *supplier_; diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc index 2972fc5c..20609fb6 100644 --- a/src/processor/minidump_processor.cc +++ b/src/processor/minidump_processor.cc @@ -44,43 +44,41 @@ MinidumpProcessor::MinidumpProcessor(SymbolSupplier *supplier) MinidumpProcessor::~MinidumpProcessor() { } -bool MinidumpProcessor::Process(const string &minidump_file, - CallStack *stack) { +CallStack* MinidumpProcessor::Process(const string &minidump_file) { Minidump dump(minidump_file); if (!dump.Read()) { - return false; + return NULL; } MinidumpException *exception = dump.GetException(); if (!exception) { - return false; + return NULL; } MinidumpThreadList *threads = dump.GetThreadList(); if (!threads) { - return false; + return NULL; } // TODO(bryner): get all the threads MinidumpThread *thread = threads->GetThreadByID(exception->GetThreadID()); if (!thread) { - return false; + return NULL; } MinidumpMemoryRegion *thread_memory = thread->GetMemory(); if (!thread_memory) { - return false; + return NULL; } auto_ptr walker( Stackwalker::StackwalkerForCPU(exception->GetContext(), thread_memory, dump.GetModuleList(), supplier_)); if (!walker.get()) { - return false; + return NULL; } - walker->Walk(stack); - return true; + return walker->Walk(); } } // namespace google_airbag diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc index eefc64a2..ca02721e 100644 --- a/src/processor/minidump_processor_unittest.cc +++ b/src/processor/minidump_processor_unittest.cc @@ -30,6 +30,7 @@ // Unit test for MinidumpProcessor. Uses a pre-generated minidump and // corresponding symbol file, and checks the stack frames for correctness. +#include #include #include "google/call_stack.h" #include "google/minidump_processor.h" @@ -37,6 +38,7 @@ #include "google/symbol_supplier.h" #include "processor/minidump.h" +using std::auto_ptr; using std::string; using google_airbag::CallStack; using google_airbag::MinidumpProcessor; @@ -74,40 +76,40 @@ static bool RunTests() { TestSymbolSupplier supplier; MinidumpProcessor processor(&supplier); - CallStack stack; string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") + "/src/processor/testdata/minidump2.dmp"; - ASSERT_TRUE(processor.Process(minidump_file, &stack)); - ASSERT_EQ(stack.frames()->size(), 4); + auto_ptr stack(processor.Process(minidump_file)); + ASSERT_TRUE(stack.get()); + ASSERT_EQ(stack->frames()->size(), 4); - ASSERT_EQ(stack.frames()->at(0)->module_base, 0x400000); - ASSERT_EQ(stack.frames()->at(0)->module_name, "c:\\test_app.exe"); - ASSERT_EQ(stack.frames()->at(0)->function_name, "CrashFunction()"); - ASSERT_EQ(stack.frames()->at(0)->source_file_name, "c:\\test_app.cc"); - ASSERT_EQ(stack.frames()->at(0)->source_line, 65); + ASSERT_EQ(stack->frames()->at(0)->module_base, 0x400000); + ASSERT_EQ(stack->frames()->at(0)->module_name, "c:\\test_app.exe"); + ASSERT_EQ(stack->frames()->at(0)->function_name, "CrashFunction()"); + ASSERT_EQ(stack->frames()->at(0)->source_file_name, "c:\\test_app.cc"); + ASSERT_EQ(stack->frames()->at(0)->source_line, 65); - ASSERT_EQ(stack.frames()->at(1)->module_base, 0x400000); - ASSERT_EQ(stack.frames()->at(1)->module_name, "c:\\test_app.exe"); - ASSERT_EQ(stack.frames()->at(1)->function_name, "main"); - ASSERT_EQ(stack.frames()->at(1)->source_file_name, "c:\\test_app.cc"); - ASSERT_EQ(stack.frames()->at(1)->source_line, 70); + ASSERT_EQ(stack->frames()->at(1)->module_base, 0x400000); + ASSERT_EQ(stack->frames()->at(1)->module_name, "c:\\test_app.exe"); + ASSERT_EQ(stack->frames()->at(1)->function_name, "main"); + ASSERT_EQ(stack->frames()->at(1)->source_file_name, "c:\\test_app.cc"); + ASSERT_EQ(stack->frames()->at(1)->source_line, 70); // This comes from the CRT - ASSERT_EQ(stack.frames()->at(2)->module_base, 0x400000); - ASSERT_EQ(stack.frames()->at(2)->module_name, "c:\\test_app.exe"); - ASSERT_EQ(stack.frames()->at(2)->function_name, "__tmainCRTStartup"); - ASSERT_EQ(stack.frames()->at(2)->source_file_name, + ASSERT_EQ(stack->frames()->at(2)->module_base, 0x400000); + ASSERT_EQ(stack->frames()->at(2)->module_name, "c:\\test_app.exe"); + ASSERT_EQ(stack->frames()->at(2)->function_name, "__tmainCRTStartup"); + ASSERT_EQ(stack->frames()->at(2)->source_file_name, "f:\\rtm\\vctools\\crt_bld\\self_x86\\crt\\src\\crt0.c"); - ASSERT_EQ(stack.frames()->at(2)->source_line, 318); + ASSERT_EQ(stack->frames()->at(2)->source_line, 318); // No debug info available for kernel32.dll - ASSERT_EQ(stack.frames()->at(3)->module_base, 0x7c800000); - ASSERT_EQ(stack.frames()->at(3)->module_name, + ASSERT_EQ(stack->frames()->at(3)->module_base, 0x7c800000); + ASSERT_EQ(stack->frames()->at(3)->module_name, "C:\\WINDOWS\\system32\\kernel32.dll"); - ASSERT_TRUE(stack.frames()->at(3)->function_name.empty()); - ASSERT_TRUE(stack.frames()->at(3)->source_file_name.empty()); - ASSERT_EQ(stack.frames()->at(3)->source_line, 0); + ASSERT_TRUE(stack->frames()->at(3)->function_name.empty()); + ASSERT_TRUE(stack->frames()->at(3)->source_file_name.empty()); + ASSERT_EQ(stack->frames()->at(3)->source_line, 0); return true; } diff --git a/src/processor/minidump_stackwalk.cc b/src/processor/minidump_stackwalk.cc index a7d57f3c..cd2cbe3d 100644 --- a/src/processor/minidump_stackwalk.cc +++ b/src/processor/minidump_stackwalk.cc @@ -114,12 +114,11 @@ int main(int argc, char **argv) { exit(1); } - CallStack stack; - stackwalker->Walk(&stack); + auto_ptr stack(stackwalker->Walk()); unsigned int index; - for (index = 0; index < stack.frames()->size(); ++index) { - StackFrame *frame = stack.frames()->at(index); + for (index = 0; index < stack->frames()->size(); ++index) { + StackFrame *frame = stack->frames()->at(index); printf("[%2d] instruction = 0x%08llx \"%s\" + 0x%08llx\n", index, frame->instruction, diff --git a/src/processor/source_line_resolver.cc b/src/processor/source_line_resolver.cc index e2e884ce..8a6fad16 100644 --- a/src/processor/source_line_resolver.cc +++ b/src/processor/source_line_resolver.cc @@ -28,18 +28,20 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include -#include #include +#include +#include #include #include #include "processor/source_line_resolver.h" #include "google/stack_frame.h" +#include "processor/linked_ptr.h" #include "processor/address_map-inl.h" #include "processor/contained_range_map-inl.h" -#include "processor/linked_ptr.h" #include "processor/range_map-inl.h" #include "processor/stack_frame_info.h" +using std::auto_ptr; using std::map; using std::vector; using std::make_pair; @@ -104,9 +106,10 @@ class SourceLineResolver::Module { // Looks up the given relative address, and fills the StackFrame struct // with the result. Additional debugging information, if available, is - // placed in frame_info. - void LookupAddress(MemAddr address, - StackFrame *frame, StackFrameInfo *frame_info) const; + // returned. If no additional information is available, returns NULL. + // A NULL return value is not an error. The caller takes ownership of + // any returned StackFrameInfo object. + StackFrameInfo* LookupAddress(MemAddr address, StackFrame *frame) const; private: friend class SourceLineResolver; @@ -163,7 +166,8 @@ class SourceLineResolver::Module { // StackInfoTypes. These are split by type because there may be overlaps // between maps of different types, but some information is only available // as certain types. - ContainedRangeMap stack_info_[STACK_INFO_LAST]; + ContainedRangeMap< MemAddr, linked_ptr > + stack_info_[STACK_INFO_LAST]; }; SourceLineResolver::SourceLineResolver() : modules_(new ModuleMap) { @@ -198,13 +202,14 @@ bool SourceLineResolver::HasModule(const string &module_name) const { return modules_->find(module_name) != modules_->end(); } -void SourceLineResolver::FillSourceLineInfo(StackFrame *frame, - StackFrameInfo *frame_info) const { +StackFrameInfo* SourceLineResolver::FillSourceLineInfo( + StackFrame *frame) const { ModuleMap::const_iterator it = modules_->find(frame->module_name); if (it != modules_->end()) { - it->second->LookupAddress(frame->instruction - frame->module_base, - frame, frame_info); + return it->second->LookupAddress(frame->instruction - frame->module_base, + frame); } + return NULL; } bool SourceLineResolver::Module::LoadMap(const string &map_file) { @@ -259,23 +264,24 @@ bool SourceLineResolver::Module::LoadMap(const string &map_file) { return true; } -void SourceLineResolver::Module::LookupAddress( - MemAddr address, StackFrame *frame, StackFrameInfo *frame_info) const { - if (frame_info) { - frame_info->valid = StackFrameInfo::VALID_NONE; +StackFrameInfo* SourceLineResolver::Module::LookupAddress( + MemAddr address, StackFrame *frame) const { + linked_ptr retrieved_info; + // Check for debugging info first, before any possible early returns. + // + // We only know about STACK_INFO_FRAME_DATA and STACK_INFO_FPO. Prefer + // them in this order. STACK_INFO_FRAME_DATA is the newer type that + // includes its own program string. STACK_INFO_FPO is the older type + // corresponding to the FPO_DATA struct. See stackwalker_x86.cc. + if (!stack_info_[STACK_INFO_FRAME_DATA].RetrieveRange(address, + &retrieved_info)) { + stack_info_[STACK_INFO_FPO].RetrieveRange(address, &retrieved_info); + } - // Check for debugging info first, before any possible early returns. - // The caller will know that frame_info was filled in by checking its - // valid field. - // - // We only know about STACK_INFO_FRAME_DATA and STACK_INFO_FPO. Prefer - // them in this order. STACK_INFO_FRAME_DATA is the newer type that - // includes its own program string. STACK_INFO_FPO is the older type - // corresponding to the FPO_DATA struct. See stackwalker_x86.cc. - if (!stack_info_[STACK_INFO_FRAME_DATA].RetrieveRange(address, - frame_info)) { - stack_info_[STACK_INFO_FPO].RetrieveRange(address, frame_info); - } + auto_ptr frame_info; + if (retrieved_info.get()) { + frame_info.reset(new StackFrameInfo()); + frame_info->CopyFrom(*retrieved_info.get()); } // First, look for a matching FUNC range. Use RetrieveNearestRange instead @@ -324,18 +330,20 @@ void SourceLineResolver::Module::LookupAddress( frame->function_base = frame->module_base + public_address; } else { // No FUNC or PUBLIC data available. - return; + return frame_info.release(); } - if (frame_info && - !(frame_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE)) { + if (!frame_info.get()) { // Even without a relevant STACK line, many functions contain information // about how much space their parameters consume on the stack. Prefer // the STACK stuff (above), but if it's not present, take the // information from the FUNC or PUBLIC line. + frame_info.reset(new StackFrameInfo()); frame_info->parameter_size = parameter_size; frame_info->valid |= StackFrameInfo::VALID_PARAMETER_SIZE; } + + return frame_info.release(); } // static @@ -518,15 +526,16 @@ bool SourceLineResolver::Module::ParseStackInfo(char *stack_info_line) { // if ContainedRangeMap were modified to allow replacement of // already-stored values. - stack_info_[type].StoreRange(rva, code_size, - StackFrameInfo(prolog_size, - epilog_size, - parameter_size, - saved_register_size, - local_size, - max_stack_size, - allocates_base_pointer, - program_string)); + linked_ptr stack_frame_info( + new StackFrameInfo(prolog_size, + epilog_size, + parameter_size, + saved_register_size, + local_size, + max_stack_size, + allocates_base_pointer, + program_string)); + stack_info_[type].StoreRange(rva, code_size, stack_frame_info); return true; } diff --git a/src/processor/source_line_resolver.h b/src/processor/source_line_resolver.h index f0d1e54b..cf93c69e 100644 --- a/src/processor/source_line_resolver.h +++ b/src/processor/source_line_resolver.h @@ -66,8 +66,11 @@ class SourceLineResolver { // Fills in the function_base, function_name, source_file_name, // and source_line fields of the StackFrame. The instruction and // module_name fields must already be filled in. Additional debugging - // information, if available, is placed in frame_info. - void FillSourceLineInfo(StackFrame *frame, StackFrameInfo *frame_info) const; + // information, if available, is returned. If the information is not + // available, returns NULL. A NULL return value does not indicate an + // error. The caller takes ownership of any returned StackFrameInfo + // object. + StackFrameInfo* FillSourceLineInfo(StackFrame *frame) const; private: template class MemAddrMap; diff --git a/src/processor/source_line_resolver_unittest.cc b/src/processor/source_line_resolver_unittest.cc index 6cc9afd3..e9ee9d91 100644 --- a/src/processor/source_line_resolver_unittest.cc +++ b/src/processor/source_line_resolver_unittest.cc @@ -28,12 +28,16 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #include #include "processor/source_line_resolver.h" #include "google/stack_frame.h" +#include "processor/linked_ptr.h" #include "processor/stack_frame_info.h" +using std::auto_ptr; using std::string; +using google_airbag::linked_ptr; using google_airbag::SourceLineResolver; using google_airbag::StackFrame; using google_airbag::StackFrameInfo; @@ -55,12 +59,10 @@ static bool VerifyEmpty(const StackFrame &frame) { return true; } -static void ClearSourceLineInfo(StackFrame *frame, - StackFrameInfo *frame_info) { +static void ClearSourceLineInfo(StackFrame *frame) { frame->function_name.clear(); frame->source_file_name.clear(); frame->source_line = 0; - frame_info->program_string.clear(); } static bool RunTests() { @@ -74,62 +76,64 @@ static bool RunTests() { ASSERT_TRUE(resolver.HasModule("module2")); StackFrame frame; - StackFrameInfo frame_info; frame.instruction = 0x1000; frame.module_name = "module1"; - resolver.FillSourceLineInfo(&frame, &frame_info); + auto_ptr frame_info(resolver.FillSourceLineInfo(&frame)); ASSERT_EQ(frame.function_name, "Function1_1"); ASSERT_EQ(frame.source_file_name, "file1_1.cc"); ASSERT_EQ(frame.source_line, 44); - ASSERT_FALSE(frame_info.allocates_base_pointer); - ASSERT_EQ(frame_info.program_string, + ASSERT_TRUE(frame_info.get()); + ASSERT_FALSE(frame_info->allocates_base_pointer); + ASSERT_EQ(frame_info->program_string, "$eip 4 + ^ = $esp $ebp 8 + = $ebp $ebp ^ ="); - ClearSourceLineInfo(&frame, &frame_info); + ClearSourceLineInfo(&frame); frame.instruction = 0x800; - resolver.FillSourceLineInfo(&frame, &frame_info); + frame_info.reset(resolver.FillSourceLineInfo(&frame)); ASSERT_TRUE(VerifyEmpty(frame)); - ASSERT_FALSE(frame_info.allocates_base_pointer); - ASSERT_TRUE(frame_info.program_string.empty()); + ASSERT_FALSE(frame_info.get()); frame.instruction = 0x1280; - resolver.FillSourceLineInfo(&frame, &frame_info); + frame_info.reset(resolver.FillSourceLineInfo(&frame)); ASSERT_EQ(frame.function_name, "Function1_3"); ASSERT_TRUE(frame.source_file_name.empty()); ASSERT_EQ(frame.source_line, 0); - ASSERT_FALSE(frame_info.allocates_base_pointer); - ASSERT_TRUE(frame_info.program_string.empty()); + ASSERT_TRUE(frame_info.get()); + ASSERT_FALSE(frame_info->allocates_base_pointer); + ASSERT_TRUE(frame_info->program_string.empty()); frame.instruction = 0x1380; - resolver.FillSourceLineInfo(&frame, &frame_info); + frame_info.reset(resolver.FillSourceLineInfo(&frame)); ASSERT_EQ(frame.function_name, "Function1_4"); ASSERT_TRUE(frame.source_file_name.empty()); ASSERT_EQ(frame.source_line, 0); - ASSERT_FALSE(frame_info.allocates_base_pointer); - ASSERT_FALSE(frame_info.program_string.empty()); + ASSERT_TRUE(frame_info.get()); + ASSERT_FALSE(frame_info->allocates_base_pointer); + ASSERT_FALSE(frame_info->program_string.empty()); frame.instruction = 0x2180; frame.module_name = "module2"; - resolver.FillSourceLineInfo(&frame, &frame_info); + frame_info.reset(resolver.FillSourceLineInfo(&frame)); ASSERT_EQ(frame.function_name, "Function2_2"); ASSERT_EQ(frame.source_file_name, "file2_2.cc"); ASSERT_EQ(frame.source_line, 21); - ASSERT_EQ(frame_info.prolog_size, 1); + ASSERT_TRUE(frame_info.get()); + ASSERT_EQ(frame_info->prolog_size, 1); frame.instruction = 0x216f; frame.module_name = "module2"; - resolver.FillSourceLineInfo(&frame, &frame_info); + resolver.FillSourceLineInfo(&frame); ASSERT_EQ(frame.function_name, "Public2_1"); - ClearSourceLineInfo(&frame, &frame_info); + ClearSourceLineInfo(&frame); frame.instruction = 0x219f; frame.module_name = "module2"; - resolver.FillSourceLineInfo(&frame, &frame_info); + resolver.FillSourceLineInfo(&frame); ASSERT_TRUE(frame.function_name.empty()); frame.instruction = 0x21a0; frame.module_name = "module2"; - resolver.FillSourceLineInfo(&frame, &frame_info); + resolver.FillSourceLineInfo(&frame); ASSERT_EQ(frame.function_name, "Public2_2"); ASSERT_FALSE(resolver.LoadModule("module3", diff --git a/src/processor/stack_frame_info.h b/src/processor/stack_frame_info.h index 4b5e5534..e6026c11 100644 --- a/src/processor/stack_frame_info.h +++ b/src/processor/stack_frame_info.h @@ -80,6 +80,19 @@ struct StackFrameInfo { allocates_base_pointer(set_allocates_base_pointer), program_string(set_program_string) {} + // CopyFrom makes "this" StackFrameInfo object identical to "that". + void CopyFrom(const StackFrameInfo &that) { + valid = that.valid; + prolog_size = that.prolog_size; + epilog_size = that.epilog_size; + parameter_size = that.parameter_size; + saved_register_size = that.saved_register_size; + local_size = that.local_size; + max_stack_size = that.max_stack_size; + allocates_base_pointer = that.allocates_base_pointer; + program_string = that.program_string; + } + // Clears the StackFrameInfo object so that users will see it as though // it contains no information. void Clear() { valid = VALID_NONE; program_string.erase(); } diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index 13bb2040..ffe258e7 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc @@ -40,8 +40,10 @@ #include "google/call_stack.h" #include "google/stack_frame.h" #include "google/symbol_supplier.h" +#include "processor/linked_ptr.h" #include "processor/minidump.h" #include "processor/source_line_resolver.h" +#include "processor/stack_frame_info.h" #include "processor/stackwalker_ppc.h" #include "processor/stackwalker_x86.h" @@ -52,15 +54,20 @@ using std::auto_ptr; Stackwalker::Stackwalker(MemoryRegion *memory, MinidumpModuleList *modules, SymbolSupplier *supplier) - : memory_(memory), stack_frame_info_(), modules_(modules), - supplier_(supplier) { + : memory_(memory), modules_(modules), supplier_(supplier) { } -void Stackwalker::Walk(CallStack *stack) { - stack_frame_info_.clear(); +CallStack* Stackwalker::Walk() { SourceLineResolver resolver; + auto_ptr stack(new CallStack()); + + // 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. @@ -72,7 +79,7 @@ void Stackwalker::Walk(CallStack *stack) { // frame_pointer fields. The frame structure comes from either the // context frame (above) or a caller frame (below). - StackFrameInfo frame_info; + linked_ptr frame_info; // Resolve the module information, if a module map was provided. if (modules_) { @@ -87,7 +94,7 @@ void Stackwalker::Walk(CallStack *stack) { resolver.LoadModule(frame->module_name, symbol_file); } } - resolver.FillSourceLineInfo(frame.get(), &frame_info); + frame_info.reset(resolver.FillSourceLineInfo(frame.get())); } } @@ -95,13 +102,15 @@ void Stackwalker::Walk(CallStack *stack) { // over the frame, because the stack now owns it. stack->frames_.push_back(frame.release()); - // Copy the frame info. - stack_frame_info_.push_back(frame_info); - frame_info.Clear(); + // 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)); + frame.reset(GetCallerFrame(stack.get(), stack_frame_info)); } + + return stack.release(); } diff --git a/src/processor/stackwalker.h b/src/processor/stackwalker.h index d4eb0745..82421bb5 100644 --- a/src/processor/stackwalker.h +++ b/src/processor/stackwalker.h @@ -43,25 +43,28 @@ #include -#include "processor/stack_frame_info.h" - namespace google_airbag { class CallStack; +template class linked_ptr; class MemoryRegion; class MinidumpContext; class MinidumpModuleList; struct StackFrame; +struct StackFrameInfo; class SymbolSupplier; +using std::vector; + class Stackwalker { public: virtual ~Stackwalker() {} - // Fills the given CallStack by calling GetContextFrame and GetCallerFrame, - // and populating the returned frames with all available data. - void Walk(CallStack* stack); + // Creates a new CallStack and populates it by calling GetContextFrame and + // GetCallerFrame. The frames are further processed to fill all available + // data. The caller takes ownership of the CallStack returned by Walk. + CallStack* Walk(); // Returns a new concrete subclass suitable for the CPU that a stack was // generated on, according to the CPU type indicated by the context @@ -86,11 +89,6 @@ class Stackwalker { // get information from the stack. MemoryRegion *memory_; - // Additional debugging information for each stack frame. This vector - // parallels the CallStack. Subclasses may use this information to help - // walk the stack. - std::vector stack_frame_info_; - private: // Obtains the context frame, the innermost called procedure in a stack // trace. Returns NULL on failure. GetContextFrame allocates a new @@ -106,7 +104,9 @@ 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; + virtual StackFrame* GetCallerFrame( + const CallStack *stack, + const vector< linked_ptr > &stack_frame_info) = 0; // A list of modules, for populating each StackFrame's module information. // This field is optional and may be NULL. diff --git a/src/processor/stackwalker_ppc.cc b/src/processor/stackwalker_ppc.cc index 2980c51a..b3898ccd 100644 --- a/src/processor/stackwalker_ppc.cc +++ b/src/processor/stackwalker_ppc.cc @@ -73,7 +73,9 @@ StackFrame* StackwalkerPPC::GetContextFrame() { } -StackFrame* StackwalkerPPC::GetCallerFrame(const CallStack *stack) { +StackFrame* StackwalkerPPC::GetCallerFrame( + const CallStack *stack, + const vector< linked_ptr > &stack_frame_info) { if (!memory_ || !stack) return NULL; diff --git a/src/processor/stackwalker_ppc.h b/src/processor/stackwalker_ppc.h index 7da76157..15e2e1ce 100644 --- a/src/processor/stackwalker_ppc.h +++ b/src/processor/stackwalker_ppc.h @@ -65,7 +65,9 @@ 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, + const vector< linked_ptr > &stack_frame_info); // Stores the CPU context corresponding to the innermost stack frame to // be returned by GetContextFrame. diff --git a/src/processor/stackwalker_selftest.cc b/src/processor/stackwalker_selftest.cc index 4e8d2fd7..5c8f93bc 100644 --- a/src/processor/stackwalker_selftest.cc +++ b/src/processor/stackwalker_selftest.cc @@ -38,6 +38,7 @@ #include +#include #include "google/airbag_types.h" #include "google/call_stack.h" @@ -46,6 +47,7 @@ #include "processor/memory_region.h" #include "processor/minidump_format.h" +using std::auto_ptr; using google_airbag::CallStack; using google_airbag::MemoryRegion; using google_airbag::StackFrame; @@ -216,23 +218,22 @@ static unsigned int CountCallerFrames() { StackwalkerPPC stackwalker = StackwalkerPPC(&context, &memory, NULL, NULL); #endif // __i386__ || __ppc__ - CallStack stack; - stackwalker.Walk(&stack); + auto_ptr stack(stackwalker.Walk()); #ifdef PRINT_STACKS printf("\n"); for(unsigned int frame_index = 0; - frame_index < stack.Count(); + frame_index < stack->frames()->size(); ++frame_index) { - StackFrame *frame = stack.FrameAt(frame_index); + StackFrame *frame = stack->frames()->at(frame_index); printf("frame %-3d instruction = 0x%08llx", frame_index, frame->instruction); #if defined(__i386__) - StackFrameX86 *frame_x86 = reinterpret_cast(frame.get()); + StackFrameX86 *frame_x86 = reinterpret_cast(frame); printf(" esp = 0x%08x ebp = 0x%08x\n", frame_x86->context.esp, frame_x86->context.ebp); #elif defined(__ppc__) - StackFramePPC *frame_ppc = reinterpret_cast(frame.get()); + StackFramePPC *frame_ppc = reinterpret_cast(frame); printf(" gpr[1] = 0x%08x\n", frame_ppc->context.gpr[1]); #endif // __i386__ || __ppc__ } @@ -241,7 +242,7 @@ static unsigned int CountCallerFrames() { // Subtract 1 because the caller wants the number of frames beneath // itself. Because the caller called us, subract two for our frame and its // frame, which are included in stack->size(). - return stack.frames()->size() - 2; + return stack->frames()->size() - 2; } diff --git a/src/processor/stackwalker_x86.cc b/src/processor/stackwalker_x86.cc index b865ac7d..04a7e8c4 100644 --- a/src/processor/stackwalker_x86.cc +++ b/src/processor/stackwalker_x86.cc @@ -37,8 +37,10 @@ #include "processor/stackwalker_x86.h" #include "google/call_stack.h" #include "google/stack_frame_cpu.h" +#include "processor/linked_ptr.h" #include "processor/minidump.h" #include "processor/postfix_evaluator-inl.h" +#include "processor/stack_frame_info.h" namespace google_airbag { @@ -73,13 +75,15 @@ StackFrame* StackwalkerX86::GetContextFrame() { } -StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) { +StackFrame* StackwalkerX86::GetCallerFrame( + const CallStack *stack, + const vector< linked_ptr > &stack_frame_info) { if (!memory_ || !stack) return NULL; StackFrameX86 *last_frame = static_cast( stack->frames()->back()); - StackFrameInfo *last_frame_info = &stack_frame_info_.back(); + StackFrameInfo *last_frame_info = stack_frame_info.back().get(); // 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 @@ -113,12 +117,13 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) { // 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_frame_info.size(); u_int32_t last_frame_callee_parameter_size = 0; if (frames_already_walked >= 2) { StackFrameInfo *last_frame_callee_info = - &stack_frame_info_[frames_already_walked - 2]; - if (last_frame_callee_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE) { + stack_frame_info[frames_already_walked - 2].get(); + if (last_frame_callee_info && + last_frame_callee_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE) { last_frame_callee_parameter_size = last_frame_callee_info->parameter_size; } @@ -135,7 +140,7 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) { dictionary["$esp"] = last_frame->context.esp; dictionary[".cbCalleeParams"] = last_frame_callee_parameter_size; - if (last_frame_info->valid == StackFrameInfo::VALID_ALL) { + if (last_frame_info && last_frame_info->valid == StackFrameInfo::VALID_ALL) { // FPO debugging data is available. Initialize constants. dictionary[".cbSavedRegs"] = last_frame_info->saved_register_size; dictionary[".cbLocals"] = last_frame_info->local_size; @@ -144,7 +149,8 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) { last_frame_info->local_size + last_frame_info->saved_register_size; } - if (last_frame_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE) { + if (last_frame_info && + last_frame_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE) { // This is treated separately because it can either come from FPO data or // from other debugging data. dictionary[".cbParams"] = last_frame_info->parameter_size; @@ -156,7 +162,7 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) { // the return address and the values of other registers in the calling // function. string program_string; - if (last_frame_info->valid == StackFrameInfo::VALID_ALL) { + if (last_frame_info && last_frame_info->valid == StackFrameInfo::VALID_ALL) { // FPO data available. if (!last_frame_info->program_string.empty()) { // The FPO data has its own program string, which will tell us how to diff --git a/src/processor/stackwalker_x86.h b/src/processor/stackwalker_x86.h index 9f133417..80e5a30b 100644 --- a/src/processor/stackwalker_x86.h +++ b/src/processor/stackwalker_x86.h @@ -65,7 +65,9 @@ class StackwalkerX86 : public Stackwalker { // stack conventions (saved %ebp at [%ebp], saved %eip at 4[%ebp], or // alternate conventions as guided by stack_frame_info_). virtual StackFrame* GetContextFrame(); - virtual StackFrame* GetCallerFrame(const CallStack *stack); + virtual StackFrame* GetCallerFrame( + const CallStack *stack, + const vector< linked_ptr > &stack_frame_info); // Stores the CPU context corresponding to the innermost stack frame to // be returned by GetContextFrame.