Message ID | Pine.LNX.4.64.1403291620110.11808@digraph.polyomino.org.uk |
---|---|
State | New |
Headers | show |
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
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
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 --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), };