Update dump_syms to correctly filter on cpu subtype.

Update dump_syms to correctly filter on cpu subtype.

 Right now, if an archive contain multiple executable for the same CPU but with
different subtype, there is no way to dump any but the first one.
Committed: https://code.google.com/p/google-breakpad/source/detail?r=1061

[Mark Mentovai]

https://breakpad.appspot.com/476002/diff/2001/src/client/mac/handler/minidump...
File src/client/mac/handler/minidump_generator.cc (right):

https://breakpad.appspot.com/476002/diff/2001/src/client/mac/handler/minidump...
src/client/mac/handler/minidump_generator.cc:1307: result =
macho.UUIDCommand(cpu_type, 0, identifier);
Instead of 0, can you use some sort of symbolic constant? Something predefined,
even?

Each CPU family has its own _ALL whose value seems to always be 0, but it’s not
appropriate to use any architecture-specific type in this
architecture-independent code, so that would favor defining your own constant.
There’s also the possibility of using CPU_SUBTYPE_MULTIPLE, but that has value
-1 instead of 0. See <mach/machine.h>.

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/file_id.cc
File src/common/mac/file_id.cc (right):

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/file_id.cc#newco...
src/common/mac/file_id.cc:74: int cpu_subtype,
I’m surprised to see this being typed as int instead of the more descriptive
cpu_subtype_t.

But, then, I’m surprised to see that cpu_type_t isn’t being used for cpu_type.

Easily fixable?

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/macho_walker.cc
File src/common/mac/macho_walker.cc (right):

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/macho_walker.cc#...
src/common/mac/macho_walker.cc:85: // if |cpu_type| is 0, use the native cpu
type.
In this case, it looks like CPU_TYPE_ANY is the system-predefined macro for this
kind of thing, but it has value -1.

Also, you might want to note somewhere (like in the header) that cpu_subtype is
ignored if cpu_type is whatever “wildcard” value is in use.

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/macho_walker.cc#...
src/common/mac/macho_walker.cc:90: valid_cpu_subtype = arch->cpusubtype;
This changes the meaning. Formerly, if cpu_type was “wildcard,” you’d set
cpu_type to the native CPU type, and effectively leave cpu_subtype as
“wildcard.” Now, you’re setting cpu_subtype to the native CPU subtype. I would
guess that this behavior change would be detectable and cause undesirable
effects.

[qsr]

https://breakpad.appspot.com/476002/diff/2001/src/client/mac/handler/minidump...
File src/client/mac/handler/minidump_generator.cc (right):

https://breakpad.appspot.com/476002/diff/2001/src/client/mac/handler/minidump...
src/client/mac/handler/minidump_generator.cc:1307: result =
macho.UUIDCommand(cpu_type, 0, identifier);
 Used CPU_SUBTYPE_MULTIPLE, this is looking almost exactly like what we want.

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/file_id.cc
File src/common/mac/file_id.cc (right):

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/file_id.cc#newco...
src/common/mac/file_id.cc:74: int cpu_subtype,
 Trying.

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/macho_walker.cc
File src/common/mac/macho_walker.cc (right):

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/macho_walker.cc#...
src/common/mac/macho_walker.cc:85: // if |cpu_type| is 0, use the native cpu
type.
On 2012/10/03 17:57:49, Mark Mentovai wrote:
> In this case, it looks like CPU_TYPE_ANY is the system-predefined macro for
this
> kind of thing, but it has value -1.

 I agree with you, but prefer not to change that here as this was already like
that, and I do not want to introduce side-effects as I'm not sure to undertand
when 0 is used.

> 
> Also, you might want to note somewhere (like in the header) that cpu_subtype
is
> ignored if cpu_type is whatever “wildcard” value is in use.

https://breakpad.appspot.com/476002/diff/2001/src/common/mac/macho_walker.cc#...
src/common/mac/macho_walker.cc:90: valid_cpu_subtype = arch->cpusubtype;
 Well, all depend on what we mean by current cpu. But I agree on the side effect
part of this, will use the wildcard instead.

[Mark Mentovai]

LGTM otherwise

https://breakpad.appspot.com/476002/diff/4003/src/common/mac/macho_walker.cc
File src/common/mac/macho_walker.cc (right):

https://breakpad.appspot.com/476002/diff/4003/src/common/mac/macho_walker.cc#...
src/common/mac/macho_walker.cc:83: bool MachoWalker::WalkHeader(int cpu_type,
int cpu_subtype) {
You didn’t fix the types in this class?

[qsr]

https://breakpad.appspot.com/476002/diff/4003/src/common/mac/macho_walker.cc
File src/common/mac/macho_walker.cc (right):

https://breakpad.appspot.com/476002/diff/4003/src/common/mac/macho_walker.cc#...
src/common/mac/macho_walker.cc:83: bool MachoWalker::WalkHeader(int cpu_type,
int cpu_subtype) {
 Missed it. Fixed.