Date: Mon, 11 Mar 2019 20:18:01 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Justin Hibbits <chmeeedalf@gmail.com> Cc: Alexey Dokuchaev <danfe@freebsd.org>, Justin Hibbits <jhibbits@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r344960 - head/sys/powerpc/powerpc Message-ID: <20190311191740.J2090@besplex.bde.org> In-Reply-To: <20190310171640.31bb9c54@titan.knownspace> References: <201903090318.x293IcLc023548@repo.freebsd.org> <20190309085058.GA60945@FreeBSD.org> <20190310171640.31bb9c54@titan.knownspace>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 10 Mar 2019, Justin Hibbits wrote:
> On Sat, 9 Mar 2019 08:50:58 +0000
> Alexey Dokuchaev <danfe@freebsd.org> wrote:
>
>> On Sat, Mar 09, 2019 at 03:18:38AM +0000, Justin Hibbits wrote:
>>> ...
>>> Log:
>>> powerpc: Print trap frame address for fatal traps
>>> ...
>>> Modified: head/sys/powerpc/powerpc/trap.c
>>> ==============================================================================
>>> --- head/sys/powerpc/powerpc/trap.c Sat Mar 9 03:15:09
>>> 2019 (r344959) +++ head/sys/powerpc/powerpc/trap.c
>>> Sat Mar 9 03:18:37 2019 (r344960) @@ -546,6 +546,7 @@
>>> printtrap(u_int vector, struct trapframe *frame, int i printf("
>>> current msr = 0x%" PRIxPTR "\n", mfmsr()); printf("
>>> lr = 0x%" PRIxPTR " (0x%" PRIxPTR ")\n", frame->lr,
>>> frame->lr - (register_t)(__startkernel - KERNBASE));
No one wrote that as stated above. Some mail program mangled it.
>>> + printf(" frame = %p\n", frame);
>>> printf(" curthread = %p\n", curthread);
>>
>> Some of those are printed with %PRIxPTR, some with %p. Perhaps this
>> inconsistency can be avoided?
>> ...
>
> PRIxPTR is for printing a pointer-like value that's not a pointer. The
> other values printed are of type register_t, not void *.
PRI*any is for writing style bugs in the source and sometimes in the output.
Sometimes also type errors. It should never be used.
The above (not the 1 line in this commit, but nearby) demonstrates common
errors in using it:
- the format given by PRIany is not fully specified. It may contain a
lebgth modifier. Thus it is unusable in carefully formatted output
where you would want to add your own length modifier. It also
requires knowing too much about this feature to know that adding an
0x prefix or perhaps a '#', '0' or ' ' modifier doesn't mess up the
format.
This is not much of a problem here. The lengths are only accidentally
the same. Maybe they are even the same as for %p format.
- first type error: PRIxPTR only has defined behaviour for printing variables
of type uintptr_t, but mfmsr() has type register_t. The type mismatch is
not only logical, but physical (register_t is signed). Correctness depends
on register_t having the same size as uintptr_t and the treatment of signed
values as unsigned not being too magic (I think it is implementation-defined
but not undefined; however, printing uintptr_t using PRTdPTR gives undefined
behaviour on values not representable as intptr_t.
It requires knowing too much to know that the above use of PRIxPTR
is correct enough. As usual, it is easier to not use PRIany. Just
cast to uintmax_t and use %jx. This is much shorter and clearer after
correcting the above by adding a cast to uintptr_t.
Using uintmax_t has the minor problem that it is wasteful for 32-bit
args and will become wasteful for 64-bit args and more wasteful for
32-bit args when uintmax_t becomes 128, 256, ... bits.
- second type error: %p only has defined behaviour for printing variables
of type void *, but curthread has type struct thread *.
This can be fixed by casting to void *, or fixed better by casting to
uintmax_t and using %jx. More details below.
- poor formatting from %p. %p is guaranteed to bad for formatted output.
It is specified to give an (any) implementation-defined sequence of
printing characters. To use it except for low quality debugging
output, not quite as above (the above attempts medium quality, with
some alignment of fields but no attention to field widths for number
values), you first have to know what the implementation defines,
then don't use it when it is unsuitable. It is easiest to never use
it. In FreeBSD, printf(3) documents its format as being as if it
is %#x or %#lx. This gives no control over the field width. Its
format is different and undocumented for printf(9), but not as bad
-- field widths and maybe other things now work for %p. The 0x
prefix usually given by '#' is still unavoidable.
Conversion of pointers to uintmax_t or intmax_t gives full control
over the format, just like for converted integer types. This is not
quite easier and clearer for pointers. 3 casts are needed to go from
an arbitrary pointer to a uintmax_t. First to const volatile void *
(not to plain void *, since that gives cast-qual warnings if the
pointer is const or volatile). Then to uintptr_t. Then to uintmax_t.
In theory, conversion of a pointer to an integer using these casts may
change its representation, and we should use a different method to see the
original bits. %p is no good for this of course -- it might convert
to an integer using casts too, and its its format is poorly documented.
To see the original bits, memcpy() the pointer to an array of bytes and
print the bytes. In practice, conversion using casts preserves all the
bits, and the only difference between printing the integer and printing
the bytes is that printf() has better support for the former and the
output may be more difficult to understand even if it is in hex, due
to endianness differences.
ps (in both the kernel and userland IIRC) has some support for printing
kernel addresses in a compressed format with a fixed number of leading
0 and/or f nybbles suppressed. This is most needed and also most broken
for 64-bit addresses. On some arches, all kernel addresses are in the
top half of the address space, so plain %[lx] format gives the same
field width for all kernel addresses, but on 64-bit arches this is 16
bytes wide and tends to break formatting in other ways.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190311191740.J2090>