From owner-svn-src-all@FreeBSD.ORG Mon Aug 23 01:46:39 2010 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 52D351065672; Mon, 23 Aug 2010 01:46:39 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail09.syd.optusnet.com.au (mail09.syd.optusnet.com.au [211.29.132.190]) by mx1.freebsd.org (Postfix) with ESMTP id DE2878FC15; Mon, 23 Aug 2010 01:46:38 +0000 (UTC) Received: from c122-107-127-123.carlnfd1.nsw.optusnet.com.au (c122-107-127-123.carlnfd1.nsw.optusnet.com.au [122.107.127.123]) by mail09.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o7N1k6sw027805 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 23 Aug 2010 11:46:09 +1000 Date: Mon, 23 Aug 2010 11:46:06 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Dimitry Andric In-Reply-To: <4C6FDBCF.6070101@andric.com> Message-ID: <20100823111937.D21446@delplex.bde.org> References: <201008191259.o7JCxv3i004613@svn.freebsd.org> <20100820075236.L18914@delplex.bde.org> <4C6DC0F8.9040001@andric.com> <20100821054033.M19850@delplex.bde.org> <4C6FDBCF.6070101@andric.com> 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, Rui Paulo , Bruce Evans Subject: Re: svn commit: r211505 - head/contrib/gcc 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: Mon, 23 Aug 2010 01:46:39 -0000 On Sat, 21 Aug 2010, Dimitry Andric wrote: > On 2010-08-20 22:36, Bruce Evans wrote: >> On Fri, 20 Aug 2010, Dimitry Andric wrote: > [...] >>> But will the casts not potentially hide problems, if you pass the wrong >>> types to those macros? Maybe it is better if the compiler complains >>> that some argument is of an incompatible type, than just forcing it to >>> cast? >> This is unclear. All integer types are compatible to some extent. >> Upcasting them always works and downcasting them works iff the value >> is not changed. Rui Paulo wrote in a another reply: > I think he meant that downcasting might not work. Yes, but note that for function calls with a prototype in scope, conversion always occurs and no dignostic is required for the dangerous downcasting (or sidecasting with a sign change?) cases. It is only with raw macros or raw asms that you even have a chance to see the original type in the callee. I guess my argument is essentially that since the original code wants to cast everything so as to get almost function call protocol for the macros, we shouldn't change this, except for the lvalues where using function calls would have made the bug obvious. > I meant this in the context of this llvm PR, about matching inline asm > input constraints with output constraints of an incompatible type: > > http://llvm.org/bugs/show_bug.cgi?id=3373 > > Clang is currently somewhat pickier about the arguments to inline asm, > which we also noticed in OpenSSL code, where a rotate-left macro is > defined (for i386 and amd64) as: > > # define ROTATE(a,n) ({ register unsigned int ret; \ > asm ( \ > "roll %1,%0" \ > : "=r"(ret) \ > : "I"(n), "0"(a) \ > : "cc"); \ > ret; \ > }) > > On amd64, it was being called with the 'a' argument being of unsigned > long type. Clang complained: > > crypto/openssl/crypto/md4/md4_dgst.c:117:2: > error: unsupported inline asm: input with type 'unsigned long' matching > output with type 'unsigned int' > R0(A,B,C,D,X( 0), 3,0); HOST_c2l(data,l); X( 2)=l; > ^~~~~~~~~~~~~~~~~~~~~~ > > In this case, the OpenSSL developers chose to explicitly cast 'a' to > 'unsigned int' (see ). Slightly wrong, since you want a macro named ROTATE() to be type-generic, but this one just doesn't work for 64-bit unsigned longs (and without the cast it may be be broken for [u_]chars and [u_]shorts) (and with the cast it is probably broken for signed chars and signed shorts with a negative value). But the authors may know that they only use it on 32-bit values. Bruce