svn commit: r325728 - head/lib/libkvm
brde at optusnet.com.au
Fri Feb 8 08:43:23 UTC 2019
On Wed, 6 Feb 2019, Will Andrews wrote:
> On Tue, Feb 5, 2019 at 10:25 AM Bruce Evans <brde at optusnet.com.au> wrote:
>> Signed kp_offset seems wrong. Apart from it not reaching the top of 64-
>> bit address spaces, adding unsigned kp_len to it gives an unsigned type.
> It's appropriate, because in this context, we return page information
> including addresses that would be valid pointer references, but are not
> included in the core file. Minidumps omit large numbers of physical pages,
> so calls to kvm_walk_pages() will generate large numbers of kvm_page
> instances that have an offset of -1.
off_t was already used internally for the return type of _kvm_pt_find().
.offset is initalized to _kvm_pt_find() using especially large style
bugs (most of _kvm_visit_cb() is written in an initalizer).
off_t is very inappropriate for small offsets from C pointers. ptrdiff_t
would be right for that, except C99 mis-specify by only requiring it to
hold up to +-65535, so on perverse implementations with ptrdiff_t = int17_t,
subtraction of pointers into an object with more than 65535 elements gives
undefined behaviour. In practice, ptrdiff_t is not perversely implemented
but system code like vm can't use it because it only covers half the address
kvm should uses its own type for this so as to not have to worry about
signedness problems or the bloat of off_t or the unportability of ptrdiff_t.
I rather like APIs which abuse the sign bit for an out of band error
value, but this is not very appropriate. time_t is such an API, if
it is broken to POSIX spec (not opaque) and is signed (because buggy
code expects that). But only bad code compares with -1. The error
value is (time_t)-1, as sometimes needed in implementations where
time_t is unsigned. Not quite similarly for mmap(). Its error value
is MMAP_FAILED which is (void *)-1 in FreeBSD. Both of these values
may be in band. -1 is 1 before the Epoch in a time_t. POSIX doesn;t
require this to work, but some libraries support it. C99 and POSIX
are missing the specifications of errno necessary to detect if an
in-band error value is an error for these functions and most others.
off_t is signed, so an uncast -1 works right as an error value for
lseek(). This is not a feature. -1 is in band for lseek() on devices
on 64-bit arches on FreeBSD (this is a POSIX extension. IIRC, POSIX
allows anything for devices).
The magic -1's for time_t and mmap() are at least documented. struct
kvm_page and its member 'offset' and kvm_walk_pages() are undocumented.
I think time_t and mmap() were misdesigned originally but were improved
a little by specifying a cast or a macro. MMAP_FAILED can't be just
(void *)-1 on systems where the compiler objects to this case.
More information about the svn-src-all