svn commit: r211505 - head/contrib/gcc

[Bruce Evans]

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 <http://cvs.openssl.org/chngview?cn=19818>).

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