In the AMD64 stackwalker, use heuristics to provide %rbp more often, as

subsequent frames are usually unable to use CFI if they don't have an %rbp
value.

a=mrmiller, r=jimb


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@960 4c0a9323-5329-0410-9bdc-e9ce6186880e

2 files changed, 110 insertions, 14 deletions

diff --git a/src/processor/stackwalker_amd64.cc b/src/processor/stackwalker_amd64.cc
index fd9ccdf3..5a9ddb19 100644
--- a/src/processor/stackwalker_amd64.cc
+++ b/src/processor/stackwalker_amd64.cc
@@ -146,18 +146,13 @@ StackFrameAMD64 *StackwalkerAMD64::GetCallerByStackScan(
     const vector<StackFrame *> &frames) {
   StackFrameAMD64 *last_frame = static_cast<StackFrameAMD64 *>(frames.back());
   u_int64_t last_rsp = last_frame->context.rsp;
-  u_int64_t caller_rsp, caller_rip;
-  
-  if (!ScanForReturnAddress(last_rsp, &caller_rsp, &caller_rip)) {
+  u_int64_t caller_rip_address, caller_rip;
+
+  if (!ScanForReturnAddress(last_rsp, &caller_rip_address, &caller_rip)) {
     // No plausible return address was found.
     return NULL;
   }
 
-  // ScanForReturnAddress found a reasonable return address. Advance
-  // %rsp to the location above the one where the return address was
-  // found.
-  caller_rsp += 8;
-
   // Create a new stack frame (ownership will be transferred to the caller)
   // and fill it in.
   StackFrameAMD64 *frame = new StackFrameAMD64();
@@ -165,10 +160,34 @@ StackFrameAMD64 *StackwalkerAMD64::GetCallerByStackScan(
   frame->trust = StackFrame::FRAME_TRUST_SCAN;
   frame->context = last_frame->context;
   frame->context.rip = caller_rip;
-  frame->context.rsp = caller_rsp;
+  // The caller's %rsp is directly underneath the return address pushed by
+  // the call.
+  frame->context.rsp = caller_rip_address + 8;
   frame->context_validity = StackFrameAMD64::CONTEXT_VALID_RIP |
                             StackFrameAMD64::CONTEXT_VALID_RSP;
 
+  // Other unwinders give up if they don't have an %rbp value, so see if we
+  // can pass some plausible value on.
+  if (last_frame->context_validity & StackFrameAMD64::CONTEXT_VALID_RBP) {
+    // Functions typically push their caller's %rbp immediately upon entry,
+    // and then set %rbp to point to that. So if the callee's %rbp is
+    // pointing to the first word below the alleged return address, presume
+    // that the caller's %rbp is saved there.
+    if (caller_rip_address - 8 == last_frame->context.rbp) {
+      u_int64_t caller_rbp = 0;
+      if (memory_->GetMemoryAtAddress(last_frame->context.rbp, &caller_rbp) &&
+          caller_rbp > caller_rip_address) {
+        frame->context.rbp = caller_rbp;
+        frame->context_validity |= StackFrameAMD64::CONTEXT_VALID_RBP;
+      }
+    } else if (last_frame->context.rbp >= caller_rip_address + 8) {
+      // If the callee's %rbp is plausible as a value for the caller's
+      // %rbp, presume that the callee left it unchanged.
+      frame->context.rbp = last_frame->context.rbp;
+      frame->context_validity |= StackFrameAMD64::CONTEXT_VALID_RBP;
+    }
+  }
+
   return frame;
 }
 
diff --git a/src/processor/stackwalker_amd64_unittest.cc b/src/processor/stackwalker_amd64_unittest.cc
index 0cb5fd57..a107bee8 100644
--- a/src/processor/stackwalker_amd64_unittest.cc
+++ b/src/processor/stackwalker_amd64_unittest.cc
@@ -182,7 +182,7 @@ TEST_F(GetCallerFrame, ScanWithoutSymbols) {
   stack_section.start() = 0x8000000080000000ULL;
   u_int64_t return_address1 = 0x50000000b0000100ULL;
   u_int64_t return_address2 = 0x50000000b0000900ULL;
-  Label frame1_sp, frame2_sp;
+  Label frame1_sp, frame2_sp, frame1_rbp;
   stack_section
     // frame 0
     .Append(16, 0)                      // space
@@ -198,6 +198,11 @@ TEST_F(GetCallerFrame, ScanWithoutSymbols) {
     .D64(0x40000000b0000000ULL)         // more junk
     .D64(0x50000000d0000000ULL)
 
+    .Mark(&frame1_rbp)
+    .D64(stack_section.start())         // This is in the right place to be
+                                        // a saved rbp, but it's bogus, so
+                                        // we shouldn't report it.
+
     .D64(return_address2)               // actual return address
     // frame 2
     .Mark(&frame2_sp)
@@ -206,6 +211,7 @@ TEST_F(GetCallerFrame, ScanWithoutSymbols) {
   RegionFromSection();
     
   raw_context.rip = 0x40000000c0000200ULL;
+  raw_context.rbp = frame1_rbp.Value();
   raw_context.rsp = stack_section.start().Value();
 
   StackwalkerAMD64 walker(&system_info, &raw_context, &stack_region, &modules,
@@ -222,10 +228,12 @@ TEST_F(GetCallerFrame, ScanWithoutSymbols) {
   StackFrameAMD64 *frame1 = static_cast<StackFrameAMD64 *>(frames->at(1));
   EXPECT_EQ(StackFrame::FRAME_TRUST_SCAN, frame1->trust);
   ASSERT_EQ((StackFrameAMD64::CONTEXT_VALID_RIP |
-             StackFrameAMD64::CONTEXT_VALID_RSP),
+             StackFrameAMD64::CONTEXT_VALID_RSP |
+             StackFrameAMD64::CONTEXT_VALID_RBP),
             frame1->context_validity);
   EXPECT_EQ(return_address1, frame1->context.rip);
   EXPECT_EQ(frame1_sp.Value(), frame1->context.rsp);
+  EXPECT_EQ(frame1_rbp.Value(), frame1->context.rbp);
 
   StackFrameAMD64 *frame2 = static_cast<StackFrameAMD64 *>(frames->at(2));
   EXPECT_EQ(StackFrame::FRAME_TRUST_SCAN, frame2->trust);
@@ -243,7 +251,7 @@ TEST_F(GetCallerFrame, ScanWithFunctionSymbols) {
   // lies within a function's bounds.
   stack_section.start() = 0x8000000080000000ULL;
   u_int64_t return_address = 0x50000000b0000110ULL;
-  Label frame1_sp;
+  Label frame1_sp, frame1_rbp;
 
   stack_section
     // frame 0
@@ -258,10 +266,12 @@ TEST_F(GetCallerFrame, ScanWithFunctionSymbols) {
     .D64(return_address)                // actual return address
     // frame 1
     .Mark(&frame1_sp)
-    .Append(32, 0);                     // end of stack
+    .Append(32, 0)                      // end of stack
+    .Mark(&frame1_rbp);
   RegionFromSection();
     
   raw_context.rip = 0x40000000c0000200ULL;
+  raw_context.rbp = frame1_rbp.Value();
   raw_context.rsp = stack_section.start().Value();
 
   SetModuleSymbols(&module1,
@@ -286,14 +296,81 @@ TEST_F(GetCallerFrame, ScanWithFunctionSymbols) {
   StackFrameAMD64 *frame1 = static_cast<StackFrameAMD64 *>(frames->at(1));
   EXPECT_EQ(StackFrame::FRAME_TRUST_SCAN, frame1->trust);
   ASSERT_EQ((StackFrameAMD64::CONTEXT_VALID_RIP |
-             StackFrameAMD64::CONTEXT_VALID_RSP),
+             StackFrameAMD64::CONTEXT_VALID_RSP |
+             StackFrameAMD64::CONTEXT_VALID_RBP),
             frame1->context_validity);
   EXPECT_EQ(return_address, frame1->context.rip);
   EXPECT_EQ(frame1_sp.Value(), frame1->context.rsp);
+  EXPECT_EQ(frame1_rbp.Value(), frame1->context.rbp);
   EXPECT_EQ("echidna", frame1->function_name);
   EXPECT_EQ(0x50000000b0000100ULL, frame1->function_base);
 }
 
+TEST_F(GetCallerFrame, CallerPushedRBP) {
+  // Functions typically push their %rbp upon entry and set %rbp pointing
+  // there.  If stackwalking finds a plausible address for the next frame's
+  // %rbp directly below the return address, assume that it is indeed the
+  // next frame's %rbp.
+  stack_section.start() = 0x8000000080000000ULL;
+  u_int64_t return_address = 0x50000000b0000110ULL;
+  Label frame0_rbp, frame1_sp, frame1_rbp;
+
+  stack_section
+    // frame 0
+    .Append(16, 0)                      // space
+
+    .D64(0x40000000b0000000ULL)         // junk that's not
+    .D64(0x50000000b0000000ULL)         // a return address
+
+    .D64(0x40000000c0001000ULL)         // a couple of plausible addresses
+    .D64(0x50000000b000aaaaULL)         // that are not within functions
+
+    .Mark(&frame0_rbp)
+    .D64(frame1_rbp)                    // caller-pushed %rbp
+    .D64(return_address)                // actual return address
+    // frame 1
+    .Mark(&frame1_sp)
+    .Append(32, 0)                      // body of frame1
+    .Mark(&frame1_rbp);                 // end of stack
+  RegionFromSection();
+
+  raw_context.rip = 0x40000000c0000200ULL;
+  raw_context.rbp = frame0_rbp.Value();
+  raw_context.rsp = stack_section.start().Value();
+
+  SetModuleSymbols(&module1,
+                   // The youngest frame's function.
+                   "FUNC 100 400 10 sasquatch\n");
+  SetModuleSymbols(&module2,
+                   // The calling frame's function.
+                   "FUNC 100 400 10 yeti\n");
+
+  StackwalkerAMD64 walker(&system_info, &raw_context, &stack_region, &modules,
+                          &supplier, &resolver);
+  ASSERT_TRUE(walker.Walk(&call_stack));
+  frames = call_stack.frames();
+  ASSERT_EQ(2U, frames->size());
+
+  StackFrameAMD64 *frame0 = static_cast<StackFrameAMD64 *>(frames->at(0));
+  EXPECT_EQ(StackFrame::FRAME_TRUST_CONTEXT, frame0->trust);
+  ASSERT_EQ(StackFrameAMD64::CONTEXT_VALID_ALL, frame0->context_validity);
+  EXPECT_EQ(frame0_rbp.Value(), frame0->context.rbp);
+  EXPECT_EQ("sasquatch", frame0->function_name);
+  EXPECT_EQ(0x40000000c0000100ULL, frame0->function_base);
+
+  StackFrameAMD64 *frame1 = static_cast<StackFrameAMD64 *>(frames->at(1));
+  EXPECT_EQ(StackFrame::FRAME_TRUST_SCAN, frame1->trust);
+  ASSERT_EQ((StackFrameAMD64::CONTEXT_VALID_RIP |
+             StackFrameAMD64::CONTEXT_VALID_RSP |
+             StackFrameAMD64::CONTEXT_VALID_RBP),
+            frame1->context_validity);
+  EXPECT_EQ(return_address, frame1->context.rip);
+  EXPECT_EQ(frame1_sp.Value(), frame1->context.rsp);
+  EXPECT_EQ(frame1_rbp.Value(), frame1->context.rbp);
+  EXPECT_EQ("yeti", frame1->function_name);
+  EXPECT_EQ(0x50000000b0000100ULL, frame1->function_base);
+}
+
 struct CFIFixture: public StackwalkerAMD64Fixture {
   CFIFixture() {
     // Provide a bunch of STACK CFI records; we'll walk to the caller