soft-fp: Fix _FP_TO_INT latent bug in overflow handling
diff mbox

Message ID Pine.LNX.4.64.1409190139150.20342@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers Sept. 19, 2014, 1:40 a.m. UTC
This patch, relative to a tree with
<https://sourceware.org/ml/libc-alpha/2014-09/msg00461.html> and
<https://sourceware.org/ml/libc-alpha/2014-09/msg00463.html> (pending
review) applied, fixes a latent bug in _FP_TO_INT regarding handling
of arguments with maximum exponent (infinities and NaNs).  If the
maximum exponent is below that calculated as an overflow threshold,
such values would incorrectly be treated as normal values for the
purposes of the conversion.  This could not occur for any of the
conversions actually occurring in glibc, libgcc or the Linux kernel
(the maximum exponent for float is, just, big enough to ensure
overflow for unsigned __int128), but would apply if soft-fp were used
for IEEE binary16.  Appropriate checks are inserted to ensure that the
maximum exponent is always treated as an overflowing exponent, and
never as a normal one.

Tested for powerpc-nofpu that the disassembly of installed shared
libraries is unchanged by this patch.

2014-09-19  Joseph Myers  <joseph@codesourcery.com>

	* soft-fp/op-common.h (_FP_TO_INT): Ensure maximum exponent is
	treated as invalid conversion, not as normal exponent.

Comments

Joseph Myers Sept. 26, 2014, 6:18 p.m. UTC | #1
Ping.  This patch 
<https://sourceware.org/ml/libc-alpha/2014-09/msg00464.html> is pending 
review.
Joseph Myers Oct. 2, 2014, 10:20 a.m. UTC | #2
Ping^2.  This patch 
<https://sourceware.org/ml/libc-alpha/2014-09/msg00464.html> is still 
pending review.
Carlos O'Donell Oct. 9, 2014, 12:28 a.m. UTC | #3
On 09/18/2014 09:40 PM, Joseph S. Myers wrote:
> This patch, relative to a tree with
> <https://sourceware.org/ml/libc-alpha/2014-09/msg00461.html> and
> <https://sourceware.org/ml/libc-alpha/2014-09/msg00463.html> (pending
> review) applied, fixes a latent bug in _FP_TO_INT regarding handling
> of arguments with maximum exponent (infinities and NaNs).  If the
> maximum exponent is below that calculated as an overflow threshold,
> such values would incorrectly be treated as normal values for the
> purposes of the conversion.  This could not occur for any of the
> conversions actually occurring in glibc, libgcc or the Linux kernel
> (the maximum exponent for float is, just, big enough to ensure
> overflow for unsigned __int128), but would apply if soft-fp were used
> for IEEE binary16.  Appropriate checks are inserted to ensure that the
> maximum exponent is always treated as an overflowing exponent, and
> never as a normal one.
> 
> Tested for powerpc-nofpu that the disassembly of installed shared
> libraries is unchanged by this patch.
> 
> 2014-09-19  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* soft-fp/op-common.h (_FP_TO_INT): Ensure maximum exponent is
> 	treated as invalid conversion, not as normal exponent.

Looks good to me.

I'm assuming that implicit boolean coercion is allowed in soft-fp?

> diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h
> index 833658d..8c928af 100644
> --- a/soft-fp/op-common.h
> +++ b/soft-fp/op-common.h
> @@ -1381,7 +1381,9 @@
>  	  else								\
>  	    FP_SET_EXCEPTION (FP_EX_INEXACT);				\
>  	}								\
> -      else if (X##_e >= _FP_EXPBIAS_##fs + rsize - (rsigned > 0 || X##_s) \
> +      else if (X##_e >= (_FP_EXPMAX_##fs < _FP_EXPBIAS_##fs + rsize	\
> +			 ? _FP_EXPMAX_##fs				\
> +			 : _FP_EXPBIAS_##fs + rsize - (rsigned > 0 || X##_s)) \

OK.

>  	       || (!rsigned && X##_s))					\
>  	{								\
>  	  /* Overflow or converting to the most negative integer.  */	\
> @@ -1398,7 +1400,10 @@
>  		r = ~r;							\
>  	    }								\
>  									\
> -	  if (rsigned && X##_s && X##_e == _FP_EXPBIAS_##fs + rsize - 1) \
> +	  if (_FP_EXPBIAS_##fs + rsize - 1 < _FP_EXPMAX_##fs		\
> +	      && rsigned						\
> +	      && X##_s							\
> +	      && X##_e == _FP_EXPBIAS_##fs + rsize - 1)			\

OK.

>  	    {								\
>  	      /* Possibly converting to most negative integer; check the \
>  		 mantissa.  */						\
> 

Cheers,
Carlos.

Patch
diff mbox

diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h
index 833658d..8c928af 100644
--- a/soft-fp/op-common.h
+++ b/soft-fp/op-common.h
@@ -1381,7 +1381,9 @@ 
 	  else								\
 	    FP_SET_EXCEPTION (FP_EX_INEXACT);				\
 	}								\
-      else if (X##_e >= _FP_EXPBIAS_##fs + rsize - (rsigned > 0 || X##_s) \
+      else if (X##_e >= (_FP_EXPMAX_##fs < _FP_EXPBIAS_##fs + rsize	\
+			 ? _FP_EXPMAX_##fs				\
+			 : _FP_EXPBIAS_##fs + rsize - (rsigned > 0 || X##_s)) \
 	       || (!rsigned && X##_s))					\
 	{								\
 	  /* Overflow or converting to the most negative integer.  */	\
@@ -1398,7 +1400,10 @@ 
 		r = ~r;							\
 	    }								\
 									\
-	  if (rsigned && X##_s && X##_e == _FP_EXPBIAS_##fs + rsize - 1) \
+	  if (_FP_EXPBIAS_##fs + rsize - 1 < _FP_EXPMAX_##fs		\
+	      && rsigned						\
+	      && X##_s							\
+	      && X##_e == _FP_EXPBIAS_##fs + rsize - 1)			\
 	    {								\
 	      /* Possibly converting to most negative integer; check the \
 		 mantissa.  */						\