From owner-svn-src-head@FreeBSD.ORG Fri Feb 13 07:19:09 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id F23D4C25; Fri, 13 Feb 2015 07:19:08 +0000 (UTC) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id B220532B; Fri, 13 Feb 2015 07:19:08 +0000 (UTC) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id CAA87D63AE1; Fri, 13 Feb 2015 18:18:55 +1100 (AEDT) Date: Fri, 13 Feb 2015 18:18:55 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Bjoern A. Zeeb" Subject: Re: svn commit: r278634 - head/lib/libc/gen In-Reply-To: <9A683D99-C1E9-4736-982C-69F583D3A40D@FreeBSD.org> Message-ID: <20150213172738.C1007@besplex.bde.org> References: <201502122107.t1CL7gaO004041@svn.freebsd.org> <54DD2A87.2050008@FreeBSD.org> <9A683D99-C1E9-4736-982C-69F583D3A40D@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=baJSDo/B c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=6I5d2MoRAAAA:8 a=2IwoPmx0YhHHISiYWswA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Pedro Giffuni , src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Feb 2015 07:19:09 -0000 On Thu, 12 Feb 2015, Bjoern A. Zeeb wrote: >> On 12 Feb 2015, at 22:34 , Pedro Giffuni wrote: >> >> >> On 02/12/15 17:27, Bjoern A. Zeeb wrote: >>>> On 12 Feb 2015, at 21:07 , Pedro F. Giffuni wrote: >>>> ... >>>> Log: >>>> ulimit(3): Fix broken check. >>>> ... >>> Did this compile? >> >> Yes! Any log message to share? > > Now I do again; had lost them due to buildworld starting over again: > > ===> lib/libc_nonshared (obj,depend,all,install) > cc1: warnings being treated as errors > /scratch/tmp/bz/head.svn/lib/libc/gen/ulimit.c: In function 'ulimit': > /scratch/tmp/bz/head.svn/lib/libc/gen/ulimit.c:56: warning: comparison is always false due to limited range of data type > /scratch/tmp/bz/head.svn/lib/libc/gen/ulimit.c:57: warning: overflow in implicit constant conversion > --- ulimit.So --- > *** [ulimit.So] Error code 1 Grump. The first warning inhibits doing clean range checking. Considered the following simplified case: long arg; /* * We want to know if arg is representable as an rlim_t. This is * remarkably difficult. The following doesn't work due to * compiler zeal and other bugs: */ if (arg > RLIM_INFINITY) err(...); The check is false on all arches since RLIM_INFINITY happens to be >= LONG_MAX, and excessively zealous compilers warn. This omits complications for negative args (see below). There are also system bugs for negative args. POSIX specifies that rlim_t shall be an unsigned type, but in FreeBSD it is still a signed type like it always was. FreeBSD is also missing POSIX's magic values RLIM_SAVED_{MAX,CUR} for handling unrepresentable values. After fixing the type, these could be used for the historical mishandling of historical negative values and might even allow binary compatibility for those values. The second warning is about a bug. A last-minute change made the error handling the same for all negative values (representable or not) as for unrepresentable positive values. But that doesn't work, since it give an overflow in live code as well as in dead code. We also depend on the compiler understanding the range checking and not warning for overflows in dead code. The unsimplified code for this is: if (arg > RLIM_INFINITY / 512) arg = RLIM_INFINITY / 512; or equivalently: arg = MIN(arg, RLIM_INFINITY / 512); The check is vacuously false on 64-bit arches only. The assignment overflows on 32-bit arches only. So zealous compilers have something to warn about in both lines. We want a warning for only the second line iff it is live and overflows. This requires the compiler to understand when the first line is vacuously false so that the second line is dead, but not warn about the first line. Other common forms of the range check shouldn't work any better: if ((rlim_t)arg > RLIM_INFINITY) If rlim_t were unsigned as specified by POSIX, then this cast would probably just be a bug. Otherwise, it should have no effect in the vacuously false case. We depend on it having no effect so that the compiler knows when the following code is dead. rlim_t tmp; tmp = arg; if (tmp != arg) err(1, "arg not representable by rlim_t"); The compiler should still be able to see when the comparison is vacuously false. This form of the comparison is also more difficult to write when the limit is not simply the maximum for the type. Here we want to multiply by 512, but there may be no type that can hold the result. This shows that the MIN() macro is hard to used with mixed types and zealous compilers. Using doesn't avoid many of the problems in the imin() family. With this family, you have to translate all typedefed types to basic types and promote to a larger common type, if any. This tends to hide the original values, so the zealous compilers are not smart enough to warn. However, smart ones should see inside: arg = llmin(arg, RLIM_INFINITY / 512); and warn for it too. Add complications for portability to BSD and POSIX for the full mess. Translation of typedefed types for the imin() family becomes impossible because there is no suitable larger common type in some cases; using MIN() gives unwanted (un)sign extension in these cases. Bruce