diff options
author | Ted Mielczarek <ted@mielczarek.org> | 2016-04-19 15:20:09 -0400 |
---|---|---|
committer | Ted Mielczarek <ted@mielczarek.org> | 2016-04-19 15:20:09 -0400 |
commit | ea2e22b3526754d5d60a405b6c8657f37a4d066a (patch) | |
tree | 7bcf3b0f95926d9ccd83c4c9719ff0faf6742726 /src/common/android/breakpad_getcontext.S | |
parent | Pass VERBOSE=1 to make check in travis to get failing test output (diff) | |
download | breakpad-ea2e22b3526754d5d60a405b6c8657f37a4d066a.tar.xz |
Make x86-64 frame pointer unwinding stricter
The x86-64 frame pointer-based unwind method will accept values
that aren't valid for the frame pointer register and the return address.
This fixes it to reject non-8-byte-aligned frame pointers, as
well as non-canonical addresses for the return address it finds.
A colleague of mine asked me why Breakpad gave a bad stack
for a crash in our crash-stats system:
https://crash-stats.mozilla.com/report/index/a472c842-2c7b-4ca7-a267-478cf2160405
Digging in, it turns out that the function in frame 0 is a leaf function,
so MSVC doesn't generate an entry in the unwind table for it, so
dump_syms doesn't produce a STACK CFI entry for it in the symbol file.
The stackwalker tries frame pointer unwinding, and %rbp is set to a
value that sort-of works, so it produces a garbage frame 1 and then
is lost. Either of the two checks in this patch would have stopped
the stackwalker from using the frame pointer.
It's possible we could do something smarter on the dump_syms side,
like enumerating all functions and outputing some default STACK CFI rule
for those that don't have unwind info, but that wouldn't fix crashes
from existing builds without re-dumping symbols for them. In any event,
these checks should always pass for valid frame pointer-using functions.
R=mark@chromium.org
BUG=https://bugzilla.mozilla.org/show_bug.cgi?id=1263001
Review URL: https://codereview.chromium.org/1902783002 .
Diffstat (limited to 'src/common/android/breakpad_getcontext.S')
0 files changed, 0 insertions, 0 deletions