diff --git a/src/google_airbag/processor/call_stack.h b/src/google_airbag/processor/call_stack.h index 580d4927..d938944e 100644 --- a/src/google_airbag/processor/call_stack.h +++ b/src/google_airbag/processor/call_stack.h @@ -56,17 +56,18 @@ template class linked_ptr; class CallStack { public: + CallStack() { Clear(); } ~CallStack(); + // Resets the CallStack to its initial empty state + void Clear(); + const vector* frames() const { return &frames_; } private: // Stackwalker is responsible for building the frames_ vector. friend class Stackwalker; - // Disallow instantiation other than by friends. - CallStack() : frames_() {} - // Storage for pushed frames. vector frames_; }; diff --git a/src/google_airbag/processor/minidump_processor.h b/src/google_airbag/processor/minidump_processor.h index 06c185ff..604eae45 100644 --- a/src/google_airbag/processor/minidump_processor.h +++ b/src/google_airbag/processor/minidump_processor.h @@ -42,15 +42,21 @@ class SymbolSupplier; class MinidumpProcessor { public: + // Return type for Process() + enum ProcessResult { + PROCESS_OK, // the minidump was processed successfully + PROCESS_ERROR, // there was an error processing the minidump + PROCESS_INTERRUPTED, // processing was interrupted by the SymbolSupplier + }; + // Initializes this MinidumpProcessor. supplier should be an // implementation of the SymbolSupplier abstract base class. explicit MinidumpProcessor(SymbolSupplier *supplier); ~MinidumpProcessor(); - // Returns a new ProcessState object produced by processing the minidump - // file. The caller takes ownership of the ProcessState. Returns NULL on - // failure. - ProcessState* Process(const string &minidump_file); + // Processes the minidump file and fills process_state with the result. + ProcessResult Process(const string &minidump_file, + ProcessState *process_state); // Returns a textual representation of the base CPU type that the minidump // in dump was produced on. Returns an empty string if this information diff --git a/src/google_airbag/processor/process_state.h b/src/google_airbag/processor/process_state.h index 69005e2b..6fe12155 100644 --- a/src/google_airbag/processor/process_state.h +++ b/src/google_airbag/processor/process_state.h @@ -47,8 +47,12 @@ class CodeModules; class ProcessState { public: + ProcessState() : modules_(NULL) { Clear(); } ~ProcessState(); + // Resets the ProcessState to its default values + void Clear(); + // Accessors. See the data declarations below. u_int32_t time_date_stamp() const { return time_date_stamp_; } bool crashed() const { return crashed_; } @@ -66,11 +70,6 @@ class ProcessState { // MinidumpProcessor is responsible for building ProcessState objects. friend class MinidumpProcessor; - // Disallow instantiation other than by friends. - ProcessState() : time_date_stamp_(0), crashed_(false), crash_reason_(), - crash_address_(0), requesting_thread_(-1), threads_(), - os_(), os_version_(), cpu_(), cpu_info_(), modules_(NULL) {} - // The time-date stamp of the minidump (time_t format) u_int32_t time_date_stamp_; diff --git a/src/google_airbag/processor/stackwalker.h b/src/google_airbag/processor/stackwalker.h index b3f2333e..70da125e 100644 --- a/src/google_airbag/processor/stackwalker.h +++ b/src/google_airbag/processor/stackwalker.h @@ -61,10 +61,11 @@ class Stackwalker { public: virtual ~Stackwalker() {} - // Creates a new CallStack and populates it by calling GetContextFrame and + // Populates the given CallStack 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(); + // data. Returns true if the stackwalk completed, or false if it was + // interrupted by SymbolSupplier::GetSymbolFile(). + bool Walk(CallStack *stack); // Returns a new concrete subclass suitable for the CPU that a stack was // generated on, according to the CPU type indicated by the context diff --git a/src/google_airbag/processor/symbol_supplier.h b/src/google_airbag/processor/symbol_supplier.h index 47a2be08..d456174b 100644 --- a/src/google_airbag/processor/symbol_supplier.h +++ b/src/google_airbag/processor/symbol_supplier.h @@ -42,10 +42,24 @@ class CodeModule; class SymbolSupplier { public: + // Result type for GetSymbolFile + enum SymbolResult { + // no symbols were found, but continue processing + NOT_FOUND, + + // symbols were found, and the path has been placed in symbol_file + FOUND, + + // stops processing the minidump immediately + INTERRUPT, + }; + virtual ~SymbolSupplier() {} - // Returns the path to the symbol file for the given module. - virtual string GetSymbolFile(const CodeModule *module) = 0; + // Retrieves the symbol file for the given CodeModule, placing the + // path in symbol_file if successful. + virtual SymbolResult GetSymbolFile(const CodeModule *module, + string *symbol_file) = 0; }; } // namespace google_airbag diff --git a/src/processor/call_stack.cc b/src/processor/call_stack.cc index 8c1362f3..ee67aace 100644 --- a/src/processor/call_stack.cc +++ b/src/processor/call_stack.cc @@ -39,6 +39,10 @@ namespace google_airbag { CallStack::~CallStack() { + Clear(); +} + +void CallStack::Clear() { for (vector::const_iterator iterator = frames_.begin(); iterator != frames_.end(); ++iterator) { diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc index 99aa04cc..ab027e8a 100644 --- a/src/processor/minidump_processor.cc +++ b/src/processor/minidump_processor.cc @@ -45,13 +45,14 @@ MinidumpProcessor::MinidumpProcessor(SymbolSupplier *supplier) MinidumpProcessor::~MinidumpProcessor() { } -ProcessState* MinidumpProcessor::Process(const string &minidump_file) { +MinidumpProcessor::ProcessResult MinidumpProcessor::Process( + const string &minidump_file, ProcessState *process_state) { Minidump dump(minidump_file); if (!dump.Read()) { - return NULL; + return PROCESS_ERROR; } - scoped_ptr process_state(new ProcessState()); + process_state->Clear(); const MDRawHeader *header = dump.header(); assert(header); @@ -91,7 +92,7 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) { MinidumpThreadList *threads = dump.GetThreadList(); if (!threads) { - return NULL; + return PROCESS_ERROR; } bool found_requesting_thread = false; @@ -101,12 +102,12 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) { ++thread_index) { MinidumpThread *thread = threads->GetThreadAtIndex(thread_index); if (!thread) { - return NULL; + return PROCESS_ERROR; } u_int32_t thread_id; if (!thread->GetThreadID(&thread_id)) { - return NULL; + return PROCESS_ERROR; } // If this thread is the thread that produced the minidump, don't process @@ -122,7 +123,7 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) { if (has_requesting_thread && thread_id == requesting_thread_id) { if (found_requesting_thread) { // There can't be more than one requesting thread. - return NULL; + return PROCESS_ERROR; } // Use processed_state->threads_.size() instead of thread_index. @@ -148,7 +149,7 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) { MinidumpMemoryRegion *thread_memory = thread->GetMemory(); if (!thread_memory) { - return NULL; + return PROCESS_ERROR; } // Use process_state->modules_ instead of module_list, because the @@ -165,23 +166,22 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) { process_state->modules_, supplier_)); if (!stackwalker.get()) { - return NULL; + return PROCESS_ERROR; } - scoped_ptr stack(stackwalker->Walk()); - if (!stack.get()) { - return NULL; + scoped_ptr stack(new CallStack()); + if (!stackwalker->Walk(stack.get())) { + return PROCESS_INTERRUPTED; } - process_state->threads_.push_back(stack.release()); } // If a requesting thread was indicated, it must be present. if (has_requesting_thread && !found_requesting_thread) { - return NULL; + return PROCESS_ERROR; } - return process_state.release(); + return PROCESS_OK; } // Returns the MDRawSystemInfo from a minidump, or NULL if system info is diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc index c8a0777f..766c2a30 100644 --- a/src/processor/minidump_processor_unittest.cc +++ b/src/processor/minidump_processor_unittest.cc @@ -62,18 +62,33 @@ using google_airbag::SymbolSupplier; class TestSymbolSupplier : public SymbolSupplier { public: - virtual string GetSymbolFile(const CodeModule *module); + TestSymbolSupplier() : interrupt_(false) {} + + virtual SymbolResult GetSymbolFile(const CodeModule *module, + string *symbol_file); + + // When set to true, causes the SymbolSupplier to return INTERRUPT + void set_interrupt(bool interrupt) { interrupt_ = interrupt; } + + private: + bool interrupt_; }; -string TestSymbolSupplier::GetSymbolFile(const CodeModule *module) { - if (module && module->code_file() == "C:\\test_app.exe") { - return string(getenv("srcdir") ? getenv("srcdir") : ".") + - "/src/processor/testdata/symbols/test_app.pdb/" + - module->debug_identifier() + - "/test_app.sym"; +SymbolSupplier::SymbolResult TestSymbolSupplier::GetSymbolFile( + const CodeModule *module, string *symbol_file) { + if (interrupt_) { + return INTERRUPT; } - return ""; + if (module && module->code_file() == "C:\\test_app.exe") { + *symbol_file = string(getenv("srcdir") ? getenv("srcdir") : ".") + + "/src/processor/testdata/symbols/test_app.pdb/" + + module->debug_identifier() + + "/test_app.sym"; + return FOUND; + } + + return NOT_FOUND; } static bool RunTests() { @@ -83,19 +98,20 @@ static bool RunTests() { string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") + "/src/processor/testdata/minidump2.dmp"; - scoped_ptr state(processor.Process(minidump_file)); - ASSERT_TRUE(state.get()); - ASSERT_EQ(state->cpu(), "x86"); - ASSERT_EQ(state->cpu_info(), "GenuineIntel family 6 model 13 stepping 8"); - ASSERT_EQ(state->os(), "Windows NT"); - ASSERT_EQ(state->os_version(), "5.1.2600 Service Pack 2"); - ASSERT_TRUE(state->crashed()); - ASSERT_EQ(state->crash_reason(), "EXCEPTION_ACCESS_VIOLATION"); - ASSERT_EQ(state->crash_address(), 0x45); - ASSERT_EQ(state->threads()->size(), 1); - ASSERT_EQ(state->requesting_thread(), 0); + ProcessState state; + ASSERT_EQ(processor.Process(minidump_file, &state), + MinidumpProcessor::PROCESS_OK); + ASSERT_EQ(state.cpu(), "x86"); + ASSERT_EQ(state.cpu_info(), "GenuineIntel family 6 model 13 stepping 8"); + ASSERT_EQ(state.os(), "Windows NT"); + ASSERT_EQ(state.os_version(), "5.1.2600 Service Pack 2"); + ASSERT_TRUE(state.crashed()); + ASSERT_EQ(state.crash_reason(), "EXCEPTION_ACCESS_VIOLATION"); + ASSERT_EQ(state.crash_address(), 0x45); + ASSERT_EQ(state.threads()->size(), 1); + ASSERT_EQ(state.requesting_thread(), 0); - CallStack *stack = state->threads()->at(0); + CallStack *stack = state.threads()->at(0); ASSERT_TRUE(stack); ASSERT_EQ(stack->frames()->size(), 4); @@ -131,17 +147,23 @@ static bool RunTests() { ASSERT_TRUE(stack->frames()->at(3)->source_file_name.empty()); ASSERT_EQ(stack->frames()->at(3)->source_line, 0); - ASSERT_EQ(state->modules()->module_count(), 13); - ASSERT_TRUE(state->modules()->GetMainModule()); - ASSERT_EQ(state->modules()->GetMainModule()->code_file(), "C:\\test_app.exe"); - ASSERT_FALSE(state->modules()->GetModuleForAddress(0)); - ASSERT_EQ(state->modules()->GetMainModule(), - state->modules()->GetModuleForAddress(0x400000)); - ASSERT_EQ(state->modules()->GetModuleForAddress(0x7c801234)->debug_file(), + ASSERT_EQ(state.modules()->module_count(), 13); + ASSERT_TRUE(state.modules()->GetMainModule()); + ASSERT_EQ(state.modules()->GetMainModule()->code_file(), "C:\\test_app.exe"); + ASSERT_FALSE(state.modules()->GetModuleForAddress(0)); + ASSERT_EQ(state.modules()->GetMainModule(), + state.modules()->GetModuleForAddress(0x400000)); + ASSERT_EQ(state.modules()->GetModuleForAddress(0x7c801234)->debug_file(), "kernel32.pdb"); - ASSERT_EQ(state->modules()->GetModuleForAddress(0x77d43210)->version(), + ASSERT_EQ(state.modules()->GetModuleForAddress(0x77d43210)->version(), "5.1.2600.2622"); + // Test that the symbol supplier can interrupt processing + state.Clear(); + supplier.set_interrupt(true); + ASSERT_EQ(processor.Process(minidump_file, &state), + MinidumpProcessor::PROCESS_INTERRUPTED); + return true; } diff --git a/src/processor/minidump_stackwalk.cc b/src/processor/minidump_stackwalk.cc index 62bfd555..3c55933d 100644 --- a/src/processor/minidump_stackwalk.cc +++ b/src/processor/minidump_stackwalk.cc @@ -195,18 +195,18 @@ static bool PrintMinidumpProcess(const string &minidump_file, MinidumpProcessor minidump_processor(symbol_supplier.get()); // Process the minidump. - scoped_ptr process_state( - minidump_processor.Process(minidump_file)); - if (!process_state.get()) { + ProcessState process_state; + if (minidump_processor.Process(minidump_file, &process_state) != + MinidumpProcessor::PROCESS_OK) { fprintf(stderr, "MinidumpProcessor::Process failed\n"); return false; } // Print OS and CPU information. - string cpu = process_state->cpu(); - string cpu_info = process_state->cpu_info(); - printf("Operating system: %s\n", process_state->os().c_str()); - printf(" %s\n", process_state->os_version().c_str()); + string cpu = process_state.cpu(); + string cpu_info = process_state.cpu_info(); + printf("Operating system: %s\n", process_state.os().c_str()); + printf(" %s\n", process_state.os_version().c_str()); printf("CPU: %s\n", cpu.c_str()); if (!cpu_info.empty()) { // This field is optional. @@ -215,36 +215,36 @@ static bool PrintMinidumpProcess(const string &minidump_file, printf("\n"); // Print crash information. - if (process_state->crashed()) { - printf("Crash reason: %s\n", process_state->crash_reason().c_str()); - printf("Crash address: 0x%llx\n", process_state->crash_address()); + if (process_state.crashed()) { + printf("Crash reason: %s\n", process_state.crash_reason().c_str()); + printf("Crash address: 0x%llx\n", process_state.crash_address()); } else { printf("No crash\n"); } // If the thread that requested the dump is known, print it first. - int requesting_thread = process_state->requesting_thread(); + int requesting_thread = process_state.requesting_thread(); if (requesting_thread != -1) { printf("\n"); printf("Thread %d (%s)\n", requesting_thread, - process_state->crashed() ? "crashed" : - "requested dump, did not crash"); - PrintStack(process_state->threads()->at(requesting_thread), cpu); + process_state.crashed() ? "crashed" : + "requested dump, did not crash"); + PrintStack(process_state.threads()->at(requesting_thread), cpu); } // Print all of the threads in the dump. - int thread_count = process_state->threads()->size(); + int thread_count = process_state.threads()->size(); for (int thread_index = 0; thread_index < thread_count; ++thread_index) { if (thread_index != requesting_thread) { // Don't print the crash thread again, it was already printed. printf("\n"); printf("Thread %d\n", thread_index); - PrintStack(process_state->threads()->at(thread_index), cpu); + PrintStack(process_state.threads()->at(thread_index), cpu); } } - PrintModules(process_state->modules()); + PrintModules(process_state.modules()); return true; } diff --git a/src/processor/process_state.cc b/src/processor/process_state.cc index 895ee5f7..d1e21cb8 100644 --- a/src/processor/process_state.cc +++ b/src/processor/process_state.cc @@ -40,13 +40,27 @@ namespace google_airbag { ProcessState::~ProcessState() { + Clear(); +} + +void ProcessState::Clear() { + time_date_stamp_ = 0; + crashed_ = false; + crash_reason_.clear(); + crash_address_ = 0; + requesting_thread_ = -1; for (vector::const_iterator iterator = threads_.begin(); iterator != threads_.end(); ++iterator) { delete *iterator; } - + threads_.clear(); + os_.clear(); + os_version_.clear(); + cpu_.clear(); + cpu_info_.clear(); delete modules_; + modules_ = NULL; } } // namespace google_airbag diff --git a/src/processor/simple_symbol_supplier.cc b/src/processor/simple_symbol_supplier.cc index 32b94342..e2bd9184 100644 --- a/src/processor/simple_symbol_supplier.cc +++ b/src/processor/simple_symbol_supplier.cc @@ -33,16 +33,19 @@ // // Author: Mark Mentovai +#include + #include "processor/simple_symbol_supplier.h" #include "google_airbag/processor/code_module.h" #include "processor/pathname_stripper.h" namespace google_airbag { -string SimpleSymbolSupplier::GetSymbolFileAtPath(const CodeModule *module, - const string &root_path) { +SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFileAtPath( + const CodeModule *module, const string &root_path, string *symbol_file) { + assert(symbol_file); if (!module) - return ""; + return NOT_FOUND; // Start with the base path. string path = root_path; @@ -51,14 +54,14 @@ string SimpleSymbolSupplier::GetSymbolFileAtPath(const CodeModule *module, path.append("/"); string debug_file_name = PathnameStripper::File(module->debug_file()); if (debug_file_name.empty()) - return ""; + return NOT_FOUND; path.append(debug_file_name); // Append the identifier as a directory name. path.append("/"); string identifier = module->debug_identifier(); if (identifier.empty()) - return ""; + return NOT_FOUND; path.append(identifier); // Transform the debug file name into one ending in .sym. If the existing @@ -76,7 +79,8 @@ string SimpleSymbolSupplier::GetSymbolFileAtPath(const CodeModule *module, } path.append(".sym"); - return path; + *symbol_file = path; + return FOUND; } } // namespace google_airbag diff --git a/src/processor/simple_symbol_supplier.h b/src/processor/simple_symbol_supplier.h index 30485d24..fdfcf0ca 100644 --- a/src/processor/simple_symbol_supplier.h +++ b/src/processor/simple_symbol_supplier.h @@ -92,13 +92,15 @@ class SimpleSymbolSupplier : public SymbolSupplier { // Returns the path to the symbol file for the given module. See the // description above. - virtual string GetSymbolFile(const CodeModule *module) { - return GetSymbolFileAtPath(module, path_); + virtual SymbolResult GetSymbolFile(const CodeModule *module, + string *symbol_file) { + return GetSymbolFileAtPath(module, path_, symbol_file); } protected: - string GetSymbolFileAtPath(const CodeModule *module, - const string &root_path); + SymbolResult GetSymbolFileAtPath(const CodeModule *module, + const string &root_path, + string *symbol_file); private: string path_; diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index 4e4a6b9f..edbe428a 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc @@ -34,6 +34,8 @@ // Author: Mark Mentovai +#include + #include "google_airbag/processor/stackwalker.h" #include "google_airbag/processor/call_stack.h" #include "google_airbag/processor/code_module.h" @@ -57,10 +59,10 @@ Stackwalker::Stackwalker(MemoryRegion *memory, const CodeModules *modules, } -CallStack* Stackwalker::Walk() { +bool Stackwalker::Walk(CallStack *stack) { + assert(stack); SourceLineResolver resolver; - - scoped_ptr stack(new CallStack()); + stack->Clear(); // stack_frame_info parallels the CallStack. The vector is passed to the // GetCallerFrame function. It contains information that may be helpful @@ -87,9 +89,18 @@ CallStack* Stackwalker::Walk() { if (module) { frame->module = module; if (!resolver.HasModule(frame->module->code_file()) && supplier_) { - string symbol_file = supplier_->GetSymbolFile(module); - if (!symbol_file.empty()) { - resolver.LoadModule(frame->module->code_file(), symbol_file); + string symbol_file; + SymbolSupplier::SymbolResult symbol_result = + supplier_->GetSymbolFile(module, &symbol_file); + + switch (symbol_result) { + case SymbolSupplier::FOUND: + resolver.LoadModule(frame->module->code_file(), symbol_file); + break; + case SymbolSupplier::NOT_FOUND: + break; // nothing to do + case SymbolSupplier::INTERRUPT: + return false; } } frame_info.reset(resolver.FillSourceLineInfo(frame.get())); @@ -105,10 +116,10 @@ CallStack* Stackwalker::Walk() { frame_info.reset(NULL); // Get the next frame and take ownership. - frame.reset(GetCallerFrame(stack.get(), stack_frame_info)); + frame.reset(GetCallerFrame(stack, stack_frame_info)); } - return stack.release(); + return true; }