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

[Stephen Canon via llvm-commits]

This doesn’t actually avoid UB, it just moves it from the negation to a conversion from unsigned to signed.  With the patch, if a is INT_MIN, we have:

	1. convert to unsigned (fully defined, results in 2**N + a for some N)
	2. negate (fully defined, results in -a)
	3. implicit convert back to signed in (UB because -INT_MIN cannot be represented as int).

The approach that I suggested before avoids this problem by having the result type of the absolute value operation be unsigned.

– Steve

> On Nov 2, 2015, at 6:32 AM, 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)
> @@ -33,7 +33,7 @@
>     rep_t sign = 0;
>     if (a < 0) {
>         sign = signBit;
> -        a = -a;
> +        a = -(unsigned int)a;
>     }
> 
>     // Exponent of (fp_t)a is the width of abs(a).
> Index: lib/builtins/floatsisf.c
> ===================================================================
> --- lib/builtins/floatsisf.c	(revision 251670)
> +++ lib/builtins/floatsisf.c	(working copy)
> @@ -33,7 +33,7 @@
>     rep_t sign = 0;
>     if (a < 0) {
>         sign = signBit;
> -        a = -a;
> +        a = -(unsigned int)a;
>     }
> 
>     // Exponent of (fp_t)a is the width of abs(a).