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

Unified Diff: src/processor/stackwalker_amd64.cc

Issue 1902783002: Make x86-64 frame pointer unwinding stricter (Closed) Base URL: https://chromium.googlesource.com/breakpad/breakpad.git@master
Patch Set: Created 4 years, 8 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 | « no previous file | 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 8ccf73c42940d6fc6c278aebb05db4c5b14b024a..440724a1e3b8f94402f0a1927a9d27946081bfe9 100644
--- a/src/processor/stackwalker_amd64.cc
+++ b/src/processor/stackwalker_amd64.cc
@@ -164,6 +164,12 @@ bool StackwalkerAMD64::IsEndOfStack(uint64_t caller_rip, uint64_t caller_rsp,
return false;
}
+// Returns true if `ptr` is not in x86-64 canonical form.
+// https://en.wikipedia.org/wiki/X86-64#Virtual_address_space_details
+static bool is_non_canonical(uint64_t ptr) {
+ return ptr > 0x7FFFFFFFFFFF && ptr < 0xFFFF800000000000;
+}
+
StackFrameAMD64* StackwalkerAMD64::GetCallerByFramePointerRecovery(
const vector<StackFrame*>& frames) {
StackFrameAMD64* last_frame = static_cast<StackFrameAMD64*>(frames.back());
@@ -186,11 +192,22 @@ StackFrameAMD64* StackwalkerAMD64::GetCallerByFramePointerRecovery(
// %caller_rip = *(%callee_rbp + 8)
// %caller_rbp = *(%callee_rbp)
+ // If rbp is not 8-byte aligned it can't be a frame pointer.
+ if (last_rbp % 8 != 0) {
+ return NULL;
+ }
+
uint64_t caller_rip, caller_rbp;
if (memory_->GetMemoryAtAddress(last_rbp + 8, &caller_rip) &&
memory_->GetMemoryAtAddress(last_rbp, &caller_rbp)) {
uint64_t caller_rsp = last_rbp + 16;
+ // If the recovered rip is not a canonical address it can't be
+ // the return address, so rbp must not have been a frame pointer.
+ if (is_non_canonical(caller_rip)) {
+ return NULL;
+ }
+
// Simple sanity check that the stack is growing downwards as expected.
if (IsEndOfStack(caller_rip, caller_rsp, last_rsp) ||
caller_rbp < last_rbp) {
« no previous file with comments | « no previous file | src/processor/stackwalker_amd64_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698