cvs commit: src/sys/vm vm_map.c

[Bruce Evans]

On Mon, 28 Jun 2004, Andrew Gallatin wrote:

> Brian Fundakowski Feldman writes:
>  > On Mon, Jun 28, 2004 at 03:22:33PM -0400, Andrew Gallatin wrote:
>  > > Andrew Gallatin [gallatin at FreeBSD.org] wrote:
>  > > > gallatin    2004-06-28 19:15:40 UTC
>  > > >
>  > > >   FreeBSD src repository
>  > > >
>  > > >   Modified files:
>  > > >     sys/vm               vm_map.c
>  > > >   Log:
>  > > >   Fix alpha - the use of min() on longs was loosing the high bits and
>  > > >   returning wrong answers, leading to strange values vm2->vm_{s,t,d}size.

MIN() and MAX() should not be used in the kernel.  4.4BSD removed them in
the kernel, but FreeBSD broke this in rev.1.141 of sys/param.h.  They
remain removed in RELENG_4.

>  > > Why are min() and max() inlines which operate on ints?  This seems
>  > > like a real landmine for 64-bit platforms..

They actually operate on unsigned ints.  This is because they are
declared to do so.  C doesn't have any overloaded or type-generic
functions, so min()/max() must operate on some type, and that type
happens to be unsigned int.  Another common error is sign extension
bugs from using min() and max() where imin() or imax() should be used,
or vice versa.

The min()/max() family is hard to use, especially on typedefed types.
MIN()/MAX() is more type-generic, but has side effects and has the
same problems for mixing of signed types with unsigned types.  gcc
with certain warning options now detects sign mismatches in comparisions,
but the implicit casts in the function versions lose the sign information
before the comparisions are done.

>  > Also, why is GCC not generating the correct warnings?  The values passed
>  > in were definitely a 64-bit type.  Thanks for finding and fixing this.
>
> I wish I knew.   I'm afraid this may bite us at some other point?

gcc -Wconversion reports both size and sign mismatches:

%%%
Script started on Tue Jun 29 12:05:20 2004
ttyv6:bde at gamplex:/tmp> cat z.c
#define _KERNEL
#include <sys/types.h>
#include <sys/systm.h>

long long x;
int i;

int
main()
{
	return (min(x, i));
}
ttyv6:bde at gamplex:/tmp> cc -Wconversion -c z.c
z.c: In function `main':
z.c:11: warning: passing arg 1 of `min' with different width due to prototype
z.c:11: warning: passing arg 2 of `min' as unsigned due to prototype
ttyv6:bde at gamplex:/tmp> exit

Script done on Tue Jun 29 12:05:30 2004
%%%

but -Wconversion is not used by default since it would be too noisy.  It
also complains about harmless conversions like promoting 32 bit ints to
64-bit ints.

>  > The inlines seem to exist to work around the effect of using macros
>  > unknowingly on statements with side effects.  These should really be
>  > MIN(), and there seems to have been an extra tab that crept in.  Do
>  > you think you could change those things?
>
> Sure.  Already done.  Thanks for the blessing to use MIN().

MIN() should really not be used.

I once started to fix this by using sizeof() and __typeof() to decide
the correct version of the min()/max() family to call, and comparing
the choice with programmers' choices by examing the object files.
__typeof() can be used even more simply to write safe versions of MIN()
and MAX(), but it shouldn't be used because it is a gccism.  Part of
the unfinished code is:

%%%
#define __min(x, y)							\
(									\
	(sizeof(x) == 8 || sizeof(y) == 8) ?				\
		((__typeof(x))-1 == -1 && (__typeof(y))-1 == -1) ?	\
			_qmin((x), (y))					\
		:							\
			_uqmin((x), (y))				\
	:								\
		((__typeof(x))-1 == -1 && (__typeof(y))-1 == -1) ?	\
			_imin((x), (y))					\
		:							\
			_min((x), (y))					\
)
%%%

(where _min(), etc. are inline functions identical to the current
_min(), etc., and all current min functions are macros expanding to
__min(...)) There was little interest in finishing this.  I only fixed
about 10 serious type mismatches discovered by using it.  Things are
more complicated now with multiple arches and more typdefed types, but
the hard-coded 8 in the above still works for all supported arches.
It depends mainly on ints being smaller than quads and longs having
the same size as either ints or quads.

Bruce