[mac] Teach dump_syms to handle additional zerofill sections

[mac] Teach dump_syms to handle additional zerofill sections

This patch allows dump_syms to handle S_THREAD_LOCAL_ZEROFILL
and S_GB_ZEROFILL section in the same way as the more common
S_ZEROFILL section.  Previously, dump_syms would fail to dump
a binary containing a __DATA,__thread_bss section, because it
tried to look up its data (and failed).

R=mark@chromium.org

Committed: https://chromium.googlesource.com/breakpad/breakpad/+/cc7b69a8f29d67e2eba25e1658df54dd69ec1e54

[labath, 2015-09-28 13:07:01]

labath@google.com changed reviewers:
+ erikchen@chromium.org, ted.mielczarek@gmail.com

[erikchen, 2015-09-28 17:28:04]

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

[erikchen, 2015-09-28 17:28:04]

+mark, who is a much better reviewer for this code.

[Mark Mentovai, 2015-09-28 17:59:20]

https://codereview.chromium.org/1369233003/diff/1/src/common/mac/macho_reader.cc
File src/common/mac/macho_reader.cc (right):

https://codereview.chromium.org/1369233003/diff/1/src/common/mac/macho_reader...
src/common/mac/macho_reader.cc:1: // Copyright (c) 2010, Google Inc.
In the CL description:

https://www.google.com/#q=%22.thread_bss%22

Nobody knows what .thread_bss is. The section name is __DATA,__therad_bss.

https://codereview.chromium.org/1369233003/diff/1/src/common/mac/macho_reader...
src/common/mac/macho_reader.cc:484: if ((section.flags & SECTION_TYPE) ==
S_ZEROFILL || (section.flags & SECTION_TYPE) == S_THREAD_LOCAL_ZEROFILL) {
Don’t go past 80 columns. Break the line after the ||. Alternatively, this is
also two lines and may be slightly more readable:

  const uint32_t section_type = section.flags & SECTION_TYPE;
  if (section_type == S_ZEROFILL || section_type == S_THREAD_LOCAL_ZEROFILL) {

You may also want to include S_GB_ZEROFILL here.

Reference:
https://chromium.googlesource.com/crashpad/crashpad/+/master/snapshot/mac/mac...

[labath, 2015-09-29 08:50:14]

I've updated the commit message, added S_GB_ZEROFILL and reformatted the code.
Please take another look.

[Mark Mentovai, 2015-09-29 12:55:12]

LG otherwise

https://codereview.chromium.org/1369233003/diff/1/src/common/mac/macho_reader.cc
File src/common/mac/macho_reader.cc (right):

https://codereview.chromium.org/1369233003/diff/1/src/common/mac/macho_reader...
src/common/mac/macho_reader.cc:1: // Copyright (c) 2010, Google Inc.
Mark Mentovai - August is over wrote:
> In the CL description:
> 
> https://www.google.com/#q=%22.thread_bss%22
> 
> Nobody knows what .thread_bss is. The section name is __DATA,__therad_bss.

The CL description still mentions .thread_bss.

You can edit this with “git cl message” or in the Rietveld web UI by clicking
“Edit Issue”. Local git commit messages don’t influence the CL description once
the CL has been created. The CL description is the only thing that will be used
as the true commit message once committed in the shared repository.

[labath, 2015-09-29 13:04:18]

description updated

[Mark Mentovai, 2015-09-29 13:12:27]

LGTM

[labath, 2015-09-29 13:33:13]

The CQ bit was checked by labath@google.com

[labath, 2015-09-29 13:39:13]

The CQ bit was checked by labath@google.com

[labath, 2015-09-30 10:58:28]

I am unsure how I am supposed to commit this. I tried following the generic
chrome instructions, but this project does not seem to have a commit queue.

Could you help me out here?

[Mark Mentovai, 2015-09-30 12:20:48]

Committed patchset #2 (id:20001) manually as
cc7b69a8f29d67e2eba25e1658df54dd69ec1e54 (presubmit successful).