Patchwork avoid undefined behavior in libiberty/cplus-dem.c

login
register
mail settings
Submitter Nickolai Zeldovich
Date Jan. 7, 2013, 4:25 a.m.
Message ID <Pine.LNX.4.64.1301062320070.31599@escalante.csail.mit.edu>
Download mbox | patch
Permalink /patch/209851/
State New
Headers show

Comments

Nickolai Zeldovich - Jan. 7, 2013, 4:25 a.m.
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.
Jakub Jelinek - Jan. 7, 2013, 8:18 a.m.
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

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