Date: Fri, 15 Jul 2016 18:36:27 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Alexey Dokuchaev <danfe@freebsd.org> Cc: Bruce Evans <brde@optusnet.com.au>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, Mark Johnston <markj@freebsd.org>, src-committers@freebsd.org Subject: Re: svn commit: r302854 - head/sys/kern Message-ID: <20160715162551.K4083@besplex.bde.org> In-Reply-To: <20160715043148.GA46325@FreeBSD.org> References: <201607141849.u6EIn53i038324@repo.freebsd.org> <20160715053515.S2290@besplex.bde.org> <20160715071816.Q2585@besplex.bde.org> <20160715043148.GA46325@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 15 Jul 2016, Alexey Dokuchaev wrote: > On Fri, Jul 15, 2016 at 07:19:22AM +1000, Bruce Evans wrote: >> On Fri, 15 Jul 2016, Bruce Evans wrote: >>>> Log: >>>> Let DDB's buf printer handle NULL pointers in the buf page array. >>> >>> I noticed some other bugs in this code: >> >> Oops, that was supposed to be a private reply. > > I'm glad it leaked: %p abuse is unfortunately not that rare, and getting > developers' attention is a good thing. E.g., every time I do kldstat(8) > on my PowerPC box I sigh: > > % kldstat > Id Refs Address Size Name > 1 7 0x100000 e0c378 kernel > 2 1 0xd20dd000 1d000 tmpfs.ko > 3 2 0xd2114000 18000 geom_sched.ko > 4 1 0xd2131000 13000 gsched_rr.ko I think some mail program further broke the formatting -- it omitted leading spaces so nothing lines up. After fixing this, only the narrow Address for Id 1 messes up the alignment. Output on amd64 shows another problem. All kernel pointers are above 0xf000000000000000, so %p prints them all with the same width, but this width is is too wide -- the 'f's provide no useful info. Also, kldstat hard-codes the i386 width of 2+8 in the header, so the Size and Name columns are consistently misaligned on amd64. %p is more usable in the kernel. One of its undocumented features there is that field widths work with it. This is sometimes used to avoid the above bug. Kernel ddb used to use %8p for all pointers. The 8 in this is hard-coded for 32-bit arches, so was wrong in another way. ddb now uses %p and hard-codes the width of this in the header as 2+8 or 2+16 depending on __LP64__ and is more broken than before for small pointer values. The only important table where this is used is in ps. Spaces in this table is wasted to print just 1 pointer -- the relatively useless wchan. I sometimes compress kernel pointers in output and trace buffers by omitting leading 1 or 0 bits and trailing 0 bits as well as leading 0x. User ps is documented to trim even non-redundant leading parts of the address for *wchan (0x80324000 -> 324000 in the example), but the code hasn't matched the man page since 2001. In 4.4BSD-Lite2, this as implemented using &~ KERNBASE. That wasn't portable. The first fix replaced the 32-bit %08lx format. Now the format is %0lx. This moves the bugs much as in ddb ps, except it doesn't use %p and doesn't print the 0x prefix. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160715162551.K4083>