[PATCH] Fix undefined behaviour in lround,llround.

[Dave Korn]

    Hi all,

  Back at the start of June we had a report(*) of a bug in the gfortran nint()
function, which I tracked down to a problem in the underlying implementation
of lround() in newlib.  The bug occurs here:

>   else if (exponent_less_1023 < (8 * sizeof (long int)) - 1)
>     {
>       if (exponent_less_1023 >= 52)
>         result = ((long int) msw << (exponent_less_1023 - 20)) | (lsw << (exponent_less_1023 - 52));
>       else
>         {
>           unsigned int tmp = lsw + (0x80000000 >> (exponent_less_1023 - 20));
>           if (tmp < lsw)
>             ++msw;
>           result = ((long int) msw << (exponent_less_1023 - 20)) | (tmp >> (52 - exponent_less_1023));
>         }
>     }
>   else

  When exponent_less_1023 is exactly 52, the subexpression "((long int) msw <<
(exponent_less_1023 - 20))" becomes a left shift by 32.  On systems where a
long int is 32 bits, that's undefined behaviour; the shift amount must always
be less than the size of the left-hand operand (n1256, 6.5.7#3), and in
practice, this means that a standard x86 shift opcode is used, which has the
property of implicitly AND-ing the shift amount with 31; the result of a shift
by 32 bits thus isn't zero as the code expects, but the original unshifted
value of msw, which gets OR-d into completely the wrong place and splots bits
all over the correctly-shifted lsw part of the result.

  In this patch I define a couple of safe shift macros that return zero and
avoid the undefined behaviour when the shift amount is too great.  I've
actually been slightly over-cautious in applying it a couple of places, I
think, but this isn't the fast math code and anyway value-range analysis is a
job for a compiler, which it can do bearing in mind the actual type sizes in
effect during compilation.

newlib/ChangeLog:

	* libm/common/fdlibm.h (SAFE_LEFT_SHIFT): New macro definition.
	(SAFE_RIGHT_SHIFT): Likewise.
	* libm/common/s_llround.c (llround): Use them to avoid undefined
	behaviour.
	* libm/common/s_lround.c (lround): Likewise.

  OK?

    cheers,
      DaveK
-- 
(*) - http://cygwin.com/ml/cygwin/2010-06/threads.html#00315
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fpmath-lround-fix.diff
Type: text/x-c
Size: 3422 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/newlib/attachments/20100716/511619ca/attachment.bin>