Re: [PATCH] avoid undefined behavior in libiberty/cplus-dem.c

[Nickolai Zeldovich]

On Mon, 7 Jan 2013, Jakub Jelinek wrote:
Won't the above preclude parsing 2147483640 up to 2147483647 ?
Because then in the last iteration count 214748364 > (INT_MAX - 9) / 10.

You're right -- thanks for catching that!  Below is a patch with a more 
precise check.

Nickolai.

--- libiberty/cplus-dem.c       (revision 194959)
+++ libiberty/cplus-dem.c       (working copy)
@@ -48,7 +48,18 @@
 #include <sys/types.h>
 #include <string.h>
 #include <stdio.h>
+#ifdef HAVE_LIMITS_H
+#include <limits.h>
+#endif

+#ifndef UINT_MAX
+#define UINT_MAX       ((unsigned int)(~0U))           /* 0xFFFFFFFF */
+#endif
+
+#ifndef INT_MAX
+#define INT_MAX                ((int)(UINT_MAX >> 1))            /* 0x7FFFFFFF 
*/
+#endif
+
 #ifdef HAVE_STDLIB_H
 #include <stdlib.h>
 #else
@@ -494,28 +505,25 @@

   while (ISDIGIT ((unsigned char)**type))
     {
+      /* Check whether we can add another digit without overflow. */
+      if (count > INT_MAX / 10)
+       goto overflow;
+
       count *= 10;

-      /* Check for overflow.
-        We assume that count is represented using two's-complement;
-        no power of two is divisible by ten, so if an overflow occurs
-        when multiplying by ten, the result will not be a multiple of
-        ten.  */
-      if ((count % 10) != 0)
-       {
-         while (ISDIGIT ((unsigned char) **type))
-           (*type)++;
-         return -1;
-       }
+      if (count > INT_MAX - (**type - '0'))
+       goto overflow;

       count += **type - '0';
       (*type)++;
     }

-  if (count < 0)
-    count = -1;
+  return count;

-  return (count);
+ overflow:
+  while (ISDIGIT ((unsigned char) **type))
+    (*type)++;
+  return -1;
 }