* libm/common/fdlibm.h (SAFE_LEFT_SHIFT): New macro definition.

	(SAFE_RIGHT_SHIFT): Likewise.
	* libm/common/s_llround.c (llround): Annotate shift operations with
	possible shift amount ranges, and use SAFE_RIGHT_SHIFT to avoid
	undefined behaviour.
	* libm/common/s_lround.c (lround): Likewise.

4 files changed, 62 insertions, 7 deletions

diff --git a/newlib/ChangeLog b/newlib/ChangeLog
index 35a7cefb9..16359ce21 100644
--- a/newlib/ChangeLog
+++ b/newlib/ChangeLog
@@ -1,3 +1,12 @@
+2010-07-20  Dave Korn  <dave.korn.cygwin@gmail.com>
+
+	* libm/common/fdlibm.h (SAFE_LEFT_SHIFT): New macro definition.
+	(SAFE_RIGHT_SHIFT): Likewise.
+	* libm/common/s_llround.c (llround): Annotate shift operations with
+	possible shift amount ranges, and use SAFE_RIGHT_SHIFT to avoid
+	undefined behaviour.
+	* libm/common/s_lround.c (lround): Likewise.
+
 2010-07-19  Eric Blake  <eblake@redhat.com>
 
 	* libc/stdio/mktemp.c (_gettemp): Add parameter, all callers
diff --git a/newlib/libm/common/fdlibm.h b/newlib/libm/common/fdlibm.h
index 3877ff2dc..ab0ada1a1 100644
--- a/newlib/libm/common/fdlibm.h
+++ b/newlib/libm/common/fdlibm.h
@@ -361,3 +361,13 @@ do {								\
   sf_u.word = (i);						\
   (d) = sf_u.value;						\
 } while (0)
+
+/* Macros to avoid undefined behaviour that can arise if the amount
+   of a shift is exactly equal to the size of the shifted operand.  */
+
+#define SAFE_LEFT_SHIFT(op,amt)					\
+  (((amt) < 8 * sizeof(op)) ? ((op) << (amt)) : 0)
+
+#define SAFE_RIGHT_SHIFT(op,amt)				\
+  (((amt) < 8 * sizeof(op)) ? ((op) >> (amt)) : 0)
+
diff --git a/newlib/libm/common/s_llround.c b/newlib/libm/common/s_llround.c
index 923f885ca..3dcb5ff92 100644
--- a/newlib/libm/common/s_llround.c
+++ b/newlib/libm/common/s_llround.c
@@ -31,8 +31,10 @@ llround(double x)
   msw &= 0x000fffff;
   msw |= 0x00100000;
 
+  /* exponent_less_1023 in [-1024,1023] */
   if (exponent_less_1023 < 20)
     {
+      /* exponent_less_1023 in [-1024,19] */
       if (exponent_less_1023 < 0)
         {
           if (exponent_less_1023 < -1)
@@ -42,20 +44,34 @@ llround(double x)
         }
       else
         {
+          /* exponent_less_1023 in [0,19] */
+	  /* shift amt in [0,19] */
           msw += 0x80000 >> exponent_less_1023;
+	  /* shift amt in [20,1] */
           result = msw >> (20 - exponent_less_1023);
         }
     }
   else if (exponent_less_1023 < (8 * sizeof (long long int)) - 1)
     {
+      /* 64bit longlong: exponent_less_1023 in [20,62] */
       if (exponent_less_1023 >= 52)
-        result = ((long long int) msw << (exponent_less_1023 - 20)) | (lsw << (exponent_less_1023 - 52));
+	/* 64bit longlong: exponent_less_1023 in [52,62] */
+	/* 64bit longlong: shift amt in [32,42] */
+        result = ((long long int) msw << (exponent_less_1023 - 20))
+		    /* 64bit longlong: shift amt in [0,10] */
+                    | (lsw << (exponent_less_1023 - 52));
       else
         {
-          unsigned int tmp = lsw + (0x80000000 >> (exponent_less_1023 - 20));
+	  /* 64bit longlong: exponent_less_1023 in [20,51] */
+          unsigned int tmp = lsw
+		    /* 64bit longlong: shift amt in [0,31] */
+                    + (0x80000000 >> (exponent_less_1023 - 20));
           if (tmp < lsw)
             ++msw;
-          result = ((long long int) msw << (exponent_less_1023 - 20)) | (tmp >> (52 - exponent_less_1023));
+	  /* 64bit longlong: shift amt in [0,31] */
+          result = ((long long int) msw << (exponent_less_1023 - 20))
+		    /* ***64bit longlong: shift amt in [32,1] */
+                    | SAFE_RIGHT_SHIFT (tmp, (52 - exponent_less_1023));
         }
     }
   else
diff --git a/newlib/libm/common/s_lround.c b/newlib/libm/common/s_lround.c
index b892a743c..fecc62772 100644
--- a/newlib/libm/common/s_lround.c
+++ b/newlib/libm/common/s_lround.c
@@ -71,9 +71,10 @@ ANSI C, POSIX
   exponent_less_1023 = ((msw & 0x7ff00000) >> 20) - 1023;
   msw &= 0x000fffff;
   msw |= 0x00100000;
-
+  /* exponent_less_1023 in [-1024,1023] */
   if (exponent_less_1023 < 20)
     {
+      /* exponent_less_1023 in [-1024,19] */
       if (exponent_less_1023 < 0)
         {
           if (exponent_less_1023 < -1)
@@ -83,20 +84,39 @@ ANSI C, POSIX
         }
       else
         {
+          /* exponent_less_1023 in [0,19] */
+	  /* shift amt in [0,19] */
           msw += 0x80000 >> exponent_less_1023;
+	  /* shift amt in [20,1] */
           result = msw >> (20 - exponent_less_1023);
         }
     }
   else if (exponent_less_1023 < (8 * sizeof (long int)) - 1)
     {
+      /* 32bit long: exponent_less_1023 in [20,30] */
+      /* 64bit long: exponent_less_1023 in [20,62] */
       if (exponent_less_1023 >= 52)
-        result = ((long int) msw << (exponent_less_1023 - 20)) | (lsw << (exponent_less_1023 - 52));
+	/* 64bit long: exponent_less_1023 in [52,62] */
+	/* 64bit long: shift amt in [32,42] */
+        result = ((long int) msw << (exponent_less_1023 - 20))
+		/* 64bit long: shift amt in [0,10] */
+                | (lsw << (exponent_less_1023 - 52));
       else
         {
-          unsigned int tmp = lsw + (0x80000000 >> (exponent_less_1023 - 20));
+	  /* 32bit long: exponent_less_1023 in [20,30] */
+	  /* 64bit long: exponent_less_1023 in [20,51] */
+          unsigned int tmp = lsw
+		    /* 32bit long: shift amt in [0,10] */
+		    /* 64bit long: shift amt in [0,31] */
+                    + (0x80000000 >> (exponent_less_1023 - 20));
           if (tmp < lsw)
             ++msw;
-          result = ((long int) msw << (exponent_less_1023 - 20)) | (tmp >> (52 - exponent_less_1023));
+	  /* 32bit long: shift amt in [0,10] */
+	  /* 64bit long: shift amt in [0,31] */
+          result = ((long int) msw << (exponent_less_1023 - 20))
+		    /* ***32bit long: shift amt in [32,22] */
+		    /* ***64bit long: shift amt in [32,1] */
+                    | SAFE_RIGHT_SHIFT (tmp, (52 - exponent_less_1023));
         }
     }
   else