Date: Fri, 8 Feb 2019 19:43:15 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Will Andrews <will@firepipe.net> Cc: Ed Maste <emaste@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r325728 - head/lib/libkvm Message-ID: <20190208190822.N858@besplex.bde.org> In-Reply-To: <CADBaqmjZsB9iQQgLg%2B_XpzyEoBTgDNAD345ks0FgSi5ROmhM7w@mail.gmail.com> References: <201711112330.vABNUwXC077395@repo.freebsd.org> <CAPyFy2BwNGHkMjj2rG5N5S=7E8N=9jfAUBki1L8eCY3kMeM8fw@mail.gmail.com> <20190205202145.A1080@besplex.bde.org> <CAPyFy2C%2BZ0aOGUBbgpLZ8sJbaN2YhEbR1YkV9Ya7POTSsbLqGQ@mail.gmail.com> <20190206024025.X3175@besplex.bde.org> <CADBaqmjZsB9iQQgLg%2B_XpzyEoBTgDNAD345ks0FgSi5ROmhM7w@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 6 Feb 2019, Will Andrews wrote: > On Tue, Feb 5, 2019 at 10:25 AM Bruce Evans <brde@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 space. 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. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190208190822.N858>