# [PATCH v2] do not depend on signed integer overflow 3 messages from 2010-10-04 to 2010-10-04. Participants: Erik Faye-Lund, Jonathan Nieder. Thread: https://gitlist.dev/t/25346 ## Erik Faye-Lund, 2010-10-04 21:25 Subject: [PATCH v2] do not depend on signed integer overflow Message-ID: <1286227522-5468-1-git-send-email-kusmabite@gmail.com> URL: https://gitlist.dev/e/1286227522-5468-1-git-send-email-kusmabite%40gmail.com ``` Signed integer overflow is not defined in C, so do not depend on it. This fixes a problem with GCC 4.4.0 and -O3 where the optimizer would consider "consumed_bytes > consumed_bytes + bytes" as a constant expression, and never execute the die()-call. Signed-off-by: Erik Faye-Lund --- builtin/index-pack.c | 2 +- builtin/unpack-objects.c | 2 +- git-compat-util.h | 9 +++++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 2e680d7..e243d9d 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -161,7 +161,7 @@ static void use(int bytes) input_offset += bytes; /* make sure off_t is sufficiently large not to wrap */ - if (consumed_bytes > consumed_bytes + bytes) + if (signed_add_overflows(consumed_bytes, bytes)) die("pack too large for current definition of off_t"); consumed_bytes += bytes; } diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 685566e..f63973c 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -83,7 +83,7 @@ static void use(int bytes) offset += bytes; /* make sure off_t is sufficiently large not to wrap */ - if (consumed_bytes > consumed_bytes + bytes) + if (signed_add_overflows(consumed_bytes, bytes)) die("pack too large for current definition of off_t"); consumed_bytes += bytes; } diff --git a/git-compat-util.h b/git-compat-util.h index 81883e7..dfb5565 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -28,6 +28,15 @@ #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0])) #define bitsizeof(x) (CHAR_BIT * sizeof(x)) +/* + * Signed integer overflow is undefined in C, so here's a helper macro + * to detect if the sum of two integers will overflow. The bitsize to + * overflow at is taken from the first parameter, which must be zero + * or positive. + */ +#define signed_add_overflows(a, b) \ + ((b) > ((INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a))) - (a))) + #ifdef __GNUC__ #define TYPEOF(x) (__typeof__(x)) #else -- 1.7.3.1.51.g3f36d ``` ## Jonathan Nieder, 2010-10-04 22:33 Subject: Re: [PATCH v2] do not depend on signed integer overflow Message-ID: <20101004223331.GB9994@burratino> URL: https://gitlist.dev/e/20101004223331.GB9994%40burratino In-Reply-To: <1286227522-5468-1-git-send-email-kusmabite@gmail.com> ``` Erik Faye-Lund wrote: > +++ b/git-compat-util.h > @@ -28,6 +28,15 @@ > #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0])) > #define bitsizeof(x) (CHAR_BIT * sizeof(x)) > > +/* > + * Signed integer overflow is undefined in C, so here's a helper macro > + * to detect if the sum of two integers will overflow. The bitsize to > + * overflow at is taken from the first parameter, which must be zero > + * or positive. > + */ > +#define signed_add_overflows(a, b) \ > + ((b) > ((INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a))) - (a))) Yes, I still like it. This could be made closer to self-documenting like so: #define maximum_signed_value_of_type(a) \ (INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a))) /* * Signed overflow is undefined in C, so here's a helper macro * to detect if the sum of two signed integers will overflow. * * Requires: a >= 0, typeof(a) equals typeof(b) */ #define signed_add_overflows(a, b) \ ((b) > maximum_signed_value_of_type(a) - (a)) ``` ## Erik Faye-Lund, 2010-10-04 22:46 Subject: Re: [PATCH v2] do not depend on signed integer overflow Message-ID: URL: https://gitlist.dev/e/AANLkTim-DRC6k7wg%3DorYzVS81JRD7D6_UOr%2Bw1ti_Owr%40mail.gmail.com In-Reply-To: <20101004223331.GB9994@burratino> ``` On Tue, Oct 5, 2010 at 12:33 AM, Jonathan Nieder wrote: > Erik Faye-Lund wrote: > >> +++ b/git-compat-util.h >> @@ -28,6 +28,15 @@ >>  #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0])) >>  #define bitsizeof(x)  (CHAR_BIT * sizeof(x)) >> >> +/* >> + * Signed integer overflow is undefined in C, so here's a helper macro >> + * to detect if the sum of two integers will overflow. The bitsize to >> + * overflow at is taken from the first parameter, which must be zero >> + * or positive. >> + */ >> +#define signed_add_overflows(a, b) \ >> +    ((b) > ((INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a))) - (a))) > > Yes, I still like it.  This could be made closer to self-documenting > like so: > > #define maximum_signed_value_of_type(a) \ >        (INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a))) > > /* >  * Signed overflow is undefined in C, so here's a helper macro >  * to detect if the sum of two signed integers will overflow. >  * >  * Requires: a >= 0, typeof(a) equals typeof(b) >  */ > #define signed_add_overflows(a, b) \ >        ((b) > maximum_signed_value_of_type(a) - (a)) > I like that. Thanks for the suggestion, I'll roll a new version. ```