svn commit: r285168 - head/sys/powerpc/powerpc

[Bruce Evans]

On Sun, 5 Jul 2015, Justin Hibbits wrote:

> On Jul 5, 2015 08:30, "Bjoern A. Zeeb" <bz at freebsd.org> wrote:
>>
>> Author: bz
>> Date: Sun Jul  5 15:30:16 2015
>> New Revision: 285168
>> URL: https://svnweb.freebsd.org/changeset/base/285168
>>
>> Log:
>>   Fix GENERIC64 and LINT64 powerpc builds after r285144.
>>
>> Modified:
>>   head/sys/powerpc/powerpc/trap.c
>>
>> Modified: head/sys/powerpc/powerpc/trap.c
>>
> ==============================================================================
>> --- head/sys/powerpc/powerpc/trap.c     Sun Jul  5 14:24:35 2015
> (r285167)
>> +++ head/sys/powerpc/powerpc/trap.c     Sun Jul  5 15:30:16 2015
> (r285168)
>> @@ -426,10 +426,10 @@ printtrap(u_int vector, struct trapframe
>>                 ver = mfpvr() >> 16;
>>  #if defined(AIM)
>>                 if (MPC745X_P(ver))
>> -                       printf("    msssr0         = 0x%x\n",
>> +                       printf("    msssr0         = 0x%\n",
>>                             mfspr(SPR_MSSSR0));
>>  #elif defined(BOOKE)
>> -               printf("    mcsr           = 0x%x\n",
>> +               printf("    mcsr           = 0x%" PRIxPTR "\n",
>>                     mfspr(SPR_MCSR));
>>  #endif
>>                 break;
>>
>
> Sigh, thanks. Since those are 32 bit registers I only tested my 32 bit
> compiles.

This change introduces a style bug and just moves the type error from
a harmful one to a harmless one.

It uses the PRI* mistake.  powerpc only used this mistake 8 times before
(1 PRIxPTR in mmu_oea64.c and 7 PRIxPTR's in trap.c).

The type in the mistake doesn't even match the type of the arg.  PRIxPTR
is for printing uintptr_t, but the arg type is register_t.  These are
logically very differemt, but happen to be the same physically (both are
32 bits or both 64 bits, according to LP32 or LP64).

All of the old uses are wrong too.  The printf format is always uintptr_t,
but:
- in mmu_oea64.c, the arg type is vm_paddr_t.  This follows the LP32/LP64
   switch too
- in trap.c, the 7 uses are all to print things in the stack frame.  The
   arg type is always register_t.

amd64 and i386 never use the PRI* mistake.  They are simpler because the
LP32/LP64 switch is at the arch level.  i386 would have complications
printing things like vm_paddr_t that depend on PAE, but the PRI* mistake
is useless for handling such complications, since it has no support for
the vm_paddr_t.  Similarly for almost all types.  register_t too.

arm uses the PRI* mistake just 2 times.  Both uses are just silly.  PRIu32 
is used to print a type that isn't even a uint32_t, but the binary
promotion of a uint32_t (in the expression raw / 1000, where `raw' has
type uint32_t).  This is just an obfuscation, except on systems with
ints smaller than 32 bits where it is a micro-optimization for speed.

But POSIX now requires ints to be at least 32 bits.  So the binary
promotion of uint32_t is almost known.  It it is uint32_t if ints are
32 bits, else int.  Both cases can be handled by either %u or %d format.
Strictly, %u should be used if ints are 32 bits, else %d should be used.
But since `raw' <= UINT32_MAX, raw / 1000 < INT_MAX, so %d always works
too.  %u always works too because raw / 1000 is always nonnegative.
Perhaps PRIu32 works right in this case.  Args are subject to unary
promotions, and this will change the type from uint32_t to int iff ints
are larger than 32 bits, just like for the binary promotion.  PRIu32
must be %d and not %u then, unless it can be shown that both %d and %u
work and that the compiler doesn't warn about the harmless sign mismatch.

If ints are less than 32 bits, then the binary and unary promotions of
uint32_t are uint32_t again, and PRIu32 works right.  To print a uint32_t
without using PRI*, there is nothing better than casting it to u_long.
This works well, and should be done in MI code that supports arches with
ints less than 32 bits.  But arm is MD, and only supports 32-bit ints.
It shouldn't handle these complications.

Since arm only supports 32-bit ints, it should above these complications
by declaring 'raw' as plain u_int instead of uint32_t.  (raw is only used
in a local table that uses int for everything else.  Making it unsigned
is probably wrong too.)

arm64, mips, sparc64 and kern never use the PRI* mistake.

Printing register_t in MD code is very easy.  Just cast to u_long and use
%lx format.  This handles the LP32/LP64 switch correctly in all cases.  In
the LP64 case, everything is really long or u_long and the cast only fixes
the sign error than register_t is a signed type on most arches.  In the
LP32 case, all types are really 32 bits.  All should be u_int, but
register_t is usually signed int, and perhaps some broken types are
long or u_long.  The cast now fixes the sign error and tells the compiler
to not compain about the logical type mismatch between %lx format and
u_int args, so that the format can be %lx for both LP32 and LP64 case;
the cast acts only on logical types so it doesn't cost anything at runtime.

Printing register_t in MI code is not so easy.  MI code shouldn't assume
either LP32 or LP64, so it should cast to uintmax_t, but that has runtime
costs if register_t is smaller than uintmax_t.  The only excuse for PRI*'s
existence is to avoid these runtime costs.  But it is useless except for
the standard types that it supports.

The bug in the original version was that it didn't cast to u_long.  amd64
and i386 can just omit such casts and hard-code the format as %lx for
amd64 and %x for i386 (except, compilers should warn about the mismatch
between the signed type register_t and the unsigned format).  I can only
find powerpc using this method in 1 printf (in cpu.c).

Bruce