From f33b8d2d07a057fdd667c2e0db629ba7cbc37cc3 Mon Sep 17 00:00:00 2001 From: bryner Date: Fri, 8 Dec 2006 04:13:51 +0000 Subject: Provide a mechanism for SymbolSuppliers to interrupt processing (#93) git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@80 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/processor/call_stack.cc | 4 ++ src/processor/minidump_processor.cc | 30 +++++------ src/processor/minidump_processor_unittest.cc | 78 ++++++++++++++++++---------- src/processor/minidump_stackwalk.cc | 34 ++++++------ src/processor/process_state.cc | 16 +++++- src/processor/simple_symbol_supplier.cc | 16 +++--- src/processor/simple_symbol_supplier.h | 10 ++-- src/processor/stackwalker.cc | 27 +++++++--- 8 files changed, 136 insertions(+), 79 deletions(-) (limited to 'src/processor') 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) { +SymbolSupplier::SymbolResult TestSymbolSupplier::GetSymbolFile( + const CodeModule *module, string *symbol_file) { + if (interrupt_) { + return INTERRUPT; + } + 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"; + *symbol_file = string(getenv("srcdir") ? getenv("srcdir") : ".") + + "/src/processor/testdata/symbols/test_app.pdb/" + + module->debug_identifier() + + "/test_app.sym"; + return FOUND; } - return ""; + 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); - - CallStack *stack = state->threads()->at(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); 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; } -- cgit v1.2.1