Issue in StackwalkerAMD64::GetCallerByFramePointerRecovery.

src/processor/stackwalker_amd64.cc

Index: src/processor/stackwalker_amd64.cc
diff --git a/src/processor/stackwalker_amd64.cc b/src/processor/stackwalker_amd64.cc
index f252a33b71574e21dbf5b985978a20d12c9fcf07..9a745bc0482fcd75ec4488a283d9be368fa515c0 100644
--- a/src/processor/stackwalker_amd64.cc
+++ b/src/processor/stackwalker_amd64.cc
@@ -147,6 +147,23 @@ StackFrameAMD64* StackwalkerAMD64::GetCallerByCFIFrameInfo(
   return frame.release();
 }
 
+bool StackwalkerAMD64::IsEndOfStack(uint64_t caller_rip, uint64_t caller_rsp,
+                                    uint64_t callee_rsp) {
+  // Treat an instruction address of 0 as end-of-stack.
+  if (caller_rip == 0) {
+    return true;
+  }
+
+  // If the new stack pointer is at a lower address than the old, then
+  // that's clearly incorrect. Treat this as end-of-stack to enforce
+  // progress and avoid infinite loops.
+  if (caller_rsp < callee_rsp) {
+    return true;
+  }
+
+  return false;
+}
+
 StackFrameAMD64* StackwalkerAMD64::GetCallerByFramePointerRecovery(
     const vector<StackFrame*>& frames) {
   StackFrameAMD64* last_frame = static_cast<StackFrameAMD64*>(frames.back());
@@ -178,6 +195,11 @@ StackFrameAMD64* StackwalkerAMD64::GetCallerByFramePointerRecovery(
     if (caller_rbp < last_rbp || caller_rsp < last_rsp)

[Mark Mentovai, 2015/10/16 02:44:18]

This caller_rsp < last_rsp check is duplicated in IsEndOfStack().

Perhaps you should remove it from here and combine these into

  if (IsEndOfStack(caller_rip, caller_rsp, last_rsp) || caller_rbp < last_rbp) {

[ivanpe, 2015/10/16 03:35:42]

Done.

       return NULL;
 
+    if (IsEndOfStack(caller_rip, caller_rsp, last_rsp)) {
+      // Reached end-of-stack.
+      return NULL;
+    }
+
     StackFrameAMD64* frame = new StackFrameAMD64();
     frame->trust = StackFrame::FRAME_TRUST_FP;
     frame->context = last_frame->context;
@@ -284,15 +306,11 @@ StackFrame* StackwalkerAMD64::GetCallerFrame(const CallStack* stack,
     new_frame->context.rbp = static_cast<uint32_t>(new_frame->context.rbp);
   }
 
-  // Treat an instruction address of 0 as end-of-stack.
-  if (new_frame->context.rip == 0)
-    return NULL;
-
-  // If the new stack pointer is at a lower address than the old, then
-  // that's clearly incorrect. Treat this as end-of-stack to enforce
-  // progress and avoid infinite loops.
-  if (new_frame->context.rsp <= last_frame->context.rsp)
+  if (IsEndOfStack(new_frame->context.rip, new_frame->context.rsp,
+                   last_frame->context.rsp)) {
+    // Reached end-of-stack.
     return NULL;
+  }
 
   // new_frame->context.rip is the return address, which is the instruction
   // after the CALL that caused us to arrive at the callee. Set