diff options
author | ted.mielczarek <ted.mielczarek@4c0a9323-5329-0410-9bdc-e9ce6186880e> | 2009-10-08 14:21:50 +0000 |
---|---|---|
committer | ted.mielczarek <ted.mielczarek@4c0a9323-5329-0410-9bdc-e9ce6186880e> | 2009-10-08 14:21:50 +0000 |
commit | 8d70618ffc6f87bfd3d7bfd05c87c35ec3179a7a (patch) | |
tree | e58154807c1a5f40d9465f4ee389144402c10cb3 | |
parent | 10.6 SDK compatibility fixes. No bug. (diff) | |
download | breakpad-8d70618ffc6f87bfd3d7bfd05c87c35ec3179a7a.tar.xz |
Let x86 stackwalker scan stack in cases where program evaluation fails. Original patch by Jeff Muizelaar <jmuizelaar@mozilla.com> with some changes by me. r=mento at http://breakpad.appspot.com/32003/show
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@409 4c0a9323-5329-0410-9bdc-e9ce6186880e
-rw-r--r-- | src/google_breakpad/processor/stack_frame_cpu.h | 22 | ||||
-rw-r--r-- | src/google_breakpad/processor/stackwalker.h | 11 | ||||
-rw-r--r-- | src/processor/minidump_stackwalk.cc | 22 | ||||
-rw-r--r-- | src/processor/stackwalker.cc | 34 | ||||
-rw-r--r-- | src/processor/stackwalker_x86.cc | 82 | ||||
-rw-r--r-- | src/processor/stackwalker_x86.h | 13 | ||||
-rw-r--r-- | src/processor/testdata/minidump2.stackwalk.out | 4 |
7 files changed, 161 insertions, 27 deletions
diff --git a/src/google_breakpad/processor/stack_frame_cpu.h b/src/google_breakpad/processor/stack_frame_cpu.h index 70823b9c..3d3003b7 100644 --- a/src/google_breakpad/processor/stack_frame_cpu.h +++ b/src/google_breakpad/processor/stack_frame_cpu.h @@ -58,7 +58,23 @@ struct StackFrameX86 : public StackFrame { CONTEXT_VALID_ALL = -1 }; - StackFrameX86() : context(), context_validity(CONTEXT_VALID_NONE) {} + // Indicates how well we trust the instruction pointer we derived + // during stack walking. Since the stack walker can resort to + // stack scanning, we can wind up with dubious frames. + // In rough order of "trust metric". + enum FrameTrust { + FRAME_TRUST_NONE, // Unknown + FRAME_TRUST_SCAN, // Scanned the stack, found this + FRAME_TRUST_CFI_SCAN, // Scanned the stack using call frame info, found this + FRAME_TRUST_FP, // Derived from frame pointer + FRAME_TRUST_CFI, // Derived from call frame info + FRAME_TRUST_CONTEXT // Given as instruction pointer in a context + }; + + StackFrameX86() + : context(), + context_validity(CONTEXT_VALID_NONE), + trust(FRAME_TRUST_NONE) {} // Register state. This is only fully valid for the topmost frame in a // stack. In other frames, the values of nonvolatile registers may be @@ -70,6 +86,10 @@ struct StackFrameX86 : public StackFrame { // the OR operator doesn't work well with enumerated types. This indicates // which fields in context are valid. int context_validity; + + // Amount of trust the stack walker has in the instruction pointer + // of this frame. + FrameTrust trust; }; struct StackFramePPC : public StackFrame { diff --git a/src/google_breakpad/processor/stackwalker.h b/src/google_breakpad/processor/stackwalker.h index c463fd80..90274aae 100644 --- a/src/google_breakpad/processor/stackwalker.h +++ b/src/google_breakpad/processor/stackwalker.h @@ -42,6 +42,7 @@ #define GOOGLE_BREAKPAD_PROCESSOR_STACKWALKER_H__ #include <vector> +#include "google_breakpad/common/breakpad_types.h" namespace google_breakpad { @@ -95,6 +96,16 @@ class Stackwalker { SymbolSupplier *supplier, SourceLineResolverInterface *resolver); + // This can be used to filter out potential return addresses when + // the stack walker resorts to stack scanning. + // Returns true if any of: + // * This address is within a loaded module, but we don't have symbols + // for that module. + // * This address is within a loaded module for which we have symbols, + // and falls inside a function in that module. + // Returns false otherwise. + bool InstructionAddressSeemsValid(u_int64_t address); + // Information about the system that produced the minidump. Subclasses // and the SymbolSupplier may find this information useful. const SystemInfo *system_info_; diff --git a/src/processor/minidump_stackwalk.cc b/src/processor/minidump_stackwalk.cc index d3a830e3..01f0c0c6 100644 --- a/src/processor/minidump_stackwalk.cc +++ b/src/processor/minidump_stackwalk.cc @@ -160,6 +160,28 @@ static void PrintStack(const CallStack *stack, const string &cpu) { sequence = PrintRegister("edx", frame_x86->context.edx, sequence); sequence = PrintRegister("efl", frame_x86->context.eflags, sequence); } + const char *trust_name; + switch (frame_x86->trust) { + case StackFrameX86::FRAME_TRUST_NONE: + trust_name = "unknown"; + break; + case StackFrameX86::FRAME_TRUST_CONTEXT: + trust_name = "given as instruction pointer in context"; + break; + case StackFrameX86::FRAME_TRUST_CFI: + trust_name = "call frame info"; + break; + case StackFrameX86::FRAME_TRUST_CFI_SCAN: + trust_name = "call frame info with scanning"; + break; + case StackFrameX86::FRAME_TRUST_FP: + trust_name = "previous frame's frame pointer"; + break; + case StackFrameX86::FRAME_TRUST_SCAN: + trust_name = "stack scanning"; + break; + } + printf("\n Found by: %s", trust_name); } else if (cpu == "ppc") { const StackFramePPC *frame_ppc = reinterpret_cast<const StackFramePPC*>(frame); diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index de67bdaa..96ee6db0 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc @@ -189,5 +189,39 @@ Stackwalker* Stackwalker::StackwalkerForCPU( return cpu_stackwalker; } +bool Stackwalker::InstructionAddressSeemsValid(u_int64_t address) { + const CodeModule *module = modules_->GetModuleForAddress(address); + if (!module) { + // not inside any loaded module + return false; + } + + if (!resolver_ || !supplier_) { + // we don't have a resolver and or symbol supplier, + // but we're inside a known module + return true; + } + + if (!resolver_->HasModule(module->code_file())) { + string symbol_data, symbol_file; + SymbolSupplier::SymbolResult symbol_result = + supplier_->GetSymbolFile(module, system_info_, + &symbol_file, &symbol_data); + + if (symbol_result != SymbolSupplier::FOUND || + !resolver_->LoadModuleUsingMapBuffer(module->code_file(), + symbol_data)) { + // we don't have symbols, but we're inside a loaded module + return true; + } + } + + StackFrame frame; + frame.module = module; + frame.instruction = address; + resolver_->FillSourceLineInfo(&frame); + // we have symbols, so return true if inside a function + return !frame.function_name.empty(); +} } // namespace google_breakpad diff --git a/src/processor/stackwalker_x86.cc b/src/processor/stackwalker_x86.cc index e39df17f..4a7e811d 100644 --- a/src/processor/stackwalker_x86.cc +++ b/src/processor/stackwalker_x86.cc @@ -79,6 +79,7 @@ StackFrame* StackwalkerX86::GetContextFrame() { // straight out of the CPU context structure. frame->context = *context_; frame->context_validity = StackFrameX86::CONTEXT_VALID_ALL; + frame->trust = StackFrameX86::FRAME_TRUST_CONTEXT; frame->instruction = frame->context.eip; return frame; @@ -92,7 +93,7 @@ StackFrame* StackwalkerX86::GetCallerFrame( BPLOG(ERROR) << "Can't get caller frame without memory or stack"; return NULL; } - + StackFrameX86::FrameTrust trust = StackFrameX86::FRAME_TRUST_NONE; StackFrameX86 *last_frame = static_cast<StackFrameX86*>( stack->frames()->back()); StackFrameInfo *last_frame_info = stack_frame_info.back().get(); @@ -183,6 +184,7 @@ StackFrame* StackwalkerX86::GetCallerFrame( if (last_frame_info && last_frame_info->valid == StackFrameInfo::VALID_ALL) { // FPO data available. traditional_frame = false; + trust = StackFrameX86::FRAME_TRUST_CFI; if (!last_frame_info->program_string.empty()) { // The FPO data has its own program string, which will tell us how to // get to the caller frame, and may even fill in the values of @@ -280,6 +282,7 @@ StackFrame* StackwalkerX86::GetCallerFrame( // %eip_new = *(%ebp_old + 4) // %esp_new = %ebp_old + 8 // %ebp_new = *(%ebp_old) + trust = StackFrameX86::FRAME_TRUST_FP; program_string = "$eip $ebp 4 + ^ = " "$esp $ebp 8 + = " "$ebp $ebp ^ ="; @@ -293,7 +296,26 @@ StackFrame* StackwalkerX86::GetCallerFrame( if (!evaluator.Evaluate(program_string, &dictionary_validity) || dictionary_validity.find("$eip") == dictionary_validity.end() || dictionary_validity.find("$esp") == dictionary_validity.end()) { - return NULL; + // Program string evaluation failed. It may be that %eip is not somewhere + // with stack frame info, and %ebp is pointing to non-stack memory, so + // our evaluation couldn't succeed. We'll scan the stack for a return + // address. This can happen if the stack is in a module for which + // we don't have symbols, and that module is compiled without a + // frame pointer. + u_int32_t location_start = last_frame->context.esp; + u_int32_t location, eip; + if (!ScanForReturnAddress(location_start, location, eip)) { + // if we can't find an instruction pointer even with stack scanning, + // give up. + return NULL; + } + + // This seems like a reasonable return address. Since program string + // evaluation failed, use it and set %esp to the location above the + // one where the return address was found. + dictionary["$eip"] = eip; + dictionary["$esp"] = location + 4; + trust = StackFrameX86::FRAME_TRUST_SCAN; } // If this stack frame did not use %ebp in a traditional way, locating the @@ -321,33 +343,18 @@ StackFrame* StackwalkerX86::GetCallerFrame( u_int32_t eip = dictionary["$eip"]; if (modules_ && !modules_->GetModuleForAddress(eip)) { - const int kRASearchWords = 15; - // The instruction pointer at .raSearchStart was invalid, so start // looking one 32-bit word above that location. u_int32_t location_start = dictionary[".raSearchStart"] + 4; - - for (u_int32_t location = location_start; - location <= location_start + kRASearchWords * 4; - location += 4) { - if (!memory_->GetMemoryAtAddress(location, &eip)) - break; - - if (modules_->GetModuleForAddress(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. - // - // TODO(mmentovai): The return-address check can be made even - // stronger in modules for which debugging data is available. In - // that case, it's possible to check that the candidate return - // address is inside a known function. - - dictionary["$eip"] = eip; - dictionary["$esp"] = location + 4; - offset = location - location_start; - break; - } + u_int32_t location; + if (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. + dictionary["$eip"] = eip; + dictionary["$esp"] = location + 4; + offset = location - location_start; + trust = StackFrameX86::FRAME_TRUST_CFI_SCAN; } } @@ -392,6 +399,7 @@ StackFrame* StackwalkerX86::GetCallerFrame( // and fill it in. StackFrameX86 *frame = new StackFrameX86(); + frame->trust = trust; frame->context = last_frame->context; frame->context.eip = dictionary["$eip"]; frame->context.esp = dictionary["$esp"]; @@ -428,5 +436,27 @@ StackFrame* StackwalkerX86::GetCallerFrame( return frame; } +bool StackwalkerX86::ScanForReturnAddress(u_int32_t location_start, + u_int32_t &location_found, + u_int32_t &eip_found) { + const int kRASearchWords = 15; + for (u_int32_t location = location_start; + location <= location_start + kRASearchWords * 4; + location += 4) { + u_int32_t eip; + if (!memory_->GetMemoryAtAddress(location, &eip)) + break; + + if (modules_->GetModuleForAddress(eip) && + InstructionAddressSeemsValid(eip)) { + + eip_found = eip; + location_found = location; + return true; + } + } + // nothing found + return false; +} } // namespace google_breakpad diff --git a/src/processor/stackwalker_x86.h b/src/processor/stackwalker_x86.h index 14b9e8b9..d4137ebf 100644 --- a/src/processor/stackwalker_x86.h +++ b/src/processor/stackwalker_x86.h @@ -70,6 +70,19 @@ class StackwalkerX86 : public Stackwalker { const CallStack *stack, const vector< linked_ptr<StackFrameInfo> > &stack_frame_info); + // Scan the stack starting at location_start, looking for an address + // that looks like a valid instruction pointer. Addresses must + // 1) be contained in the current stack memory + // 2) pass the checks in Stackwalker::InstructionAddressSeemsValid + // + // Returns true if a valid-looking instruction pointer was found. + // When returning true, sets location_found to the address at which + // the value was found, and eip_found to the value contained at that + // location in memory. + bool ScanForReturnAddress(u_int32_t location_start, + u_int32_t &location_found, + u_int32_t &eip_found); + // Stores the CPU context corresponding to the innermost stack frame to // be returned by GetContextFrame. const MDRawContextX86 *context_; diff --git a/src/processor/testdata/minidump2.stackwalk.out b/src/processor/testdata/minidump2.stackwalk.out index 21b425de..be081f41 100644 --- a/src/processor/testdata/minidump2.stackwalk.out +++ b/src/processor/testdata/minidump2.stackwalk.out @@ -12,12 +12,16 @@ Thread 0 (crashed) eip = 0x0040429e esp = 0x0012fe84 ebp = 0x0012fe88 ebx = 0x7c80abc1 esi = 0x00000002 edi = 0x00000a28 eax = 0x00000045 ecx = 0x0012fe94 edx = 0x0042bc58 efl = 0x00010246 + Found by: given as instruction pointer in context 1 test_app.exe!main [test_app.cc : 65 + 0x4] eip = 0x00404200 esp = 0x0012fe90 ebp = 0x0012ff70 + Found by: call frame info 2 test_app.exe!__tmainCRTStartup [crt0.c : 327 + 0x11] eip = 0x004053ec esp = 0x0012ff78 ebp = 0x0012ffc0 + Found by: call frame info 3 kernel32.dll!BaseProcessStart + 0x22 eip = 0x7c816fd7 esp = 0x0012ffc8 ebp = 0x0012fff0 + Found by: call frame info Loaded modules: 0x00400000 - 0x0042cfff test_app.exe ??? (main) |