From owner-svn-src-all@FreeBSD.ORG Tue Jan 18 19:30:57 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1BF94106566B; Tue, 18 Jan 2011 19:30:57 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail01.syd.optusnet.com.au (mail01.syd.optusnet.com.au [211.29.132.182]) by mx1.freebsd.org (Postfix) with ESMTP id A6E8E8FC17; Tue, 18 Jan 2011 19:30:55 +0000 (UTC) Received: from c122-106-165-206.carlnfd1.nsw.optusnet.com.au (c122-106-165-206.carlnfd1.nsw.optusnet.com.au [122.106.165.206]) by mail01.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id p0IJUqVB029691 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 19 Jan 2011 06:30:53 +1100 Date: Wed, 19 Jan 2011 06:30:52 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Roman Divacky In-Reply-To: <20110118182655.GA77102@freebsd.org> Message-ID: <20110119060513.P2927@besplex.bde.org> References: <201101181523.p0IFNGeB042079@svn.freebsd.org> <20110119035030.W2099@besplex.bde.org> <201101181215.46266.jhb@freebsd.org> <20110119044805.Y2631@besplex.bde.org> <20110118182655.GA77102@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, John Baldwin , Bruce Evans Subject: Re: svn commit: r217538 - in head/sys/dev: buslogic cs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 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, 18 Jan 2011 19:30:57 -0000 On Tue, 18 Jan 2011, Roman Divacky wrote: > On Wed, Jan 19, 2011 at 04:54:34AM +1100, Bruce Evans wrote: >> On Tue, 18 Jan 2011, John Baldwin wrote: >>> ... >>> Gah, I trusted the clang warning too much. enum's are ints in C yes? So >>> clang has a bug if it thinks an enum value cannot be negative. In practice >>> all the callers of this routine do not pass in negative values, but the >>> compiler shouldn't warn about enum's bogusly. >> >> I didn't know it was a problem already when I asked. I thought >> isa_compat_io_t was global, but it is private to bt. >> >>> Is clang assuming that only defined values for an enum are ever passed in? >>> If >>> so we probably don't want to turn that checking on for the kernel as we >>> violate that in lots of places. >> >> enum values are int, but an enum type may be implemented as unsigned if >> that makes no difference for conforming code. I think conforming code can >> only used declared enum values. Thus if all declared values are >= 0, as >> is the case here, then the enum type may be implemented as unsigned. >> Apparently, this happens for clang here, and it checks what it would do >> itself. Other compilers might do it differently. > > No, it's a real bug - C99 says that: > > If an int can represent all values of the original type, the value is converted > to an int; otherwise, it is converted to an unsigned int. " (C99 6.3.1.1p2). Er, 6.3.1.1p2 just describes the binary promotions in a expression, and this clause describes the unary promotion part of that. It has little to do with enums. It says for example than u_char promotes to int provided UCHAR_MAX <= INT_MAX. enum types start as signed, so they stay signed in promotions Here are more details for enums: - 6.4.4.3p2 paraphrased: enum constants have type int. gcc with certain warnings warns about ones which are outside the range of int. - 6.7.2.1p4 paraphrased: an enum type is any integer type that can represent all the enum constants for the enum. The choice is otherwise implementation- defined. > Thus in this case the enum must be (signed) int. > > The problem is that this bug is in gcc too, compare: > > witten ~# cat enum1.c > #include > > static enum { foo, bar = 1U } z; No, this only has to hold values 0 and 1, so it can be any standard type and perhaps even an implementation-defined 1-bit unsigned type (not 1 bit signed signed since that can only hold 0 and -1). The U on 1U has no effect. > > int main (void) > { > int r = 0; > > if (z - 1 < 0) > r++; > > printf("r = %i\n", r); > } > > witten ~# gcc enum1.c && ./a.out > r = 0 > witten ~# g++ enum1.c && ./a.out > r = 1 > > So clang is just emulating gcc bug which results in this unfortunate bogus warning. This tells us that gcc chose to use an unsigned type for the enum. z - 1 is an integer expression (after promoting z), so it is valid. The result also tells us that gcc wasted space for the enum type, since if it were 1-bit unsigned, uint8_t or uint16_t then it would promote to int and z - 1 would be (int)-1. The bug is that since the choice is implementation-defined, you can't base portable error checking on the choice made. Bruce