From 0a636257d433df77242d8809efb62c216d60723a Mon Sep 17 00:00:00 2001 From: "ted.mielczarek@gmail.com" Date: Tue, 4 Dec 2012 19:30:31 +0000 Subject: Allow StackwalkerARM to scan much farther to find the caller of the context frame R=jimb at https://breakpad.appspot.com/501002/ git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1086 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/google_breakpad/processor/stackwalker.h | 6 ++- src/processor/stackwalker.cc | 1 + src/processor/stackwalker_arm.cc | 9 ++++- src/processor/stackwalker_arm_unittest.cc | 57 +++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/google_breakpad/processor/stackwalker.h b/src/google_breakpad/processor/stackwalker.h index 2979fef8..401c2f62 100644 --- a/src/google_breakpad/processor/stackwalker.h +++ b/src/google_breakpad/processor/stackwalker.h @@ -106,11 +106,14 @@ class Stackwalker { // Returns false otherwise. bool InstructionAddressSeemsValid(u_int64_t address); + // The default number of words to search through on the stack + // for a return address. + static const int kRASearchWords; + template bool ScanForReturnAddress(InstructionType location_start, InstructionType* location_found, InstructionType* ip_found) { - const int kRASearchWords = 30; return ScanForReturnAddress(location_start, location_found, ip_found, kRASearchWords); } @@ -185,7 +188,6 @@ class Stackwalker { static u_int32_t max_frames_; }; - } // namespace google_breakpad diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index 762c47f5..16880441 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc @@ -55,6 +55,7 @@ namespace google_breakpad { +const int Stackwalker::kRASearchWords = 30; u_int32_t Stackwalker::max_frames_ = 1024; Stackwalker::Stackwalker(const SystemInfo* system_info, diff --git a/src/processor/stackwalker_arm.cc b/src/processor/stackwalker_arm.cc index 2535cd0f..da07fbcf 100644 --- a/src/processor/stackwalker_arm.cc +++ b/src/processor/stackwalker_arm.cc @@ -166,7 +166,14 @@ StackFrameARM* StackwalkerARM::GetCallerByStackScan( u_int32_t last_sp = last_frame->context.iregs[MD_CONTEXT_ARM_REG_SP]; u_int32_t caller_sp, caller_pc; - if (!ScanForReturnAddress(last_sp, &caller_sp, &caller_pc)) { + // When searching for the caller of the context frame, + // allow the scanner to look farther down the stack. + const int kRASearchWords = frames.size() == 1 ? + Stackwalker::kRASearchWords * 4 : + Stackwalker::kRASearchWords; + + if (!ScanForReturnAddress(last_sp, &caller_sp, &caller_pc, + kRASearchWords)) { // No plausible return address was found. return NULL; } diff --git a/src/processor/stackwalker_arm_unittest.cc b/src/processor/stackwalker_arm_unittest.cc index 4c2ca047..a86729fd 100644 --- a/src/processor/stackwalker_arm_unittest.cc +++ b/src/processor/stackwalker_arm_unittest.cc @@ -306,6 +306,63 @@ TEST_F(GetCallerFrame, ScanWithFunctionSymbols) { EXPECT_EQ(0x50000100U, frame1->function_base); } +TEST_F(GetCallerFrame, ScanFirstFrame) { + // If the stackwalker resorts to stack scanning, it will scan much + // farther to find the caller of the context frame. + stack_section.start() = 0x80000000; + u_int32_t return_address1 = 0x50000100; + u_int32_t return_address2 = 0x50000900; + Label frame1_sp, frame2_sp; + stack_section + // frame 0 + .Append(32, 0) // space + + .D32(0x40090000) // junk that's not + .D32(0x60000000) // a return address + + .Append(96, 0) // more space + + .D32(return_address1) // actual return address + // frame 1 + .Mark(&frame1_sp) + .Append(32, 0) // space + + .D32(0xF0000000) // more junk + .D32(0x0000000D) + + .Append(96, 0) // more space + + .D32(return_address2) // actual return address + // (won't be found) + // frame 2 + .Mark(&frame2_sp) + .Append(32, 0); // end of stack + RegionFromSection(); + + raw_context.iregs[MD_CONTEXT_ARM_REG_PC] = 0x40005510; + raw_context.iregs[MD_CONTEXT_ARM_REG_SP] = stack_section.start().Value(); + + StackFrameSymbolizer frame_symbolizer(&supplier, &resolver); + StackwalkerARM walker(&system_info, &raw_context, -1, &stack_region, &modules, + &frame_symbolizer); + ASSERT_TRUE(walker.Walk(&call_stack)); + frames = call_stack.frames(); + ASSERT_EQ(2U, frames->size()); + + StackFrameARM *frame0 = static_cast(frames->at(0)); + EXPECT_EQ(StackFrame::FRAME_TRUST_CONTEXT, frame0->trust); + ASSERT_EQ(StackFrameARM::CONTEXT_VALID_ALL, frame0->context_validity); + EXPECT_EQ(0, memcmp(&raw_context, &frame0->context, sizeof(raw_context))); + + StackFrameARM *frame1 = static_cast(frames->at(1)); + EXPECT_EQ(StackFrame::FRAME_TRUST_SCAN, frame1->trust); + ASSERT_EQ((StackFrameARM::CONTEXT_VALID_PC | + StackFrameARM::CONTEXT_VALID_SP), + frame1->context_validity); + EXPECT_EQ(return_address1, frame1->context.iregs[MD_CONTEXT_ARM_REG_PC]); + EXPECT_EQ(frame1_sp.Value(), frame1->context.iregs[MD_CONTEXT_ARM_REG_SP]); +} + struct CFIFixture: public StackwalkerARMFixture { CFIFixture() { // Provide a bunch of STACK CFI records; we'll walk to the caller -- cgit v1.2.1