svn commit: r278634 - head/lib/libc/gen

[Bruce Evans]

On Fri, 13 Feb 2015, Andrey Chernov wrote:

> We even don't need to check arg excepting for < 0, because what is
> needed is rlimt_t and not arg. So this version will be better:
>
> rlimt_t targ;
>
> if (arg < 0) {
>    errno = EINVAL;
>    return (-1);
> }


This is reasonable, but not encouraged by the API or compatible with
what setrlimit() does with negative args.  (setrlimit() still uses
my hack from 1994, of converting negative args to RLIM_INFINITY.  In
4.4BSD, it doesn't even check for negative args, and mostly stores
them unchanged; then undefined behaviour tends to occur when the
stored values are used without further checking.)

In POSIX, rlim_t is an unsigned integer type, so negative args are
impossible for it and all negative args here are unrepresentable as
an rlim_t.  We can convert them to any garbage.  The historical
garbage is as good as anything.

> targ = arg;

Unnecessary overflow before checking in the theoretical case where
rlim_t is smaller than long.

> if (targ > RLIM_INFINITY / 512)
>    targ = RLIM_INFINITY / 512;

When overflow doesn't occur, using targ instead of arg in the comparison
does nothing except possibly confuse the compiler into not generating
a tautologous-compare warning.

Assigning RLIM_INFINITY to an rlim_t instead of to a long is one way to
way to fix the overflow case for the latter,

> limit.rlim_max = limit.rlim_cur = targ * 512

However, I don't like using rlim_t for the scaled value that is not
an rlimit.

An incomplete fix with handling of negative values restored is something
like:

 	intmax_t targ;

 	targ = arg;
 	if (targ > RLIM_INFINITY / 512)
 		targ = RLIM_INFINITY / 512;
 	limit.rlim_max = limit.rlim_cur = targ * 512

This is still incomplete.  The comparison is still obviously tautologous
when intmax_t == rlim_t (the amd64 case).  If intmax_t is larger than
long (the i386 case) or even rlim_t (the notyet case), then it is slightly
less obviously tautologous.  This can be fixed by sprinkling volatiles,
e.g. for targ.

Range checking in expr(1) needed much more delicate fixes including
sprinkling volatiles.  There the problem was that the compiler saw
that the behaviour was either undefined or tautologously true or false.
Optimization then allows it to remove tautologous checks.  The warnings
about tautologous checks must have been turned off, or perhaps broken
when undefined behaviour is required for the tautologies, else we
should have seen the warnings before the optimizations.  The warnings
are needed even more with the optimizations, to give a hint that
necessary code is being removed.

Bruce