Support object files larger than 2**32.

Support object files larger than 2**32.

This CL adds a new class SuperFatArch, which is very similar to struct fat_arch,
but allows for object files larger than 2**32.

[erikchen]

mark: Please review.

[Mark Mentovai]

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm
File src/common/mac/dump_syms.mm (right):

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:210: SuperFatArch
*DumpSymbols::FindBestMatchForArchitecture(cpu_type_t cpu_type, cpu_subtype_t
cpu_subtype) {
* on the type name, and keep it within 80.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:213: for (vector<SuperFatArch>::iterator it =
object_files_.begin();
const_iterator? (I’m assuming you didn’t use a C++11 range-based for because who
knows what kind of non-C++11 things this needs to be compatible with.)

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:227: for (vector<SuperFatArch>::iterator it =
object_files_.begin();
Rather than doing this as 2-pass, why not populate fat_arch_vector in the loop
above?

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:261: exit(1);
What’s this about? You do handle the NULL return where this is called.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:264: 
Extra blank line.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch.h
File src/common/mac/super_fat_arch.h (right):

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch...
src/common/mac/super_fat_arch.h:50: SuperFatArch(const struct fat_arch &arch) :
explicit

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch...
src/common/mac/super_fat_arch.h:58: bool CanConvertToFatArch() {
If you’re going with the single-pass loop as recommended in the other file,
CanConvertToFatArch() and ConvertToFatArch() [cs]hould be combined into a single
bool ConvertToFatArch(fat_arch*).

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch...
src/common/mac/super_fat_arch.h:59: return offset < UINT32_MAX && size <
UINT32_MAX && align < UINT32_MAX;
These should be <=, not <. Use <limits> and std::numeric_limits<>::max() instead
of the <stdint.h> macros. Since you’ll be using numeric_limits<>, you can use
decltype(arch.{offset,size,align}) as the template parameter, or typeof instead
of decltype for pre-C++11 GCC/clang.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch...
src/common/mac/super_fat_arch.h:63: assert(offset < UINT32_MAX);
These asserts will not remain in a combined function, but if they do remain for
any reason, IWYU: <assert.h>.

[erikchen]

mark: PTAL

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm
File src/common/mac/dump_syms.mm (right):

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:210: SuperFatArch
*DumpSymbols::FindBestMatchForArchitecture(cpu_type_t cpu_type, cpu_subtype_t
cpu_subtype) {
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> * on the type name, and keep it within 80.

Done.

On the "*" placement, I was copying from the header file:
"""
const struct fat_arch *AvailableArchitectures(size_t *count)
"""
I went ahead and fixed that as well.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:213: for (vector<SuperFatArch>::iterator it =
object_files_.begin();
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> const_iterator? (I’m assuming you didn’t use a C++11 range-based for because
who
> knows what kind of non-C++11 things this needs to be compatible with.)

Done.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:227: for (vector<SuperFatArch>::iterator it =
object_files_.begin();
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> Rather than doing this as 2-pass, why not populate fat_arch_vector in the loop
> above?

Done.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:261: exit(1);
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> What’s this about? You do handle the NULL return where this is called.

You're right. Removed the call to exit().

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/dump_syms.mm#n...
src/common/mac/dump_syms.mm:264: 
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> Extra blank line.

Done.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch.h
File src/common/mac/super_fat_arch.h (right):

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch...
src/common/mac/super_fat_arch.h:50: SuperFatArch(const struct fat_arch &arch) :
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> explicit

Done.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch...
src/common/mac/super_fat_arch.h:58: bool CanConvertToFatArch() {
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> If you’re going with the single-pass loop as recommended in the other file,
> CanConvertToFatArch() and ConvertToFatArch() [cs]hould be combined into a
single
> bool ConvertToFatArch(fat_arch*).

Done.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch...
src/common/mac/super_fat_arch.h:59: return offset < UINT32_MAX && size <
UINT32_MAX && align < UINT32_MAX;
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> These should be <=, not <. Use <limits> and std::numeric_limits<>::max()
instead
> of the <stdint.h> macros. Since you’ll be using numeric_limits<>, you can use
> decltype(arch.{offset,size,align}) as the template parameter, or typeof
instead
> of decltype for pre-C++11 GCC/clang.

I switched to std::numeric_limits. I used uint32_t as the template parameter, as
I'm avoiding C++11, and "typeof" is not part of the C++ standard. It's in GCC
and Boost. "typeid" is a runtime check, which I wanted to avoid.

https://breakpad.appspot.com/7834002/diff/20001/src/common/mac/super_fat_arch...
src/common/mac/super_fat_arch.h:63: assert(offset < UINT32_MAX);
On 2015/04/21 21:37:17, Mark Mentovai wrote:
> These asserts will not remain in a combined function, but if they do remain
for
> any reason, IWYU: <assert.h>.

Removed asserts.

[Mark Mentovai]

LGTM

https://breakpad.appspot.com/7834002/diff/90015/common/mac/dump_syms.h
File common/mac/dump_syms.h (right):

https://breakpad.appspot.com/7834002/diff/90015/common/mac/dump_syms.h#newcod...
common/mac/dump_syms.h:180: vector<SuperFatArch> object_files_;
Add an explicit () initializer to the constructor.

https://breakpad.appspot.com/7834002/diff/90015/common/mac/super_fat_arch.h
File common/mac/super_fat_arch.h (right):

https://breakpad.appspot.com/7834002/diff/90015/common/mac/super_fat_arch.h#n...
common/mac/super_fat_arch.h:44: uint32_t cputype;
<stdint.h> for this.

https://breakpad.appspot.com/7834002/diff/90015/common/mac/super_fat_arch.h#n...
common/mac/super_fat_arch.h:52: explicit SuperFatArch(const struct fat_arch
&arch) :
<mach-o/fat.h> for this.

[erikchen]

https://breakpad.appspot.com/7834002/diff/90015/common/mac/dump_syms.h
File common/mac/dump_syms.h (right):

https://breakpad.appspot.com/7834002/diff/90015/common/mac/dump_syms.h#newcod...
common/mac/dump_syms.h:180: vector<SuperFatArch> object_files_;
On 2015/04/22 04:17:54, Mark Mentovai wrote:
> Add an explicit () initializer to the constructor.

I think you're asking for the default constructor to initialize all members. Is
that correct?

[Mark Mentovai]

https://breakpad.appspot.com/7834002/diff/90015/common/mac/dump_syms.h
File common/mac/dump_syms.h (right):

https://breakpad.appspot.com/7834002/diff/90015/common/mac/dump_syms.h#newcod...
common/mac/dump_syms.h:180: vector<SuperFatArch> object_files_;
On 2015/04/22 18:11:27, erikchen wrote:
> On 2015/04/22 04:17:54, Mark Mentovai wrote:
> > Add an explicit () initializer to the constructor.
> 
> I think you're asking for the default constructor to initialize all members.
Is
> that correct?

Yes, it looks like this is the only member that it doesn’t initialize
explicitly.

[erikchen]

https://breakpad.appspot.com/7834002/diff/90015/common/mac/dump_syms.h
File common/mac/dump_syms.h (right):

https://breakpad.appspot.com/7834002/diff/90015/common/mac/dump_syms.h#newcod...
common/mac/dump_syms.h:180: vector<SuperFatArch> object_files_;
On 2015/04/22 18:21:30, Mark Mentovai wrote:
> On 2015/04/22 18:11:27, erikchen wrote:
> > On 2015/04/22 04:17:54, Mark Mentovai wrote:
> > > Add an explicit () initializer to the constructor.
> > 
> > I think you're asking for the default constructor to initialize all members.
> Is
> > that correct?
> 
> Yes, it looks like this is the only member that it doesn’t initialize
> explicitly.

Done.

https://breakpad.appspot.com/7834002/diff/90015/common/mac/super_fat_arch.h
File common/mac/super_fat_arch.h (right):

https://breakpad.appspot.com/7834002/diff/90015/common/mac/super_fat_arch.h#n...
common/mac/super_fat_arch.h:44: uint32_t cputype;
On 2015/04/22 04:17:54, Mark Mentovai wrote:
> <stdint.h> for this.

Done.

https://breakpad.appspot.com/7834002/diff/90015/common/mac/super_fat_arch.h#n...
common/mac/super_fat_arch.h:52: explicit SuperFatArch(const struct fat_arch
&arch) :
On 2015/04/22 04:17:54, Mark Mentovai wrote:
> <mach-o/fat.h> for this.

Done.

[Mark Mentovai]

LGTM

https://breakpad.appspot.com/7834002/diff/270001/common/mac/dump_syms.mm
File common/mac/dump_syms.mm (right):

https://breakpad.appspot.com/7834002/diff/270001/common/mac/dump_syms.mm#newc...
common/mac/dump_syms.mm:43: #include <assert.h>
Move this above to the C system headers section.

[erikchen]

https://breakpad.appspot.com/7834002/diff/270001/common/mac/dump_syms.mm
File common/mac/dump_syms.mm (right):

https://breakpad.appspot.com/7834002/diff/270001/common/mac/dump_syms.mm#newc...
common/mac/dump_syms.mm:43: #include <assert.h>
Done

[erikchen]

On 2015/04/22 18:31:47, erikchen wrote:
> https://breakpad.appspot.com/7834002/diff/270001/common/mac/dump_syms.mm
> File common/mac/dump_syms.mm (right):
> 
>
https://breakpad.appspot.com/7834002/diff/270001/common/mac/dump_syms.mm#newc...
> common/mac/dump_syms.mm:43: #include <assert.h>
> Done

Manually committed as r1453.