aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorted.mielczarek <ted.mielczarek@4c0a9323-5329-0410-9bdc-e9ce6186880e>2009-10-08 14:21:50 +0000
committerted.mielczarek <ted.mielczarek@4c0a9323-5329-0410-9bdc-e9ce6186880e>2009-10-08 14:21:50 +0000
commit8d70618ffc6f87bfd3d7bfd05c87c35ec3179a7a (patch)
treee58154807c1a5f40d9465f4ee389144402c10cb3
parent10.6 SDK compatibility fixes. No bug. (diff)
downloadbreakpad-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.h22
-rw-r--r--src/google_breakpad/processor/stackwalker.h11
-rw-r--r--src/processor/minidump_stackwalk.cc22
-rw-r--r--src/processor/stackwalker.cc34
-rw-r--r--src/processor/stackwalker_x86.cc82
-rw-r--r--src/processor/stackwalker_x86.h13
-rw-r--r--src/processor/testdata/minidump2.stackwalk.out4
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)