Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(546)

Unified Diff: src/processor/stackwalker_amd64.cc

Issue 1408973002: Issue in StackwalkerAMD64::GetCallerByFramePointerRecovery. (Closed) Base URL: https://chromium.googlesource.com/breakpad/breakpad.git@master
Patch Set: Combining IsEndOfStack and rbp checks Created 5 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/processor/stackwalker_amd64.h ('k') | src/processor/stackwalker_amd64_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/processor/stackwalker_amd64.cc
diff --git a/src/processor/stackwalker_amd64.cc b/src/processor/stackwalker_amd64.cc
index f252a33b71574e21dbf5b985978a20d12c9fcf07..8ccf73c42940d6fc6c278aebb05db4c5b14b024a 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());
@@ -175,8 +192,11 @@ StackFrameAMD64* StackwalkerAMD64::GetCallerByFramePointerRecovery(
uint64_t caller_rsp = last_rbp + 16;
// Simple sanity check that the stack is growing downwards as expected.
- if (caller_rbp < last_rbp || caller_rsp < last_rsp)
+ if (IsEndOfStack(caller_rip, caller_rsp, last_rsp) ||
+ caller_rbp < last_rbp) {
+ // Reached end-of-stack or stack is not growing downwards.
return NULL;
+ }
StackFrameAMD64* frame = new StackFrameAMD64();
frame->trust = StackFrame::FRAME_TRUST_FP;
@@ -284,15 +304,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
« no previous file with comments | « src/processor/stackwalker_amd64.h ('k') | src/processor/stackwalker_amd64_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698