diff mbox

Fix scalb spurious "invalid" exceptions (bug 16770)

Message ID Pine.LNX.4.64.1403291620110.11808@digraph.polyomino.org.uk
State Committed
Headers show

Commit Message

Joseph Myers March 29, 2014, 4:21 p.m. UTC
This patch fixes bug 16770, spurious "invalid" exceptions from scalb
when testing whether the second argument is an integer, by inserting
appropriate range checks to determine whether a cast to int is safe.
(Note that invalid_fn is a function that handles both nonintegers and
large integers, distinguishing them reliably using functions such as
__rint; note also that there are no issues with scalb needing to avoid
spurious "inexact" exceptions - it's an old-POSIX XSI function, not a
standard C function bound to an IEEE 754 operation - although the
return value is still fully determined.)

Tested x86_64 and x86.

2014-03-29  Joseph Myers  <joseph@codesourcery.com>

	[BZ #16770]
	* math/e_scalb.c (__ieee754_scalb): Check second argument is not
	too large before casting to int.
	* math/e_scalbf.c (__ieee754_scalbf): Likewise.
	* math/e_scalbl.c (__ieee754_scalbl): Likewise.
	* math/libm-test.inc (scalb_test_data): Add more tests.

Comments

Andreas Jaeger March 29, 2014, 5:13 p.m. UTC | #1
On 03/29/2014 05:21 PM, Joseph S. Myers wrote:
> This patch fixes bug 16770, spurious "invalid" exceptions from scalb
> when testing whether the second argument is an integer, by inserting
> appropriate range checks to determine whether a cast to int is safe.
> (Note that invalid_fn is a function that handles both nonintegers and
> large integers, distinguishing them reliably using functions such as
> __rint; note also that there are no issues with scalb needing to avoid
> spurious "inexact" exceptions - it's an old-POSIX XSI function, not a
> standard C function bound to an IEEE 754 operation - although the
> return value is still fully determined.)

Thanks,
Andreas
Rich Felker March 29, 2014, 7:06 p.m. UTC | #2
On Sat, Mar 29, 2014 at 04:21:23PM +0000, Joseph S. Myers wrote:
> This patch fixes bug 16770, spurious "invalid" exceptions from scalb
> when testing whether the second argument is an integer, by inserting
> appropriate range checks to determine whether a cast to int is safe.
> (Note that invalid_fn is a function that handles both nonintegers and
> large integers, distinguishing them reliably using functions such as
> __rint; note also that there are no issues with scalb needing to avoid
> spurious "inexact" exceptions - it's an old-POSIX XSI function, not a
> standard C function bound to an IEEE 754 operation - although the
> return value is still fully determined.)
> 
> Tested x86_64 and x86.
> 
> 2014-03-29  Joseph Myers  <joseph@codesourcery.com>
> 
> 	[BZ #16770]
> 	* math/e_scalb.c (__ieee754_scalb): Check second argument is not
> 	too large before casting to int.
> 	* math/e_scalbf.c (__ieee754_scalbf): Likewise.
> 	* math/e_scalbl.c (__ieee754_scalbl): Likewise.
> 	* math/libm-test.inc (scalb_test_data): Add more tests.
> 
> diff --git a/math/e_scalb.c b/math/e_scalb.c
> index bddedfa..146d49e 100644
> --- a/math/e_scalb.c
> +++ b/math/e_scalb.c
> @@ -50,7 +50,7 @@ __ieee754_scalb (double x, double fn)
>  	return x;
>        return x / -fn;
>      }
> -  if (__glibc_unlikely ((double) (int) fn != fn))
> +  if (__glibc_unlikely (fabs (fn) >= 0x1p31 || (double) (int) fn != fn))

Off-by-one for the negative case: -0x1p31 is representable as int.
Maybe it doesn't matter anyway, but if it does, you could test
fn>=0x1p31 and fn<-0x1p31 separately.

Rich
Joseph Myers March 29, 2014, 8:31 p.m. UTC | #3
On Sat, 29 Mar 2014, Rich Felker wrote:

> > -  if (__glibc_unlikely ((double) (int) fn != fn))
> > +  if (__glibc_unlikely (fabs (fn) >= 0x1p31 || (double) (int) fn != fn))
> 
> Off-by-one for the negative case: -0x1p31 is representable as int.
> Maybe it doesn't matter anyway, but if it does, you could test
> fn>=0x1p31 and fn<-0x1p31 separately.

The purpose of this check is to be safe: to avoid the cast if it might be 
out of range; there is no functional need for the cast to occur just 
because it's in range.  Any threshold above 65000.0 (the arbitrary value 
used in invalid_fn to decide whether to call __scalbn with a positive or 
negative scale) would work as well here.
diff mbox

Patch

diff --git a/math/e_scalb.c b/math/e_scalb.c
index bddedfa..146d49e 100644
--- a/math/e_scalb.c
+++ b/math/e_scalb.c
@@ -50,7 +50,7 @@  __ieee754_scalb (double x, double fn)
 	return x;
       return x / -fn;
     }
-  if (__glibc_unlikely ((double) (int) fn != fn))
+  if (__glibc_unlikely (fabs (fn) >= 0x1p31 || (double) (int) fn != fn))
     return invalid_fn (x, fn);
 
   return __scalbn (x, (int) fn);
diff --git a/math/e_scalbf.c b/math/e_scalbf.c
index 319752c..3f2e853 100644
--- a/math/e_scalbf.c
+++ b/math/e_scalbf.c
@@ -50,7 +50,7 @@  __ieee754_scalbf (float x, float fn)
 	return x;
       return x / -fn;
     }
-  if (__glibc_unlikely ((float) (int) fn != fn))
+  if (__glibc_unlikely (fabsf (fn) >= 0x1p31f || (float) (int) fn != fn))
     return invalid_fn (x, fn);
 
   return __scalbnf (x, (int) fn);
diff --git a/math/e_scalbl.c b/math/e_scalbl.c
index 5815a0d..739db7a 100644
--- a/math/e_scalbl.c
+++ b/math/e_scalbl.c
@@ -50,7 +50,7 @@  __ieee754_scalbl (long double x, long double fn)
 	return x;
       return x / -fn;
     }
-  if (__glibc_unlikely ((long double) (int) fn != fn))
+  if (__glibc_unlikely (fabsl (fn) >= 0x1p31L || (long double) (int) fn != fn))
     return invalid_fn (x, fn);
 
   return __scalbnl (x, (int) fn);
diff --git a/math/libm-test.inc b/math/libm-test.inc
index cefcb96..0eff34a 100644
--- a/math/libm-test.inc
+++ b/math/libm-test.inc
@@ -9134,6 +9134,23 @@  static const struct test_ff_f_data scalb_test_data[] =
     TEST_ff_f (scalb, plus_infty, qnan_value, qnan_value, NO_INEXACT_EXCEPTION),
     TEST_ff_f (scalb, qnan_value, qnan_value, qnan_value, NO_INEXACT_EXCEPTION),
 
+    TEST_ff_f (scalb, max_value, max_value, plus_oflow, OVERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, max_value, -max_value, plus_uflow, UNDERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, 1, max_value, plus_oflow, OVERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, 1, -max_value, plus_uflow, UNDERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, min_value, max_value, plus_oflow, OVERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, min_value, -max_value, plus_uflow, UNDERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, min_subnorm_value, max_value, plus_oflow, OVERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, min_subnorm_value, -max_value, plus_uflow, UNDERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, -max_value, max_value, minus_oflow, OVERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, -max_value, -max_value, minus_uflow, UNDERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, -1, max_value, minus_oflow, OVERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, -1, -max_value, minus_uflow, UNDERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, -min_value, max_value, minus_oflow, OVERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, -min_value, -max_value, minus_uflow, UNDERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, -min_subnorm_value, max_value, minus_oflow, OVERFLOW_EXCEPTION),
+    TEST_ff_f (scalb, -min_subnorm_value, -max_value, minus_uflow, UNDERFLOW_EXCEPTION),
+
     TEST_ff_f (scalb, 0.8L, 4, 12.8L),
     TEST_ff_f (scalb, -0.854375L, 5, -27.34L),
   };