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

Issue 413002: Allow processing dumps with missing stack memory for some threads (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 5 months ago by Ted Mielczarek
Modified:
6 years, 1 month ago
CC:
google-breakpad-dev_googlegroups.com
Visibility:
Public.

Description

I'm not sure if this patch is really something we want to take, but we started
seeing some minidumps with missing stack memory for a few threads (we're writing
minidumps of the Flash player's sandboxed subprocess without its cooperation, so
there's probably some issues there). This patch lets us get stacks for the
threads that work. In general I think getting some data out of a dump is better
than nothing, but as-implemented this kind of sucks because it just drops
threads without stack memory on the floor.

Patch Set 1 #

Total comments: 1

Patch Set 2 : Updated patch #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/google_breakpad/processor/minidump.h View 1 7 chunks +50 lines, -40 lines 0 comments Download
M src/google_breakpad/processor/minidump_processor.h View 1 1 chunk +0 lines, -9 lines 0 comments Download
M src/processor/minidump.cc View 1 1 chunk +4 lines, -4 lines 0 comments Download
M src/processor/minidump_processor.cc View 1 1 chunk +10 lines, -8 lines 0 comments Download
M src/processor/minidump_processor_unittest.cc View 1 4 chunks +192 lines, -17 lines 0 comments Download
M src/processor/minidump_stackwalk.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download
M src/processor/minidump_unittest.cc View 1 1 chunk +87 lines, -0 lines 0 comments Download
M src/processor/stackwalker_amd64.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/processor/stackwalker_amd64_unittest.cc View 1 1 chunk +18 lines, -0 lines 0 comments Download
M src/processor/stackwalker_arm.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/processor/stackwalker_arm_unittest.cc View 1 1 chunk +15 lines, -0 lines 0 comments Download
M src/processor/stackwalker_ppc.cc View 1 1 chunk +3 lines, -3 lines 0 comments Download
M src/processor/stackwalker_sparc.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/processor/stackwalker_x86.cc View 1 2 chunks +3 lines, -3 lines 0 comments Download
M src/processor/stackwalker_x86_unittest.cc View 1 1 chunk +17 lines, -0 lines 0 comments Download
M src/processor/synth_minidump.h View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 13
Ted Mielczarek
6 years, 5 months ago #1
Ted Mielczarek
Mark, what do you think about this?
6 years, 5 months ago #2
Mark Mentovai
http://breakpad.appspot.com/413002/diff/1/src/processor/minidump.cc File src/processor/minidump.cc (right): http://breakpad.appspot.com/413002/diff/1/src/processor/minidump.cc#newcode1330 src/processor/minidump.cc:1330: valid_ = true; To me, valid_ implies that everything ...
6 years, 5 months ago #3
Ted Mielczarek
On 2012/07/10 18:18:32, Mark Mentovai wrote: > http://breakpad.appspot.com/413002/diff/1/src/processor/minidump.cc#newcode1330 > src/processor/minidump.cc:1330: valid_ = true; > To ...
6 years, 5 months ago #4
Mark Mentovai
Well, right, you’d need a way to signal “invalid by reason of no stack memory” ...
6 years, 5 months ago #5
Ted Mielczarek
On Tue, Jul 10, 2012 at 3:51 PM, <mark@chromium.org> wrote: > Well, right, you’d need ...
6 years, 5 months ago #6
Mark Mentovai
I’d like to get at least the context frame, or at the very worst, a ...
6 years, 5 months ago #7
Ted Mielczarek
On Tue, Jul 10, 2012 at 4:00 PM, Mark Mentovai <mark@chromium.org> wrote: > I’d like ...
6 years, 5 months ago #8
Mark Mentovai
It’s more palatable if it’s documented. On Tue, Jul 10, 2012 at 8:16 PM, Ted ...
6 years, 5 months ago #9
Ted Mielczarek
6 years, 1 month ago #10
Ted Mielczarek
Okay, I made the requested changes. The rest of this patch is mostly bloat from ...
6 years, 1 month ago #11
Michael Krebs
BTW, I just applied this patch in order to stackwalk a minidump gotten due to ...
6 years, 1 month ago #12
Michael Krebs
6 years, 1 month ago #13
LGTM.

So I got a chance to take a look at this, and it seems fine to me.  And, as I
mentioned, I successfully tried this on an actual minidump where a thread was
missing its stack memory.  See issue crosbug.com/35846.
Sign in to reply to this message.

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