This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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]

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


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;
 }



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