diff mbox

avoid undefined behavior in libiberty/cplus-dem.c

Message ID Pine.LNX.4.64.1301062320070.31599@escalante.csail.mit.edu
State New
Headers show

Commit Message

Nickolai Zeldovich Jan. 7, 2013, 4:25 a.m. UTC 
consume_count() in libiberty/cplus-dem.c relies on signed integer overflow 
(which is undefined behavior in C) to detect overflow when parsing a count 
value.  As a result, recent versions of gcc (e.g., 4.7.2) will remove that 
if check altogether as dead code, since it can only be true with UB.  The 
patch below replaces the overflow check with one that does not rely on UB.

Nickolai.

Comments

Jakub Jelinek Jan. 7, 2013, 8:18 a.m. UTC | #1 
On Sun, Jan 06, 2013 at 11:25:44PM -0500, Nickolai Zeldovich wrote:
> @@ -494,20 +505,15 @@
> 
>    while (ISDIGIT ((unsigned char)**type))
>      {
> -      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)
> +      /* Check whether we can add another digit without overflow. */
> +      if (count > (INT_MAX - 9) / 10)
>  	{
>  	  while (ISDIGIT ((unsigned char) **type))
>  	    (*type)++;
>  	  return -1;
>  	}
> 
> +      count *= 10;
>        count += **type - '0';
>        (*type)++;

Won't the above preclude parsing 2147483640 up to 2147483647 ?
Because then in the last iteration count 214748364 > (INT_MAX - 9) / 10.

	Jakub
diff mbox

Patch

Index: libiberty/cplus-dem.c
===================================================================
--- 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,20 +505,15 @@ 

    while (ISDIGIT ((unsigned char)**type))
      {
-      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)
+      /* Check whether we can add another digit without overflow. */
+      if (count > (INT_MAX - 9) / 10)
  	{
  	  while (ISDIGIT ((unsigned char) **type))
  	    (*type)++;
  	  return -1;
  	}

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