Fix printing of x86_64 registers in minidump_stackwalk

I guess I missed this when I initially implemented it, but we've been printing
the 64-bit x86_64 registers as 32-bit values in minidump_stackwalk, which is
dumb.

[Mark Mentovai]

http://breakpad.appspot.com/262001/diff/1/2
File src/processor/minidump_stackwalk.cc (right):

http://breakpad.appspot.com/262001/diff/1/2#newcode96
Line 96: static int PrintRegister(const char *name, u_int64_t value, int
sequence) {
Can you do this without overloading the name?

http://breakpad.appspot.com/262001/diff/1/2#newcode97
Line 97: if (sequence % 3 == 0) {
Why 3 instead of 4? (Or, alternatively, why 4 instead of 3 above?) It seems that
the thing that would be printed here is exactly as long as the thing that would
be printed in the 32-bit version of this function. Shouldn’t they use the same
value?

[Ted Mielczarek]

http://breakpad.appspot.com/262001/diff/1/2
File src/processor/minidump_stackwalk.cc (right):

http://breakpad.appspot.com/262001/diff/1/2#newcode96
Line 96: static int PrintRegister(const char *name, u_int64_t value, int
sequence) {
On 2011/01/28 19:18:30, Mark Mentovai wrote:
> Can you do this without overloading the name?

I can, this just involved fewer lines of change.

http://breakpad.appspot.com/262001/diff/1/2#newcode97
Line 97: if (sequence % 3 == 0) {
On 2011/01/28 19:18:30, Mark Mentovai wrote:
> Why 3 instead of 4? (Or, alternatively, why 4 instead of 3 above?) It seems
that
> the thing that would be printed here is exactly as long as the thing that
would
> be printed in the 32-bit version of this function. Shouldn’t they use the same
> value?

Because I failed to change the %08 to %016 here, which is what I really want.
The previous impl prints 8-digit 32-bit hex strings, this one prints 16-digit
64-bit hex strings. The 4/3 are arbitrary numbers of "how many can we print
without wrapping at 80 characters".

[Ted Mielczarek]

Having made the %08 -> %016 change, I realize I really wanted 2 as the magic
number there.

[Mark Mentovai]

http://breakpad.appspot.com/262001/diff/6001/7001
File src/processor/minidump_stackwalk.cc (right):

http://breakpad.appspot.com/262001/diff/6001/7001#newcode98
Line 98: if (sequence % 2 == 0) {
Maybe you should leave this at sequence % 4, and…

http://breakpad.appspot.com/262001/diff/6001/7001#newcode103
Line 103: return ++sequence;
make this return sequence + 2.

Would that properly handle cases where 32-bit and 16-bit registers were printed
on the same line?

[Ted Mielczarek]

On 2011/01/28 19:45:28, Mark Mentovai wrote:
> Would that properly handle cases where 32-bit and 16-bit registers were
printed
> on the same line?

No, because you can get it into odd states if you have, say, one 32-bit value
and 2 64-bit values. I can try to make this slightly smarter if you'd like, and
roughly count field lengths instead of just using modulo to print a newline
every N fields, or I can just leave it with what I've got and add a comment
saying that they're not intended to be mixed.

[Mark Mentovai]

Good point. Can you make them mix properly by doing field lengths, then?

[Ted Mielczarek]

Feels a little over-engineered for what it does, but this handles things
properly.

[Mark Mentovai]

LGTM

http://breakpad.appspot.com/262001/diff/13001/14001
File src/processor/minidump_stackwalk.cc (right):

http://breakpad.appspot.com/262001/diff/13001/14001#newcode87
Line 87: static int PrintRegister(const char *name, u_int32_t value, int
sequence) {
These aren’t sequences now. Maybe start_col?

http://breakpad.appspot.com/262001/diff/13001/14001#newcode89
Line 89: sprintf(buffer, " %5s = 0x%08x", name, value);
Use snprintf. Evil callers might provide stupidly long names.