Fix dump generation on 64-bit Mac OS X

This patch fixes a few issues in the minidump generation code on Mac OS X
x86-64. With this patch + Jim's patch queue, I'm able to produce a minidump on
64-bit OS X, dump the symbols, and walk the stack successfully.

Mark, can you take a look?

[Mark Mentovai]

http://breakpad.appspot.com/103001/diff/1/6
File src/client/mac/Breakpad.xcodeproj/project.pbxproj (right):

http://breakpad.appspot.com/103001/diff/1/6#newcode1323
Line 1323: ONLY_ACTIVE_ARCH_PRE_XCODE_3_1 = "$(NATIVE_ARCH_ACTUAL)";
?

http://breakpad.appspot.com/103001/diff/1/7
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/103001/diff/1/7#newcode53
Line 53: #define LC_SEGMENT_COMMAND		LC_SEGMENT_64
Why so much horizontal whitespace?

http://breakpad.appspot.com/103001/diff/1/7#newcode55
Line 55: #define LC_SEGMENT_COMMAND		LC_SEGMENT
LC_SEGMENT_COMMAND = “load command segment command.” I suggest LC_SEGMENT_ARCH,
which is a little more clear about it being the right segment load command for
this architecture.

http://breakpad.appspot.com/103001/diff/1/7#newcode56
Line 56: #endif
#else #error unknown CPU?

http://breakpad.appspot.com/103001/diff/1/7#newcode623
Line 623: // hw.cputype is always I386 even on an x86-64 system
Whitespace again.

http://breakpad.appspot.com/103001/diff/1/7#newcode642
Line 642: "movl %%ebx,%1 \n\t"            \
You don’t need this line. Just specify "=b" (ebx) for the second constraint and
omit the mov. The only actual asm opcode that belongs in this macro is “cpuid.”
You don’t have to worry about %ebx’ unavailability in 64-bit mode.

As written, this cpuid macro has a serious bug: you haven’t told gcc that you’re
using %ebx for output, but you clobber %ebx on this line. That’s bad, because
gcc will assume that %ebx is unchanged before and after the asm block. I got
away with trashing %ebx in the 32-bit version above by pushing and popping it.

http://breakpad.appspot.com/103001/diff/1/7#newcode644
Line 644: "=g" (ebx),                   \
This line looks screwed-up too, and on the one before, the backslash isn’t
aligned. Maybe you’re working with weird tab settings? Could you please take a
look and fix these throughout? It’s hard to read with the indentation jumping
all over the place. Follow the example of the other asm cpuid block above.

[Ted Mielczarek]

Thanks for the review! A few comments here, I'll get a cleaned up patch for you.

http://breakpad.appspot.com/103001/diff/1/6
File src/client/mac/Breakpad.xcodeproj/project.pbxproj (right):

http://breakpad.appspot.com/103001/diff/1/6#newcode1323
Line 1323: ONLY_ACTIVE_ARCH_PRE_XCODE_3_1 = "$(NATIVE_ARCH_ACTUAL)";
On 2010/04/29 04:48:06, Mark Mentovai wrote:
> ?

Beats me, I'm using XCode 3.2.2, might be something stupid it did. I can
manually revert this if you want.

http://breakpad.appspot.com/103001/diff/1/7
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/103001/diff/1/7#newcode55
Line 55: #define LC_SEGMENT_COMMAND		LC_SEGMENT
On 2010/04/29 04:48:06, Mark Mentovai wrote:
> LC_SEGMENT_COMMAND = “load command segment command.” I suggest
LC_SEGMENT_ARCH,
> which is a little more clear about it being the right segment load command for
> this architecture.

This (and the previous indentation thing) are due to my copy/pasting this from
the dyld source:
http://www.opensource.apple.com/source/dyld/dyld-132.13/src/dyldInitializatio...

http://breakpad.appspot.com/103001/diff/1/7#newcode56
Line 56: #endif
On 2010/04/29 04:48:06, Mark Mentovai wrote:
> #else #error unknown CPU?

The second one isn't an elif, it's either LP64 or not, there's not a third
option.

http://breakpad.appspot.com/103001/diff/1/7#newcode623
Line 623: // hw.cputype is always I386 even on an x86-64 system
On 2010/04/29 04:48:06, Mark Mentovai wrote:
> Whitespace again.

I did most of this editing in XCode, and didn't realize it defaults to tabs
until I was almost done. I thought I got it all, I'll clean up the rest.

http://breakpad.appspot.com/103001/diff/1/7#newcode642
Line 642: "movl %%ebx,%1 \n\t"            \
On 2010/04/29 04:48:06, Mark Mentovai wrote:
> You don’t need this line. Just specify "=b" (ebx) for the second constraint
and
> omit the mov. The only actual asm opcode that belongs in this macro is
“cpuid.”
> You don’t have to worry about %ebx’ unavailability in 64-bit mode.
> 
> As written, this cpuid macro has a serious bug: you haven’t told gcc that
you’re
> using %ebx for output, but you clobber %ebx on this line. That’s bad, because
> gcc will assume that %ebx is unchanged before and after the asm block. I got
> away with trashing %ebx in the 32-bit version above by pushing and popping it.

Sorry, my inline assembly skills are not very good! I'll fix that.

[Mark Mentovai]

http://breakpad.appspot.com/103001/diff/1/7
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/103001/diff/1/7#newcode622
Line 622: case CPU_TYPE_I386:
Also put case CPU_TYPE_X86_64 here in case hw.cputype changes to report that on
x86-64?

[Ted Mielczarek]

Updated with mark's comments

[Mark Mentovai]

LGTM

http://breakpad.appspot.com/103001/diff/12001/13006
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/103001/diff/12001/13006#newcode648
Line 648: #endif
Here is where you might consider placing an #error.