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

Issue 4724002: Handle failures of copying process data from a core file. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 1 month ago by Ben Chan
Modified:
9 years, 1 month ago
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Handle failures of copying process data from a core file.

When LinuxCoreDumper fails to copy process data from a core file, it
fills the return buffer with a repeated sequence of a special marker.
However, MinidumpWriter doesn't know about that and may incorrectly
interpret the data. In many cases, MinidumpWriter simply copies the
gibberish data to the minidump, which isn't too bad. However, the
gibberish data may cause MinidumpWriter to behave badly in some other
cases. For example, when MinidumpWriter tries to iterate through the
linked list of all loaded DSOs via the r_map field of a r_debug struct,
if the linked list is filed with the special marker, the code keeps
iterating through the same address.

This CL addresses the issue by having LinuxCoreDumper::CopyFromProcess()
returns a Boolean value to indicate if the expected data is found from
the core file. MinidumpWriter can then decide how to handle that.

BUG=chromium:453484
TEST=Run core2md with the test data attached to chromium:453484.
R=mark@chromium.org

Committed: https://code.google.com/p/google-breakpad/source/detail?r=1420

Patch Set 1 #

Total comments: 9

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/client/linux/minidump_writer/linux_core_dumper.cc View 2 chunks +3 lines, -1 line 0 comments Download
M src/client/linux/minidump_writer/linux_core_dumper.h View 1 chunk +3 lines, -2 lines 0 comments Download
M src/client/linux/minidump_writer/linux_dumper.h View 1 chunk +2 lines, -2 lines 0 comments Download
M src/client/linux/minidump_writer/linux_ptrace_dumper.cc View 2 chunks +2 lines, -1 line 0 comments Download
M src/client/linux/minidump_writer/linux_ptrace_dumper.h View 1 chunk +2 lines, -2 lines 0 comments Download
M src/client/linux/minidump_writer/minidump_writer.cc View 1 2 4 chunks +19 lines, -8 lines 0 comments Download

Messages

Total messages: 10
Ben Chan
9 years, 1 month ago #1
Mark Mentovai
https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc File src/client/linux/minidump_writer/minidump_writer.cc (right): https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc#newcode687 src/client/linux/minidump_writer/minidump_writer.cc:687: if (!dumper_->CopyFromProcess(&dyn, GetCrashThread(), dynamic+i++, This looks weird enough as ...
9 years, 1 month ago #2
Ben Chan
https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc File src/client/linux/minidump_writer/minidump_writer.cc (right): https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc#newcode687 src/client/linux/minidump_writer/minidump_writer.cc:687: if (!dumper_->CopyFromProcess(&dyn, GetCrashThread(), dynamic+i++, On 2015/02/02 17:19:06, Mark Mentovai ...
9 years, 1 month ago #3
Ben Chan
On 2015/02/02 18:01:23, Ben Chan wrote: > https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc > File src/client/linux/minidump_writer/minidump_writer.cc (right): > > https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc#newcode687 ...
9 years, 1 month ago #4
Mark Mentovai
https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc File src/client/linux/minidump_writer/minidump_writer.cc (right): https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc#newcode687 src/client/linux/minidump_writer/minidump_writer.cc:687: if (!dumper_->CopyFromProcess(&dyn, GetCrashThread(), dynamic+i++, Ben Chan wrote: > On ...
9 years, 1 month ago #5
Ben Chan
https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc File src/client/linux/minidump_writer/minidump_writer.cc (right): https://breakpad.appspot.com/4724002/diff/1/src/client/linux/minidump_writer/minidump_writer.cc#newcode715 src/client/linux/minidump_writer/minidump_writer.cc:715: break; On 2015/02/02 22:23:01, Mark Mentovai wrote: > On ...
9 years, 1 month ago #6
Mark Mentovai
Then how about return false, which is generally how this function signals failure?
9 years, 1 month ago #7
Ben Chan
On 2015/02/02 22:30:58, Mark Mentovai wrote: > Then how about return false, which is generally ...
9 years, 1 month ago #8
Mark Mentovai
LGTM
9 years, 1 month ago #9
Ben Chan
9 years, 1 month ago #10
Message was sent while issue was closed.
Committed patchset #3 (id:100007) manually as r1420 (presubmit successful).
Sign in to reply to this message.

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