Chromium Code Reviews (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out

Issue 1902783002: Make x86-64 frame pointer unwinding stricter (Closed)

4 years, 8 months ago by Ted Mielczarek
4 years, 8 months ago
Mark Mentovai
Base URL:
Target Ref:


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: 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. BUG= Committed:

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+169 lines, -51 lines) Patch
M src/processor/ View 2 chunks +17 lines, -0 lines 0 comments Download
M src/processor/ View 26 chunks +152 lines, -51 lines 0 comments Download


Total messages: 4 (1 generated)
Ted Mielczarek
4 years, 8 months ago (2016-04-19 14:21:13 UTC) #1
Mark Mentovai
4 years, 8 months ago (2016-04-19 15:26:28 UTC) #2
Ted Mielczarek
4 years, 8 months ago (2016-04-19 19:20:14 UTC) #4
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
ea2e22b3526754d5d60a405b6c8657f37a4d066a (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698