[PATCH] gdb fix for count overflow check

[PATCH] gdb fix for count overflow check

[Eric Lubin]

There is a bug in the overflow check. The overflow check tries to assume that signed integers will wrap around on overflow, and thus a number that wraps around after a multiplication by 10 should no longer be divisible by 10. Unfortunately, signed integer overflow is undefined behavior (see section 2.3 of http://pdos.csail.mit.edu/papers/ub:apsys12.pdf). and a check for signed integer overflow can be optimized out by the compiler. Thus, the check for whether count is not divisible by 10 can be optimized to false by compilers, therefore rendering the error check useless. To mitigate this problem, we check whether the multiplication will overflow, before the operation, by comparing count to INT_MAX/ 10. An integer will overflow when multiplied by 10 if and only if that integer is larger than floor(INT_MAX / 10).


CHANGELOG:
2013-12-07  Eric Lubin  <eric@lubin.us>

	* libiberty/cplus-dem.c: Fixed an integer overflow check.


patch:

diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index e948487..298b8ed 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -492,22 +492,19 @@ consume_count (const char **type)
   if (! ISDIGIT ((unsigned char)**type))
     return -1;
 
+  const int overflow = INT_MAX / 10;
+
   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 for overflow.*/
+      if (count > overflow)
 	{
 	  while (ISDIGIT ((unsigned char) **type))
 	    (*type)++;
 	  return -1;
 	}
-
+      count *= 10;
       count += **type - '0';
       (*type)++;
     }

Re: [PATCH] gdb fix for count overflow check

[Joel Brobecker]

Hello Eric,

> There is a bug in the overflow check. The overflow check tries to
> assume that signed integers will wrap around on overflow, and thus a
> number that wraps around after a multiplication by 10 should no longer
> be divisible by 10. Unfortunately, signed integer overflow is
> undefined behavior (see section 2.3 of
> http://pdos.csail.mit.edu/papers/ub:apsys12.pdf). and a check for
> signed integer overflow can be optimized out by the compiler. Thus,
> the check for whether count is not divisible by 10 can be optimized to
> false by compilers, therefore rendering the error check useless. To
> mitigate this problem, we check whether the multiplication will
> overflow, before the operation, by comparing count to INT_MAX/ 10. An
> integer will overflow when multiplied by 10 if and only if that
> integer is larger than floor(INT_MAX / 10).
> 
> 
> CHANGELOG:
> 2013-12-07  Eric Lubin  <eric@lubin.us>
> 
> 	* libiberty/cplus-dem.c: Fixed an integer overflow check.

Thanks for the patch. The file you are changing is actually shared
between the GCC and binutils-gdb projects, but under GCC maintainership.
So, would you mind re-sending the patch to gcc-patches@gcc.gnu.org?
Once approved there, it's OK to sync the copy in binutils-gdb.git.

Thank you,
-- 
Joel