From owner-svn-src-all@FreeBSD.ORG Tue May 13 05:32:40 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C8F28EBC; Tue, 13 May 2014 05:32:40 +0000 (UTC) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id 89B912D78; Tue, 13 May 2014 05:32:40 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 0CAC4783F63; Tue, 13 May 2014 15:32:32 +1000 (EST) Date: Tue, 13 May 2014 15:32:31 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andrey Chernov Subject: Re: svn commit: r265901 - stable/10/sys/kern In-Reply-To: <5371081A.2070703@freebsd.org> Message-ID: <20140513150452.R2958@besplex.bde.org> References: <201405120427.s4C4RAZf093033@svn.freebsd.org> <5370F110.5050502@freebsd.org> <20140512170322.GA2479@stack.nl> <5371081A.2070703@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=QIpRGG7L c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=syaZHqrdBKwA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=i1eFTGOA1oohQBZ4AMsA:9 a=g5t_JHQO4ZTDTcVu:21 a=hJ3xz1m3iuSP41rW:21 a=CjuIK1q_8ugA:10 Cc: src-committers@freebsd.org, svn-src-stable-10@freebsd.org, Jilles Tjoelker , svn-src-stable@freebsd.org, Don Lewis , svn-src-all@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 May 2014 05:32:40 -0000 On Mon, 12 May 2014, Andrey Chernov wrote: > On 12.05.2014 21:03, Jilles Tjoelker wrote: >> On Mon, May 12, 2014 at 08:04:32PM +0400, Andrey Chernov wrote: >>> On 12.05.2014 8:27, Don Lewis wrote: >>>> + if (start + amask < start) { >>>> + DPRINTF(("start+amask wrapped around\n")); >>>> + goto out; >>>> + } >> >>> Checking for overflow _after_ it happens is unportable and dangerous, >>> since wrapping around is not only one possible result. They should be >>> rewritten like that: >> >>> if (start > ULONG_MAX - amask) >> >> Unsigned types wrap around per the C standard. Overflow checking after >> it happens is fine. >> >> You are right for signed types. > > You are right. The C Standard, 6.2.5, paragraph 9 [ISO/IEC 9899:2011], > states: Actually, there is a problem with promotions. The rman code is correct, since all the types are hard-coded as u_long (even correctly spelled as that), so we know that the promotions don't expand them. (Expansion would allow start + amask to hold a larger value, and would also change the type to signed.) Similar code in vm is technically broken, since the types are are usually vm_offset_t and that can be anything in theory. It might be u_char or u_short. Both normally promote to int. POSIX requires u_char to be 8 bits, so it is always too small for vm_offset_t, but u_short can be large enough (e.g., u_short 64 bits, u_int 65 bits, u_long 130 bits, u_long_long 260 bits, uintmax_t 520 bits for a machine with a 64-bit address space but 65-bit registers :-). > "A computation involving unsigned operands can never overflow, because a > result that cannot be represented by the resulting unsigned integer type > is reduced modulo the number that is one greater than the largest value > that can be represented by the resulting type." This is bad wording, since the "resulting type" isn't always unsigned unless "unsigned operands" is interpreted as "operands, which are unsigned after promotion". Similar code in vm: % int % sys_mmap(td, uap) % ... % vm_offset_t addr; % ... % vm_size_t size, pageoff; % ... % if (addr + size < addr) % return (EINVAL); Here addr and size have logically different types, so they might have quite different promotitions (largest wins). Typedefs are too hard for me. Rewriting this using (__maxof(__typeof(foo)) - foo) (where foo is one of the operands in the addition) works well here too. Bruce