From be81ededf84628cca376bd79d7fb097f42da2709 Mon Sep 17 00:00:00 2001 From: jimblandy Date: Tue, 22 Jan 2013 22:38:41 +0000 Subject: [PATCH] Print the correct return address, even on architectures where StackFrame::instruction is offset. a=bruce.dawson, r=jimblandy git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1105 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/google_breakpad/processor/stack_frame.h | 30 +++++++++++++++---- .../processor/stack_frame_cpu.h | 6 ++++ src/processor/minidump_stackwalk.cc | 20 ++++++++----- src/processor/stackwalker_amd64.cc | 19 +++++++----- src/processor/stackwalker_x86.cc | 20 ++++++++----- .../minidump2.stackwalk.machine_readable.out | 6 ++-- .../testdata/minidump2.stackwalk.out | 6 ++-- 7 files changed, 72 insertions(+), 35 deletions(-) diff --git a/src/google_breakpad/processor/stack_frame.h b/src/google_breakpad/processor/stack_frame.h index 2f31e5c1..e8803804 100644 --- a/src/google_breakpad/processor/stack_frame.h +++ b/src/google_breakpad/processor/stack_frame.h @@ -83,11 +83,31 @@ struct StackFrame { } }; - // The program counter location as an absolute virtual address. For the - // innermost called frame in a stack, this will be an exact program counter - // or instruction pointer value. For all other frames, this will be within - // the instruction that caused execution to branch to a called function, - // but may not necessarily point to the exact beginning of that instruction. + // Return the actual return address, as saved on the stack or in a + // register. See the comments for 'instruction', below, for details. + virtual u_int64_t ReturnAddress() const { return instruction; } + + // The program counter location as an absolute virtual address. + // + // - For the innermost called frame in a stack, this will be an exact + // program counter or instruction pointer value. + // + // - For all other frames, this address is within the instruction that + // caused execution to branch to this frame's callee (although it may + // not point to the exact beginning of that instruction). This ensures + // that, when we look up the source code location for this frame, we + // get the source location of the call, not of the point at which + // control will resume when the call returns, which may be on the next + // line. (If the compiler knows the callee never returns, it may even + // place the call instruction at the very end of the caller's machine + // code, such that the "return address" (which will never be used) + // immediately after the call instruction is in an entirely different + // function, perhaps even from a different source file.) + // + // On some architectures, the return address as saved on the stack or in + // a register is fine for looking up the point of the call. On others, it + // requires adjustment. ReturnAddress returns the address as saved by the + // machine. u_int64_t instruction; // The module in which the instruction resides. diff --git a/src/google_breakpad/processor/stack_frame_cpu.h b/src/google_breakpad/processor/stack_frame_cpu.h index 90328760..34171f37 100644 --- a/src/google_breakpad/processor/stack_frame_cpu.h +++ b/src/google_breakpad/processor/stack_frame_cpu.h @@ -77,6 +77,9 @@ struct StackFrameX86 : public StackFrame { cfi_frame_info(NULL) {} ~StackFrameX86(); + // Overriden to return the return address as saved on the stack. + virtual u_int64_t ReturnAddress() const; + // Register state. This is only fully valid for the topmost frame in a // stack. In other frames, the values of nonvolatile registers may be // present, given sufficient debugging information. Refer to @@ -147,6 +150,9 @@ struct StackFrameAMD64 : public StackFrame { StackFrameAMD64() : context(), context_validity(CONTEXT_VALID_NONE) {} + // Overriden to return the return address as saved on the stack. + virtual u_int64_t ReturnAddress() const; + // Register state. This is only fully valid for the topmost frame in a // stack. In other frames, which registers are present depends on what // debugging information we had available. Refer to context_validity. diff --git a/src/processor/minidump_stackwalk.cc b/src/processor/minidump_stackwalk.cc index 7218fd88..e7edc963 100644 --- a/src/processor/minidump_stackwalk.cc +++ b/src/processor/minidump_stackwalk.cc @@ -144,6 +144,8 @@ static void PrintStack(const CallStack *stack, const string &cpu) { const StackFrame *frame = stack->frames()->at(frame_index); printf("%2d ", frame_index); + u_int64_t instruction_address = frame->ReturnAddress(); + if (frame->module) { printf("%s", PathnameStripper::File(frame->module->code_file()).c_str()); if (!frame->function_name.empty()) { @@ -153,16 +155,16 @@ static void PrintStack(const CallStack *stack, const string &cpu) { printf(" [%s : %d + 0x%" PRIx64 "]", source_file.c_str(), frame->source_line, - frame->instruction - frame->source_line_base); + instruction_address - frame->source_line_base); } else { - printf(" + 0x%" PRIx64, frame->instruction - frame->function_base); + printf(" + 0x%" PRIx64, instruction_address - frame->function_base); } } else { printf(" + 0x%" PRIx64, - frame->instruction - frame->module->base_address()); + instruction_address - frame->module->base_address()); } } else { - printf("0x%" PRIx64, frame->instruction); + printf("0x%" PRIx64, instruction_address); } printf("\n "); @@ -275,6 +277,8 @@ static void PrintStackMachineReadable(int thread_num, const CallStack *stack) { printf("%d%c%d%c", thread_num, kOutputSeparator, frame_index, kOutputSeparator); + u_int64_t instruction_address = frame->ReturnAddress(); + if (frame->module) { assert(!frame->module->code_file().empty()); printf("%s", StripSeparator(PathnameStripper::File( @@ -289,13 +293,13 @@ static void PrintStackMachineReadable(int thread_num, const CallStack *stack) { kOutputSeparator, frame->source_line, kOutputSeparator, - frame->instruction - frame->source_line_base); + instruction_address - frame->source_line_base); } else { printf("%c%c%c0x%" PRIx64, kOutputSeparator, // empty source file kOutputSeparator, // empty source line kOutputSeparator, - frame->instruction - frame->function_base); + instruction_address - frame->function_base); } } else { printf("%c%c%c%c0x%" PRIx64, @@ -303,7 +307,7 @@ static void PrintStackMachineReadable(int thread_num, const CallStack *stack) { kOutputSeparator, // empty source file kOutputSeparator, // empty source line kOutputSeparator, - frame->instruction - frame->module->base_address()); + instruction_address - frame->module->base_address()); } } else { // the printf before this prints a trailing separator for module name @@ -312,7 +316,7 @@ static void PrintStackMachineReadable(int thread_num, const CallStack *stack) { kOutputSeparator, // empty source file kOutputSeparator, // empty source line kOutputSeparator, - frame->instruction); + instruction_address); } printf("\n"); } diff --git a/src/processor/stackwalker_amd64.cc b/src/processor/stackwalker_amd64.cc index 0fec3d71..3aa7c736 100644 --- a/src/processor/stackwalker_amd64.cc +++ b/src/processor/stackwalker_amd64.cc @@ -33,6 +33,7 @@ // // Author: Mark Mentovai, Ted Mielczarek +#include #include "common/scoped_ptr.h" #include "google_breakpad/processor/call_stack.h" @@ -100,6 +101,11 @@ StackwalkerAMD64::StackwalkerAMD64(const SystemInfo* system_info, (sizeof(cfi_register_map_) / sizeof(cfi_register_map_[0]))) { } +u_int64_t StackFrameAMD64::ReturnAddress() const +{ + assert(context_validity & StackFrameAMD64::CONTEXT_VALID_RIP); + return context.rip; +} StackFrame* StackwalkerAMD64::GetContextFrame() { if (!context_) { @@ -226,14 +232,11 @@ StackFrame* StackwalkerAMD64::GetCallerFrame(const CallStack* stack) { if (new_frame->context.rsp <= last_frame->context.rsp) return NULL; - // new_frame->context.rip is the return address, which is one instruction - // past the CALL that caused us to arrive at the callee. Set - // new_frame->instruction to one less than that. This won't reference the - // beginning of the CALL instruction, but it's guaranteed to be within - // the CALL, which is sufficient to get the source line information to - // match up with the line that contains a function call. Callers that - // require the exact return address value may access the context.rip - // field of StackFrameAMD64. + // new_frame->context.rip is the return address, which is the instruction + // after the CALL that caused us to arrive at the callee. Set + // new_frame->instruction to one less than that, so it points within the + // CALL instruction. See StackFrame::instruction for details, and + // StackFrameAMD64::ReturnAddress. new_frame->instruction = new_frame->context.rip - 1; return new_frame.release(); diff --git a/src/processor/stackwalker_x86.cc b/src/processor/stackwalker_x86.cc index 28e27809..19fc2a11 100644 --- a/src/processor/stackwalker_x86.cc +++ b/src/processor/stackwalker_x86.cc @@ -33,6 +33,7 @@ // // Author: Mark Mentovai +#include #include #include "common/scoped_ptr.h" @@ -105,6 +106,12 @@ StackFrameX86::~StackFrameX86() { cfi_frame_info = NULL; } +u_int64_t StackFrameX86::ReturnAddress() const +{ + assert(context_validity & StackFrameX86::CONTEXT_VALID_EIP); + return context.eip; +} + StackFrame* StackwalkerX86::GetContextFrame() { if (!context_) { BPLOG(ERROR) << "Can't get context frame without context"; @@ -597,14 +604,11 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack* stack) { if (new_frame->context.esp <= last_frame->context.esp) return NULL; - // new_frame->context.eip is the return address, which is one instruction - // past the CALL that caused us to arrive at the callee. Set - // new_frame->instruction to one less than that. This won't reference the - // beginning of the CALL instruction, but it's guaranteed to be within - // the CALL, which is sufficient to get the source line information to - // match up with the line that contains a function call. Callers that - // require the exact return address value may access the context.eip - // field of StackFrameX86. + // new_frame->context.eip is the return address, which is the instruction + // after the CALL that caused us to arrive at the callee. Set + // new_frame->instruction to one less than that, so it points within the + // CALL instruction. See StackFrame::instruction for details, and + // StackFrameAMD64::ReturnAddress. new_frame->instruction = new_frame->context.eip - 1; return new_frame.release(); diff --git a/src/processor/testdata/minidump2.stackwalk.machine_readable.out b/src/processor/testdata/minidump2.stackwalk.machine_readable.out index 2f3b0f59..60f8af4c 100644 --- a/src/processor/testdata/minidump2.stackwalk.machine_readable.out +++ b/src/processor/testdata/minidump2.stackwalk.machine_readable.out @@ -16,6 +16,6 @@ Module|kernel32.dll|5.1.2600.2945|kernel32.pdb|BCE8785C57B44245A669896B6A19B9542 Module|ntdll.dll|5.1.2600.2180|ntdll.pdb|36515FB5D04345E491F672FA2E2878C02|0x7c900000|0x7c9affff|0 0|0|test_app.exe|`anonymous namespace'::CrashFunction|c:\test_app.cc|58|0x3 -0|1|test_app.exe|main|c:\test_app.cc|65|0x4 -0|2|test_app.exe|__tmainCRTStartup|f:\sp\vctools\crt_bld\self_x86\crt\src\crt0.c|327|0x11 -0|3|kernel32.dll|BaseProcessStart|||0x22 +0|1|test_app.exe|main|c:\test_app.cc|65|0x5 +0|2|test_app.exe|__tmainCRTStartup|f:\sp\vctools\crt_bld\self_x86\crt\src\crt0.c|327|0x12 +0|3|kernel32.dll|BaseProcessStart|||0x23 diff --git a/src/processor/testdata/minidump2.stackwalk.out b/src/processor/testdata/minidump2.stackwalk.out index 5b988679..20d1c4d8 100644 --- a/src/processor/testdata/minidump2.stackwalk.out +++ b/src/processor/testdata/minidump2.stackwalk.out @@ -13,13 +13,13 @@ Thread 0 (crashed) 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] + 1 test_app.exe!main [test_app.cc : 65 + 0x5] eip = 0x00404200 esp = 0x0012fe90 ebp = 0x0012ff70 Found by: call frame info - 2 test_app.exe!__tmainCRTStartup [crt0.c : 327 + 0x11] + 2 test_app.exe!__tmainCRTStartup [crt0.c : 327 + 0x12] eip = 0x004053ec esp = 0x0012ff78 ebp = 0x0012ffc0 Found by: call frame info - 3 kernel32.dll!BaseProcessStart + 0x22 + 3 kernel32.dll!BaseProcessStart + 0x23 eip = 0x7c816fd7 esp = 0x0012ffc8 ebp = 0x0012fff0 Found by: call frame info