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

Issue 7684002: Add support for shared libraries in archives. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years ago by anton
Modified:
7 years ago
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/src
Visibility:
Public.

Description

Chrome on Android now supports loading the shared library directly from the APK
file.

This patch makes two changes to breakpad to enable crash reporting to work
correctly when the library is inside another file (an archive):

- Do not filter mappings which map an executable at a non-zero offset.
- If such an executable is mapped look in the ELF information for the 
shared object name and use that name in the minidump.

Note this change doesn't care about the archive format and isn't Android
specific (though loading the shared library this way is currently only done on
Android).

BUG=390618

Patch Set 1 #

Patch Set 2 : Remove whitespace change. #

Patch Set 3 : Use the Elf SONAME as the library name #

Patch Set 4 : Fix long line #

Total comments: 21

Patch Set 5 : Changes for review #

Total comments: 10

Patch Set 6 : Changes for review #4 #

Patch Set 7 : Fixes for tests and type mismatches #

Patch Set 8 : Added unittest for mapping with offset #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M client/linux/minidump_writer/linux_core_dumper.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M client/linux/minidump_writer/linux_dumper.cc View 1 2 3 4 5 6 5 chunks +83 lines, -4 lines 0 comments Download
M client/linux/minidump_writer/linux_dumper.h View 1 2 3 4 2 chunks +8 lines, -0 lines 0 comments Download
M client/linux/minidump_writer/minidump_writer.cc View 1 2 3 4 5 6 4 chunks +31 lines, -4 lines 0 comments Download
M common/linux/elf_core_dump_unittest.cc View 1 2 3 4 5 6 3 chunks +11 lines, -10 lines 0 comments Download
M common/linux/file_id.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M common/linux/memory_mapped_file.cc View 1 2 3 4 5 6 2 chunks +18 lines, -10 lines 0 comments Download
M common/linux/memory_mapped_file.h View 1 2 3 4 5 3 chunks +3 lines, -2 lines 0 comments Download
M common/linux/memory_mapped_file_unittest.cc View 1 2 3 4 5 6 7 6 chunks +44 lines, -10 lines 0 comments Download
M tools/linux/md2core/minidump-2-core.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 11
anton
7 years ago #1
Lei Zhang (chromium)
https://breakpad.appspot.com/7684002/diff/90001/client/linux/minidump_writer/linux_dumper.cc File client/linux/minidump_writer/linux_dumper.cc (right): https://breakpad.appspot.com/7684002/diff/90001/client/linux/minidump_writer/linux_dumper.cc#newcode141 client/linux/minidump_writer/linux_dumper.cc:141: bool LinuxDumper::ElfFileSoNameFromMappedFile( nit: add a "// static" comment https://breakpad.appspot.com/7684002/diff/90001/client/linux/minidump_writer/linux_dumper.cc#newcode165 ...
7 years ago #2
anton
https://breakpad.appspot.com/7684002/diff/90001/client/linux/minidump_writer/linux_dumper.cc File client/linux/minidump_writer/linux_dumper.cc (right): https://breakpad.appspot.com/7684002/diff/90001/client/linux/minidump_writer/linux_dumper.cc#newcode141 client/linux/minidump_writer/linux_dumper.cc:141: bool LinuxDumper::ElfFileSoNameFromMappedFile( On 2014/07/16 23:28:25, Lei Zhang (chromium) wrote: ...
7 years ago #3
Lei Zhang (chromium)
Generally looking good. In the CL description, please change BUG= to chromium:NNN. Can you add ...
7 years ago #4
Lei Zhang (chromium)
I recommend getting a real Breakpad checkout and running make && make check. Several unit ...
7 years ago #5
anton
On 2014/07/17 17:45:40, Lei Zhang (chromium) wrote: > I recommend getting a real Breakpad checkout ...
7 years ago #6
anton
https://breakpad.appspot.com/7684002/diff/160001/client/linux/minidump_writer/linux_dumper.cc File client/linux/minidump_writer/linux_dumper.cc (right): https://breakpad.appspot.com/7684002/diff/160001/client/linux/minidump_writer/linux_dumper.cc#newcode172 client/linux/minidump_writer/linux_dumper.cc:172: if (dyn->d_un.d_val >= dynstr_size) { On 2014/07/17 17:45:41, Lei ...
7 years ago #7
Lei Zhang (chromium)
See https://code.google.com/p/google-breakpad/source/checkout for how to get the source for Breakpad proper. https://breakpad.appspot.com/7684002/diff/160001/client/linux/minidump_writer/linux_dumper.cc File client/linux/minidump_writer/linux_dumper.cc (right): ...
7 years ago #8
anton
On 2014/07/18 20:29:00, Lei Zhang (chromium) wrote: > See https://code.google.com/p/google-breakpad/source/checkout for how to get the ...
7 years ago #9
Lei Zhang (chromium)
lgtm
7 years ago #10
rmcilroy
7 years ago #11
On 2014/07/21 21:13:54, Lei Zhang (chromium) wrote:
> lgtm

Landed as r1355
Sign in to reply to this message.

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