[PATCH v2] [compiler-rt] Avoid undefined behaviour in __floatsisf and __floatsidf

[Stephen Canon via llvm-commits]

Looks good.  Thanks for following up on this.

– Steve

> On Nov 2, 2015, at 9:59 PM, Matthew Fernandez <matthew.fernandez at gmail.com> wrote:
> 
> These two functions for soft floating point support negate their signed int argument if it is negative. In the case where the argument is INT_MIN, this negation is undefined behaviour with respect to the C standard. This change performs the negation on an unsigned value, avoiding the just-described situation. This change does not alter the intended semantics of these functions.
> 
> Signed-off-by: Matthew Fernandez <matthew.fernandez at gmail.com>
> 
> Index: lib/builtins/floatsidf.c
> ===================================================================
> --- lib/builtins/floatsidf.c	(revision 251670)
> +++ lib/builtins/floatsidf.c	(working copy)
> @@ -31,13 +31,14 @@
> 
>     // All other cases begin by extracting the sign and absolute value of a
>     rep_t sign = 0;
> +    unsigned int aAbs = a;
>     if (a < 0) {
>         sign = signBit;
> -        a = -a;
> +        aAbs = -aAbs;
>     }
> 
>     // Exponent of (fp_t)a is the width of abs(a).
> -    const int exponent = (aWidth - 1) - __builtin_clz(a);
> +    const int exponent = (aWidth - 1) - __builtin_clz(aAbs);
>     rep_t result;
> 
>     // Shift a into the significand field and clear the implicit bit.  Extra
> @@ -44,7 +45,7 @@
>     // cast to unsigned int is necessary to get the correct behavior for
>     // the input INT_MIN.
>     const int shift = significandBits - exponent;
> -    result = (rep_t)(unsigned int)a << shift ^ implicitBit;
> +    result = (rep_t)aAbs << shift ^ implicitBit;
> 
>     // Insert the exponent
>     result += (rep_t)(exponent + exponentBias) << significandBits;
> Index: lib/builtins/floatsisf.c
> ===================================================================
> --- lib/builtins/floatsisf.c	(revision 251670)
> +++ lib/builtins/floatsisf.c	(working copy)
> @@ -31,23 +31,24 @@
> 
>     // All other cases begin by extracting the sign and absolute value of a
>     rep_t sign = 0;
> +    unsigned int aAbs = a;
>     if (a < 0) {
>         sign = signBit;
> -        a = -a;
> +        aAbs = -aAbs;
>     }
> 
>     // Exponent of (fp_t)a is the width of abs(a).
> -    const int exponent = (aWidth - 1) - __builtin_clz(a);
> +    const int exponent = (aWidth - 1) - __builtin_clz(aAbs);
>     rep_t result;
> 
>     // Shift a into the significand field, rounding if it is a right-shift
>     if (exponent <= significandBits) {
>         const int shift = significandBits - exponent;
> -        result = (rep_t)a << shift ^ implicitBit;
> +        result = (rep_t)aAbs << shift ^ implicitBit;
>     } else {
>         const int shift = exponent - significandBits;
> -        result = (rep_t)a >> shift ^ implicitBit;
> -        rep_t round = (rep_t)a << (typeWidth - shift);
> +        result = (rep_t)aAbs >> shift ^ implicitBit;
> +        rep_t round = (rep_t)aAbs << (typeWidth - shift);
>         if (round > signBit) result++;
>         if (round == signBit) result += result & 1;
>     }