This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix scalbn, scalbln integer overflow


Bug 10135 reports an integer overflow problem in scalblnl (which
version is unspecified).

The code in ldbl-96 is an example that illustrates the problem in all
the variants of scalbn and scalbln:

        k = k+n;
        if (__builtin_expect(n> 50000 || k > 0x7ffe, 0))
          return huge*__copysignl(huge,x); /* overflow  */
        if (__builtin_expect(n< -50000, 0))
          return tiny*__copysignl(tiny,x);

Here k is an exponent, type int32_t or int64_t depending on the
implementation, which is fairly close to 0 (may be negative for
subnormals); n is an argument to the function, of type int for scalbn
and long for scalbln.  The expression k+n may overflow (either
positive or negative) except in the case where k is int64_t and n is
32-bit; the conversion storing it back in k may truncate the value if
n is 64-bit long and k is 32-bit.  Apart from undefined behavior on
integer overflow, the "k > 0x7ffe" comparison may thereby produce
incorrect results (so in particular causing the function to return
infinity instead of zero as reported in the bug).

The patch in the bug proposes reordering the tests so that the n <
-50000 test comes first.  This doesn't actually strictly avoid the
undefined behavior.  This patch makes the -50000 test come first, then
the 50000 test, then only after that test does it compute k+n (now
known not to overflow) and use that in a comparison.  It leaves the
compiler to eliminate redundancy from the two computations of k+n in
the source code, as I think this is the cleanest way to write the
code.  Testcases are added that showed the problem before the patch.

Tested x86_64 (full build and test), x86 (full build and test), MIPS
(all three ABIs, math/ tests).

2012-03-02  Joseph Myers  <joseph@codesourcery.com>

	[BZ #10135]
	* sysdeps/ieee754/dbl-64/s_scalbln.c (__scalbln): First test for
	small n, then large n, before computing and testing k+n.
	* sysdeps/ieee754/dbl-64/s_scalbn.c (__scalbn): Likewise.
	* sysdeps/ieee754/dbl-64/wordsize-64/s_scalbln.c (__scalbln):
	Likewise.
	* sysdeps/ieee754/dbl-64/wordsize-64/s_scalbn.c (__scalbn):
	Likewise.
	* sysdeps/ieee754/flt-32/s_scalblnf.c (__scalblnf): Likewise.
	* sysdeps/ieee754/flt-32/s_scalbnf.c (__scalbnf): Likewise.
	* sysdeps/ieee754/ldbl-128/s_scalblnl.c (__scalblnl): Likewise.
	* sysdeps/ieee754/ldbl-128/s_scalbnl.c (__scalbnl): Likewise.
	* sysdeps/ieee754/ldbl-128ibm/s_scalblnl.c (__scalblnl): Likewise.
	* sysdeps/ieee754/ldbl-128ibm/s_scalbnl.c (__scalbnl): Likewise.
	* sysdeps/ieee754/ldbl-96/s_scalblnl.c (__scalblnl): Likewise.
	* sysdeps/ieee754/ldbl-96/s_scalbnl.c (__scalbnl): Likewise.
	* math/libm-test.inc (scalbn_test): Add more tests.
	(scalbln_test): Likewise.

diff --git a/math/libm-test.inc b/math/libm-test.inc
index 9f7d489..e6e3e29 100644
--- a/math/libm-test.inc
+++ b/math/libm-test.inc
@@ -5682,6 +5682,15 @@ scalbn_test (void)
 
   TEST_fi_f (scalbn, 1, 0L, 1);
 
+  TEST_fi_f (scalbn, 1, INT_MAX, plus_infty);
+  TEST_fi_f (scalbn, 1, INT_MIN, plus_zero);
+  TEST_fi_f (scalbn, max_value, INT_MAX, plus_infty);
+  TEST_fi_f (scalbn, max_value, INT_MIN, plus_zero);
+  TEST_fi_f (scalbn, min_value, INT_MAX, plus_infty);
+  TEST_fi_f (scalbn, min_value, INT_MIN, plus_zero);
+  TEST_fi_f (scalbn, min_value / 4, INT_MAX, plus_infty);
+  TEST_fi_f (scalbn, min_value / 4, INT_MIN, plus_zero);
+
   END (scalbn);
 }
 
@@ -5704,6 +5713,35 @@ scalbln_test (void)
 
   TEST_fl_f (scalbln, 1, 0L, 1);
 
+  TEST_fi_f (scalbln, 1, INT_MAX, plus_infty);
+  TEST_fi_f (scalbln, 1, INT_MIN, plus_zero);
+  TEST_fi_f (scalbln, max_value, INT_MAX, plus_infty);
+  TEST_fi_f (scalbln, max_value, INT_MIN, plus_zero);
+  TEST_fi_f (scalbln, min_value, INT_MAX, plus_infty);
+  TEST_fi_f (scalbln, min_value, INT_MIN, plus_zero);
+  TEST_fi_f (scalbln, min_value / 4, INT_MAX, plus_infty);
+  TEST_fi_f (scalbln, min_value / 4, INT_MIN, plus_zero);
+
+  TEST_fi_f (scalbln, 1, LONG_MAX, plus_infty);
+  TEST_fi_f (scalbln, 1, LONG_MIN, plus_zero);
+  TEST_fi_f (scalbln, max_value, LONG_MAX, plus_infty);
+  TEST_fi_f (scalbln, max_value, LONG_MIN, plus_zero);
+  TEST_fi_f (scalbln, min_value, LONG_MAX, plus_infty);
+  TEST_fi_f (scalbln, min_value, LONG_MIN, plus_zero);
+  TEST_fi_f (scalbln, min_value / 4, LONG_MAX, plus_infty);
+  TEST_fi_f (scalbln, min_value / 4, LONG_MIN, plus_zero);
+
+#if LONG_MAX >= 0x100000000
+  TEST_fi_f (scalbln, 1, 0x88000000L, plus_infty);
+  TEST_fi_f (scalbln, 1, -0x88000000L, plus_zero);
+  TEST_fi_f (scalbln, max_value, 0x88000000L, plus_infty);
+  TEST_fi_f (scalbln, max_value, -0x88000000L, plus_zero);
+  TEST_fi_f (scalbln, min_value, 0x88000000L, plus_infty);
+  TEST_fi_f (scalbln, min_value, -0x88000000L, plus_zero);
+  TEST_fi_f (scalbln, min_value / 4, 0x88000000L, plus_infty);
+  TEST_fi_f (scalbln, min_value / 4, -0x88000000L, plus_zero);
+#endif
+
   END (scalbn);
 }
 
diff --git a/sysdeps/ieee754/dbl-64/s_scalbln.c b/sysdeps/ieee754/dbl-64/s_scalbln.c
index 89174b4..bfafefd 100644
--- a/sysdeps/ieee754/dbl-64/s_scalbln.c
+++ b/sysdeps/ieee754/dbl-64/s_scalbln.c
@@ -38,11 +38,11 @@ __scalbln (double x, long int n)
 	    k = ((hx&0x7ff00000)>>20) - 54;
 	    }
 	if (__builtin_expect(k==0x7ff, 0)) return x+x;	/* NaN or Inf */
-	k = k+n;
-	if (__builtin_expect(n> 50000 || k >  0x7fe, 0))
-	  return huge*__copysign(huge,x); /* overflow  */
 	if (__builtin_expect(n< -50000, 0))
 	  return tiny*__copysign(tiny,x); /*underflow*/
+	if (__builtin_expect(n> 50000 || k+n > 0x7fe, 0))
+	  return huge*__copysign(huge,x); /* overflow  */
+	k = k+n;
 	if (__builtin_expect(k > 0, 1))		/* normal result */
 	    {SET_HIGH_WORD(x,(hx&0x800fffff)|(k<<20)); return x;}
 	if (k <= -54)
diff --git a/sysdeps/ieee754/dbl-64/s_scalbn.c b/sysdeps/ieee754/dbl-64/s_scalbn.c
index 1e67dbe..c242f95 100644
--- a/sysdeps/ieee754/dbl-64/s_scalbn.c
+++ b/sysdeps/ieee754/dbl-64/s_scalbn.c
@@ -38,11 +38,11 @@ __scalbn (double x, int n)
 	    k = ((hx&0x7ff00000)>>20) - 54;
 	    }
 	if (__builtin_expect(k==0x7ff, 0)) return x+x;	/* NaN or Inf */
-	k = k+n;
-	if (__builtin_expect(n> 50000 || k >  0x7fe, 0))
-	  return huge*__copysign(huge,x); /* overflow  */
 	if (__builtin_expect(n< -50000, 0))
 	  return tiny*__copysign(tiny,x); /*underflow*/
+	if (__builtin_expect(n> 50000 || k+n > 0x7fe, 0))
+	  return huge*__copysign(huge,x); /* overflow  */
+	k = k+n;
 	if (__builtin_expect(k > 0, 1))		/* normal result */
 	    {SET_HIGH_WORD(x,(hx&0x800fffff)|(k<<20)); return x;}
 	if (k <= -54)
diff --git a/sysdeps/ieee754/dbl-64/wordsize-64/s_scalbln.c b/sysdeps/ieee754/dbl-64/wordsize-64/s_scalbln.c
index d6b97b5..d69eab2 100644
--- a/sysdeps/ieee754/dbl-64/wordsize-64/s_scalbln.c
+++ b/sysdeps/ieee754/dbl-64/wordsize-64/s_scalbln.c
@@ -39,11 +39,11 @@ __scalbln (double x, long int n)
 	    k = ((ix >> 52) & 0x7ff) - 54;
 	    }
 	if (__builtin_expect(k==0x7ff, 0)) return x+x;	/* NaN or Inf */
-	k = k+n;
-	if (__builtin_expect(n> 50000 || k >  0x7fe, 0))
-	  return huge*__copysign(huge,x); /* overflow  */
 	if (__builtin_expect(n< -50000, 0))
 	  return tiny*__copysign(tiny,x); /*underflow*/
+	if (__builtin_expect(n> 50000 || k+n > 0x7fe, 0))
+	  return huge*__copysign(huge,x); /* overflow  */
+	k = k+n;
 	if (__builtin_expect(k > 0, 1))		/* normal result */
 	    {INSERT_WORDS64(x,(ix&UINT64_C(0x800fffffffffffff))|(k<<52));
 	      return x;}
diff --git a/sysdeps/ieee754/dbl-64/wordsize-64/s_scalbn.c b/sysdeps/ieee754/dbl-64/wordsize-64/s_scalbn.c
index 7f0e21f..ef99b87 100644
--- a/sysdeps/ieee754/dbl-64/wordsize-64/s_scalbn.c
+++ b/sysdeps/ieee754/dbl-64/wordsize-64/s_scalbn.c
@@ -39,11 +39,11 @@ __scalbn (double x, int n)
 	    k = ((ix >> 52) & 0x7ff) - 54;
 	    }
 	if (__builtin_expect(k==0x7ff, 0)) return x+x;	/* NaN or Inf */
-	k = k+n;
-	if (__builtin_expect(n> 50000 || k >  0x7fe, 0))
-	  return huge*__copysign(huge,x); /* overflow  */
 	if (__builtin_expect(n< -50000, 0))
 	  return tiny*__copysign(tiny,x); /*underflow*/
+	if (__builtin_expect(n> 50000 || k+n > 0x7fe, 0))
+	  return huge*__copysign(huge,x); /* overflow  */
+	k = k+n;
 	if (__builtin_expect(k > 0, 1))		/* normal result */
 	    {INSERT_WORDS64(x,(ix&UINT64_C(0x800fffffffffffff))|(k<<52));
 	      return x;}
diff --git a/sysdeps/ieee754/flt-32/s_scalblnf.c b/sysdeps/ieee754/flt-32/s_scalblnf.c
index 5256c32..c80038e 100644
--- a/sysdeps/ieee754/flt-32/s_scalblnf.c
+++ b/sysdeps/ieee754/flt-32/s_scalblnf.c
@@ -35,11 +35,11 @@ __scalblnf (float x, long int n)
 	    k = ((ix&0x7f800000)>>23) - 25;
 	    }
 	if (__builtin_expect(k==0xff, 0)) return x+x;	/* NaN or Inf */
-	k = k+n;
-	if (__builtin_expect(n> 50000 || k >  0xfe, 0))
-	  return huge*copysignf(huge,x); /* overflow  */
 	if (__builtin_expect(n< -50000, 0))
 	  return tiny*copysignf(tiny,x);	/*underflow*/
+	if (__builtin_expect(n> 50000 || k+n > 0xfe, 0))
+	  return huge*copysignf(huge,x); /* overflow  */
+	k = k+n;
 	if (__builtin_expect(k > 0, 1))		/* normal result */
 	    {SET_FLOAT_WORD(x,(ix&0x807fffff)|(k<<23)); return x;}
 	if (k <= -25)
diff --git a/sysdeps/ieee754/flt-32/s_scalbnf.c b/sysdeps/ieee754/flt-32/s_scalbnf.c
index 3be2925..b3dceab 100644
--- a/sysdeps/ieee754/flt-32/s_scalbnf.c
+++ b/sysdeps/ieee754/flt-32/s_scalbnf.c
@@ -35,11 +35,11 @@ __scalbnf (float x, int n)
 	    k = ((ix&0x7f800000)>>23) - 25;
 	    }
 	if (__builtin_expect(k==0xff, 0)) return x+x;	/* NaN or Inf */
-	k = k+n;
-	if (__builtin_expect(n> 50000 || k >  0xfe, 0))
-	  return huge*__copysignf(huge,x); /* overflow  */
 	if (__builtin_expect(n< -50000, 0))
 	  return tiny*__copysignf(tiny,x);	/*underflow*/
+	if (__builtin_expect(n> 50000 || k+n > 0xfe, 0))
+	  return huge*__copysignf(huge,x); /* overflow  */
+	k = k+n;
 	if (__builtin_expect(k > 0, 1))		/* normal result */
 	    {SET_FLOAT_WORD(x,(ix&0x807fffff)|(k<<23)); return x;}
 	if (k <= -25)
diff --git a/sysdeps/ieee754/ldbl-128/s_scalblnl.c b/sysdeps/ieee754/ldbl-128/s_scalblnl.c
index f5624e2..44a293a 100644
--- a/sysdeps/ieee754/ldbl-128/s_scalblnl.c
+++ b/sysdeps/ieee754/ldbl-128/s_scalblnl.c
@@ -46,10 +46,10 @@ long double __scalblnl (long double x, long int n)
 	    k = ((hx>>48)&0x7fff) - 114;
 	}
         if (k==0x7fff) return x+x;		/* NaN or Inf */
-        k = k+n;
-        if (n> 50000 || k > 0x7ffe)
-	  return huge*__copysignl(huge,x); /* overflow  */
 	if (n< -50000) return tiny*__copysignl(tiny,x); /*underflow*/
+        if (n> 50000 || k+n > 0x7ffe)
+	  return huge*__copysignl(huge,x); /* overflow  */
+        k = k+n;
         if (k > 0) 				/* normal result */
 	    {SET_LDOUBLE_MSW64(x,(hx&0x8000ffffffffffffULL)|(k<<48)); return x;}
         if (k <= -114)
diff --git a/sysdeps/ieee754/ldbl-128/s_scalbnl.c b/sysdeps/ieee754/ldbl-128/s_scalbnl.c
index b9de0f7..58637e2 100644
--- a/sysdeps/ieee754/ldbl-128/s_scalbnl.c
+++ b/sysdeps/ieee754/ldbl-128/s_scalbnl.c
@@ -46,10 +46,10 @@ long double __scalbnl (long double x, int n)
 	    k = ((hx>>48)&0x7fff) - 114;
 	}
         if (k==0x7fff) return x+x;		/* NaN or Inf */
-        k = k+n;
-        if (n> 50000 || k > 0x7ffe)
-	  return huge*__copysignl(huge,x); /* overflow  */
 	if (n< -50000) return tiny*__copysignl(tiny,x); /*underflow*/
+        if (n> 50000 || k+n > 0x7ffe)
+	  return huge*__copysignl(huge,x); /* overflow  */
+        k = k+n;
         if (k > 0) 				/* normal result */
 	    {SET_LDOUBLE_MSW64(x,(hx&0x8000ffffffffffffULL)|(k<<48)); return x;}
         if (k <= -114)
diff --git a/sysdeps/ieee754/ldbl-128ibm/s_scalblnl.c b/sysdeps/ieee754/ldbl-128ibm/s_scalblnl.c
index a1632e7..70df274 100644
--- a/sysdeps/ieee754/ldbl-128ibm/s_scalblnl.c
+++ b/sysdeps/ieee754/ldbl-128ibm/s_scalblnl.c
@@ -52,10 +52,10 @@ long double __scalblnl (long double x, long int n)
 	    k = ((hx>>52)&0x7ff) - 54;
 	}
 	else if (k==0x7ff) return x+x;		/* NaN or Inf */
-	k = k+n;
-	if (n> 50000 || k > 0x7fe)
-	  return huge*__copysignl(huge,x); /* overflow */
 	if (n< -50000) return tiny*__copysignl(tiny,x); /*underflow */
+	if (n> 50000 || k+n > 0x7fe)
+	  return huge*__copysignl(huge,x); /* overflow */
+	k = k+n;
 	if (k > 0) {				/* normal result */
 	    hx = (hx&0x800fffffffffffffULL)|(k<<52);
 	    if ((lx & 0x7fffffffffffffffULL) == 0) { /* low part +-0 */
diff --git a/sysdeps/ieee754/ldbl-128ibm/s_scalbnl.c b/sysdeps/ieee754/ldbl-128ibm/s_scalbnl.c
index a52cd42..c86fb29 100644
--- a/sysdeps/ieee754/ldbl-128ibm/s_scalbnl.c
+++ b/sysdeps/ieee754/ldbl-128ibm/s_scalbnl.c
@@ -52,10 +52,10 @@ long double __scalbnl (long double x, int n)
 	    k = ((hx>>52)&0x7ff) - 54;
 	}
 	else if (k==0x7ff) return x+x;		/* NaN or Inf */
-	k = k+n;
-	if (n> 50000 || k > 0x7fe)
-	  return huge*__copysignl(huge,x); /* overflow */
 	if (n< -50000) return tiny*__copysignl(tiny,x); /*underflow */
+	if (n> 50000 || k+n > 0x7fe)
+	  return huge*__copysignl(huge,x); /* overflow */
+	k = k+n;
 	if (k > 0) {				/* normal result */
 	    hx = (hx&0x800fffffffffffffULL)|(k<<52);
 	    if ((lx & 0x7fffffffffffffffULL) == 0) { /* low part +-0 */
diff --git a/sysdeps/ieee754/ldbl-96/s_scalblnl.c b/sysdeps/ieee754/ldbl-96/s_scalblnl.c
index ada587b..d0d9cc2 100644
--- a/sysdeps/ieee754/ldbl-96/s_scalblnl.c
+++ b/sysdeps/ieee754/ldbl-96/s_scalblnl.c
@@ -43,11 +43,11 @@ __scalblnl (long double x, long int n)
 	    k = (hx&0x7fff) - 63;
 	    }
 	if (__builtin_expect(k==0x7fff, 0)) return x+x;	/* NaN or Inf */
-	k = k+n;
-	if (__builtin_expect(n> 50000 || k > 0x7ffe, 0))
-	  return huge*__copysignl(huge,x); /* overflow  */
 	if (__builtin_expect(n< -50000, 0))
 	  return tiny*__copysignl(tiny,x);
+	if (__builtin_expect(n> 50000 || k+n > 0x7ffe, 0))
+	  return huge*__copysignl(huge,x); /* overflow  */
+	k = k+n;
 	if (__builtin_expect(k > 0, 1))		/* normal result */
 	    {SET_LDOUBLE_EXP(x,(es&0x8000)|k); return x;}
 	if (k <= -63)
diff --git a/sysdeps/ieee754/ldbl-96/s_scalbnl.c b/sysdeps/ieee754/ldbl-96/s_scalbnl.c
index 6a41920..98f8801 100644
--- a/sysdeps/ieee754/ldbl-96/s_scalbnl.c
+++ b/sysdeps/ieee754/ldbl-96/s_scalbnl.c
@@ -43,11 +43,11 @@ __scalbnl (long double x, int n)
 	    k = (hx&0x7fff) - 64;
 	    }
 	if (__builtin_expect(k==0x7fff, 0)) return x+x;	/* NaN or Inf */
-	k = k+n;
-	if (__builtin_expect(n> 50000 || k > 0x7ffe, 0))
-	  return huge*__copysignl(huge,x); /* overflow  */
 	if (__builtin_expect(n< -50000, 0))
 	  return tiny*__copysignl(tiny,x);
+	if (__builtin_expect(n> 50000 || k+n > 0x7ffe, 0))
+	  return huge*__copysignl(huge,x); /* overflow  */
+	k = k+n;
 	if (__builtin_expect(k > 0, 1))		/* normal result */
 	    {SET_LDOUBLE_EXP(x,(es&0x8000)|k); return x;}
 	if (k <= -64)

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]