_stp_vsnprintf() broken

[Quentin Barnes]

>On Sat, 2007-05-12 at 12:46 -0500, Quentin Barnes wrote:
>> In my ARM port, I noticed "_stp_printf("%p ", pc);" calls were
>> outputting trashed pointer values rotated and truncated.  I tracked
>> the problem down to _stp_vsnprintf().  The case for 'p' was doing
>> a va_arg(args, int64_t). 
>
>That is actually correct. I implemented %p this way because systemtap
>only supports strings, 64-bit ints, and stats. So pointers internally
>are always cast to int64_t. We can't simply use %lx to print them
>because that would result in things like ffffffffc0400e16 on 32-bit
>machines.

I'm not sure I follow here.  Do you mean %llx for 32-bit machines?

Either way, %lx and %llx print out unsigned values, so there would be
no sign extension.

>So use of %p is correct in tapset functions and systemtap scripts.
>Unfortunately it started getting used in runtime C code (notably the
>stack functions) to print pointers, which coincidentally works on some
>architectures.

So are you saying that virtually all the places in runtime/* that's using
"%p" is just broken?

There are 21 places where _stp_printf is passed a "%p" in the
runtime environment.  Not a single instance is casting the parameter
to int64_t.  The places were it is being explicitly casted in common
code or 32-bit architecture-specific code (stack-i386.c, stack.c,
sym.c), it is being casted to a pointer type (which is 32-bit for
32-bit platforms).

What is the correct fix here?  Should I cast my pointers to int64_t
before passing them to _stp_printf %p?  Or should I be doing something
else and not use %p on 32-bit architectures?

The REG_* macros in runtime/regs.h aren't casting their return
values to int64_t.  Should these be fixed too?

>So I think the thing to do is clearly document this limitation of %p,
>review all %p usage in the runtime and fix as appropriate. 
>
>I'll clean this up ASAP. Thanks for reporting the problem.
>
>Martin
>

Quentin