Add SymbolicConstantsMach and its test

Add SymbolicConstantsMach and its test.

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

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/e49510ab7ce3b24736979d38d84b7ddce8eaea05

[Mark Mentovai, 2014-09-12 21:12:17]

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

[Mark Mentovai, 2014-09-12 21:12:18]

I had been saving this because it’s a little mind-numbing—the test especially.
But once this is in, we’ll be at the 80% mark.

[Robert Sesek, 2014-09-15 23:00:17]

Well I'm glad that's over!

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
File util/mach/symbolic_constants_mach.cc (right):

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach.cc:132: {THREAD_STATE_FLAVOR_LIST_10_9,
"THREAD_STATE_FLAVOR_LIST_10_9"},
Wtf is this. Good job, Apple… polluting the kernel with product version
numbering.

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach.cc:135: std::string
ThreadStateFlavorFullToShort(const base::StringPiece& flavor) {
This function warrants a comment.

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach.cc:317: for (exception_type_t index = 0;
I do wonder if you could write a (templatized?) function to handle this loop and
return an index when something matches an input in an array.

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
File util/mach/symbolic_constants_mach_test.cc (right):

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach_test.cc:30: // If expect is NULL, the
conversion is expected to fail. If expect is empty,
Use |expect| so that it's clear you're talking about the variable, since this
isn't a docstring comment with \a?

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach_test.cc:288: {EXC_MASK_GUARD,
"EXC_MASK_GUARD", "GUARD"},
Will this compile on 10.6?

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach_test.cc:450: #define NUL_TEST_DATA(string) {
string, arraysize(string) - 1 }
Promote this to a file-level #define ?

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach_test.cc:605: const
StringToSymbolicConstantOptions kOptions[] = {
Share this table across tests where they're the same?

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach_test.cc:864: (flavor <= 7 || flavor == 9 ||
(flavor >= 14 && flavor <= 17))
Why use magic numbers here?

[Mark Mentovai, 2014-09-16 13:42:17]

By all means, please just spot-recheck this. Don’t under any circumstances do a
full re-review.

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
File util/mach/symbolic_constants_mach.cc (right):

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach.cc:132: {THREAD_STATE_FLAVOR_LIST_10_9,
"THREAD_STATE_FLAVOR_LIST_10_9"},
rsesek wrote:
> Wtf is this. Good job, Apple… polluting the kernel with product version
> numbering.

It’s garbage, too. Look at the way that thread_get_state() deals with the
different kinds of flavor lists (osfmk/i386/pcb.c). They’re all exactly the same
on x86, except one has room for three flavors, one has room for four, and one
has room for five. Instead of introducing a new flavor list flavor each time
they needed another slot, they should have made thread_get_state() fill in as
many slots as the caller indicated were available via the |count| parameter.

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach.cc:317: for (exception_type_t index = 0;
rsesek wrote:
> I do wonder if you could write a (templatized?) function to handle this loop
and
> return an index when something matches an input in an array.

I tried to make this work for a while, but gave up because I thought it was more
complicated. Most loops are subtly different enough that this would have to wind
up with extra traits parameters and the templatized function would need to
accept extra arguments. I don’t think it would be an improvement.

For example, most of the k*Names[] arrays store short names, but kFlavorNames
stores long names. And flavors also need to deal with those non-contiguous extra
flavor list flavors.

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
File util/mach/symbolic_constants_mach_test.cc (right):

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach_test.cc:288: {EXC_MASK_GUARD,
"EXC_MASK_GUARD", "GUARD"},
rsesek wrote:
> Will this compile on 10.6?

Yup, compat/mac/mach/mach.h.

https://codereview.chromium.org/563383002/diff/1/util/mach/symbolic_constants...
util/mach/symbolic_constants_mach_test.cc:864: (flavor <= 7 || flavor == 9 ||
(flavor >= 14 && flavor <= 17))
rsesek wrote:
> Why use magic numbers here?

I changed it to use the constants, but I’m not sure that it captures the “WTF is
wrong with the way these were allocated?” nature of the original code, so I also
added a comment.

[Robert Sesek, 2014-09-16 14:34:54]

LGTM

[Mark Mentovai, 2014-09-16 15:00:55]

Committed patchset #3 (id:40001) manually as e49510a (presubmit successful).

[Mark Mentovai, 2014-09-16 15:01:03]

Glad that one’s done.