Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(912)

Issue 501002: Allow StackwalkerARM to scan much farther to find caller of context frame (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 3 months ago by Ted Mielczarek
Modified:
11 years, 3 months ago
Reviewers:
jimb, Michael Krebs
CC:
google-breakpad-dev_googlegroups.com
Visibility:
Public.

Description

We're hitting a lot of crashes on ARM where the top frame is not in a known
function, so there's no unwind info. Unfortunately the caller frame is more than
120 bytes up the stack, so stack scanning fails to find it and we get a useless
crash signature. This patch allows StackwalkerARM to scan farther *only* to find
the caller of the context frame. This should help avoid getting bad frames
elsewhere in the stack while still fixing this case.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/google_breakpad/processor/stackwalker.h View 2 chunks +4 lines, -2 lines 0 comments Download
M src/processor/stackwalker.cc View 1 chunk +1 line, -0 lines 1 comment Download
M src/processor/stackwalker_arm.cc View 1 chunk +8 lines, -1 line 1 comment Download
M src/processor/stackwalker_arm_unittest.cc View 1 chunk +57 lines, -0 lines 0 comments Download

Messages

Total messages: 6
Ted Mielczarek
11 years, 3 months ago #1
Ted Mielczarek
11 years, 3 months ago #2
jimb
LGTM. http://breakpad.appspot.com/501002/diff/1/src/processor/stackwalker_arm.cc File src/processor/stackwalker_arm.cc (right): http://breakpad.appspot.com/501002/diff/1/src/processor/stackwalker_arm.cc#newcode170 src/processor/stackwalker_arm.cc:170: // farther down the stack. I read this ...
11 years, 3 months ago #3
Michael Krebs
Coincidentally, I'm seeing a problem on x86 where a stack frame is 40 words big. ...
11 years, 3 months ago #4
Ted Mielczarek
On 2012/12/05 01:07:09, Michael Krebs wrote: > Coincidentally, I'm seeing a problem on x86 where ...
11 years, 3 months ago #5
Michael Krebs
11 years, 3 months ago #6
On 2012/12/05 01:41:41, Ted Mielczarek wrote:
> This has been floated before. I don't think there's really much of a perf cost
> (it's just looking through the block of stack memory). My biggest worry has
been
> that it will lead to more bogus stack frames being included in stack traces.
> Perhaps this isn't a really valid concern and we should just let the scanner
> look through all available memory? You will probably get some really screwed
up
> stacks in certain cases with that behavior, but only in cases where you might
> otherwise not get frames at all, so maybe it's the right tradeoff.

That would be my thinking as well.  At least when I look at stack traces, I can
pretty quickly weed out what looks like bad stack frames (if I even look that
far down the trace), but it's painful if there aren't enough frames.

And I feel that with C++, at least, there can naturally be bigger stacks.

I just did a fairly unscientific experiment, where I ran minidump_stackwalk with
kRASearchWords set to 30 vs 10000.  With my test case, it took about 14-19 secs
with it set to 30, and took about 15-19 seconds with it set to 10000.  This was
basically just running it a dozen times each way, without much care to maintain
consistent machine load.  But it might still indicate a sub-5% increase in total
time.

Breakpad currently limits stack memory for each thread to 32k, which on a 32-bit
machine amounts to a max of 8k words.  So I'd be fine with letting it scan
through all available memory.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted