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

Issue 93001: Breakpad Mac symbol dumper: Add new Mach-O reader class. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 5 months ago by jimb
Modified:
9 years, 2 months ago
CC:
google-breakpad-dev_googlegroups.com
Visibility:
Public.

Description

This patch adds files defining new classes in the google_breakpad::Mach_O
namespace for parsing fat binaries and Mach-O files. These are used in the
new dumper to handle STABS debugging information, DWARF call frame
information, and .eh_frame exception handling stack walking information.

These new classes are independent of endianness and word size, and
therefore can be used on binaries of all the relevant architectures: x86,
x86_64, ppc, and ARM.

The patch adds a complete set of unit tests for the new classes.

Patch Set 1 #

Patch Set 2 : Breakpad Mac symbol dumper: Add new Mach-O reader class. #

Total comments: 163
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
A src/common/mac/macho_reader.cc View 1 1 chunk +517 lines, -0 lines 71 comments Download
A src/common/mac/macho_reader.h View 1 1 chunk +438 lines, -0 lines 60 comments Download
A src/common/mac/macho_reader_unittest.cc View 1 1 chunk +1760 lines, -0 lines 6 comments Download
M src/common/test_assembler.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/tools/mac/dump_syms/dump_syms.xcodeproj/project.pbxproj View 6 chunks +220 lines, -26 lines 0 comments Download
A src/tools/mac/dump_syms/macho_dump.cc View 1 chunk +191 lines, -0 lines 26 comments Download

Messages

Total messages: 17
jimb
Here's the right version of this patch to review. Sorry for the confusion.
9 years, 5 months ago #1
jimb
Add mark as reviewer.
9 years, 4 months ago #2
Mark Mentovai
Longer, more interesting change. I’ll get to it later.
9 years, 4 months ago #3
jimb
Adding Ted as reviewer.
9 years, 4 months ago #4
jimb
Patch set 2 has only comment changes.
9 years, 4 months ago #5
jimb
On 2010/05/20 19:29:33, jimb wrote: > Patch set 2 has only comment changes. Actually, that's ...
9 years, 4 months ago #6
Mark Mentovai
OK, let’s get started here. I like what I’ve read so far. http://breakpad.appspot.com/93001/diff/6001/7001 File src/common/mac/macho_reader.cc ...
9 years, 3 months ago #7
Mark Mentovai
I’m really enjoying reading this. Nice work. http://breakpad.appspot.com/93001/diff/6001/7001 File src/common/mac/macho_reader.cc (right): http://breakpad.appspot.com/93001/diff/6001/7001#newcode61 Line 61: << ...
9 years, 3 months ago #8
Mark Mentovai
I didn’t read the test file as carefully, other than to poke into a bunch ...
9 years, 3 months ago #9
Mark Mentovai
http://breakpad.appspot.com/93001/diff/6001/7006 File src/tools/mac/dump_syms/macho_dump.cc (right): http://breakpad.appspot.com/93001/diff/6001/7006#newcode35 Line 35: extern "C" { Unnecessary. http://breakpad.appspot.com/93001/diff/6001/7006#newcode41 Line 41: #include ...
9 years, 3 months ago #10
jimb
Thanks very much for the review! http://breakpad.appspot.com/93001/diff/6001/7001 File src/common/mac/macho_reader.cc (right): http://breakpad.appspot.com/93001/diff/6001/7001#newcode40 Line 40: #include "common/mac/macho_reader.h" ...
9 years, 3 months ago #11
Mark Mentovai
http://breakpad.appspot.com/93001/diff/6001/7001 File src/common/mac/macho_reader.cc (right): http://breakpad.appspot.com/93001/diff/6001/7001#newcode85 Line 85: if (cursor >> magic_) { jimb wrote: > ...
9 years, 3 months ago #12
jimb
http://breakpad.appspot.com/93001/diff/6001/7001 File src/common/mac/macho_reader.cc (right): http://breakpad.appspot.com/93001/diff/6001/7001#newcode197 Line 197: << "the contents of load command #" << ...
9 years, 3 months ago #13
Mark Mentovai
http://breakpad.appspot.com/93001/diff/6001/7001 File src/common/mac/macho_reader.cc (right): http://breakpad.appspot.com/93001/diff/6001/7001#newcode276 Line 276: // What's that thing Ruby programmers like to ...
9 years, 3 months ago #14
jimb
http://breakpad.appspot.com/93001/diff/6001/7003 File src/common/mac/macho_reader_unittest.cc (right): http://breakpad.appspot.com/93001/diff/6001/7003#newcode211 Line 211: .Append(10 * 5 * 4, '*'); // dummy ...
9 years, 3 months ago #15
Mark Mentovai
http://breakpad.appspot.com/93001/diff/6001/7003 File src/common/mac/macho_reader_unittest.cc (right): http://breakpad.appspot.com/93001/diff/6001/7003#newcode211 Line 211: .Append(10 * 5 * 4, '*'); // dummy ...
9 years, 3 months ago #16
Ted Mielczarek
9 years, 2 months ago #17

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