svn commit: r293979 - head/lib/libkvm

Bruce Evans brde at optusnet.com.au
Thu Jan 14 17:35:01 UTC 2016


On Thu, 14 Jan 2016, John Baldwin wrote:

> Log:
>  Fix building with GCC since PAGE_MASK is signed on i386.
> ...

> Modified: head/lib/libkvm/kvm_i386.h
> ==============================================================================
> --- head/lib/libkvm/kvm_i386.h	Thu Jan 14 15:50:13 2016	(r293978)
> +++ head/lib/libkvm/kvm_i386.h	Thu Jan 14 15:51:13 2016	(r293979)
> @@ -70,7 +70,7 @@ _Static_assert(NBPDR == I386_NBPDR, "NBP
>
> _Static_assert(PG_V == I386_PG_V, "PG_V mismatch");
> _Static_assert(PG_PS == I386_PG_PS, "PG_PS mismatch");
> -_Static_assert(PG_FRAME == I386_PG_FRAME, "PG_FRAME mismatch");
> +_Static_assert((u_int)PG_FRAME == I386_PG_FRAME, "PG_FRAME mismatch");
> _Static_assert(PG_PS_FRAME == I386_PG_PS_FRAME, "PG_PS_FRAME mismatch");
> #endif

This breaks the warning.  PG_FRAME still has the fragile type int and
the fragile value -4096.

libkvm has lots of nearby style bugs like using the long long abomination
as a suffix for the literal constants, and bogus parentheses around
single literals.  All of these are copied from the kernel (except their
ames are changed by adding an I386_ prefix).  The assertions aren't very
useful when half of the constants use lexically identically definitions
with equal ugliness.  The 2 PAE constants are the main ones that aren't
asserted to be equal.

I think PAE in the kernel could even use the signedness of PG_FRAME.
(vm_paddr_t)-4096 first sign extends to (int64_t) with value -4096
and then overflows to the correct mask of 0xfffffffffffff000 with the
correct type.  So -4096 could work for both PAE and !PAE.  But PAE
actually uses an ifdef to to define PG_FRAME as (0x000ffffffffff000ull).
This has the 2 style bugs mentioned above, and seems to have a nonsense
value.  I think PAE is only 36 bits, but the mask is for 52 bits.

I don't like explicitly typed constants, but this problem shows that
some care is needed for using constants in expressions.  int is correct
for PAGE_SIZE but not masks, except for masks of all except the sign
bit it is OK.

Bruce


More information about the svn-src-head mailing list