Add MachOImageReader and its test

Add MachOImageReader and its test, which also tests
MachOImageSegmentReader.

TEST=util_test MachOImageReader.*
R=rsesek@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/4f74716f6df9464383db75ab97aa216d08cca6b9

[Mark Mentovai, 2014-09-03 22:22:31]

mark@chromium.org changed reviewers:
+ rsesek@chromium.org

[Robert Sesek, 2014-09-04 13:18:42]

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader.cc
File util/mac/mach_o_image_reader.cc (right):

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.cc:187: kLoadCommandAddressLimit) {
nit: indent +4

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.cc:209: kLoadCommandAddressLimit) {
+4

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.cc:222: if (load_command.cmd ==
kLoadCommandReaders[reader_index].command) {
Flip this to a != and continue instead? Would reduce some nesting.

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.cc:314: *size = segment->vmsize() +
(segment->SegmentSlides() ? 0 : slide_);
This is because if the segment doesn't slide, it grows, right? Is vmsize always
zero in this case?

Recommend leaving a comment here, since this condition took a while to vet.

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.cc:416: segments_.push_back(segment);
"// Takes ownership."

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.cc:431: return false;
But keep it in segments_?

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader.h
File util/mac/mach_o_image_reader.h (right):

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.h:110: //!     memory, taking any “slide” into
account if the segment did not load at
A similar \note about how address may differ from
MachOImageSegmentReader::vmaddr() could be useful.

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.h:162: //! \return A pointer to the section
information. If \a index is out of range,
This is a behavior difference from MachOImageSegmentReader::GetSectionAtIndex().

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.h:198: void UUID(struct UUID* uuid) const;
Why `struct UUID` instead of just `UUID`?

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader.h:221: // |initialization_data| or the member
fields of their MachOImageReader
What's |initialization_data|?

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
File util/mac/mach_o_image_reader_test.cc (right):

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader_test.cc:33: const struct dyld_all_image_infos*
_dyld_get_all_image_infos();
Move this to a shared header? I've seen it before I think.

https://codereview.chromium.org/535343004/diff/1/util/mac/mach_o_image_reader...
util/mac/mach_o_image_reader_test.cc:447: EXPECT_EQ(0, memcmp(&actual_uuid,
&expected_uuid, sizeof(UUID)));
Might make sense to add UUID::operator==.

[Mark Mentovai, 2014-09-04 14:32:01]

Thanks for the review, I know this one was longer than we’ve been aiming for.

[Robert Sesek, 2014-09-04 14:38:17]

LGTM

https://codereview.chromium.org/535343004/diff/40001/util/mac/mach_o_image_re...
File util/mac/mach_o_image_reader.h (right):

https://codereview.chromium.org/535343004/diff/40001/util/mac/mach_o_image_re...
util/mac/mach_o_image_reader.h:262: struct UUID uuid_;
struct UUID here, too.

[Mark Mentovai, 2014-09-04 15:45:47]

Committed patchset #4 (id:60001) manually as 4f74716 (presubmit successful).