{"thread":{"id":"25346","subject":"[PATCH v2] do not depend on signed integer overflow","startedAt":"2010-10-04T21:25:22Z","lastAt":"2010-10-04T22:46:36Z","messageCount":3,"participants":["Erik Faye-Lund","Jonathan Nieder"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"152603","messageId":"1286227522-5468-1-git-send-email-kusmabite@gmail.com","threadId":"25346","inReplyTo":null,"subject":"[PATCH v2] do not depend on signed integer overflow","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-10-04T21:25:22Z","receivedAt":"2010-10-04T21:25:22Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Signed integer overflow is not defined in C, so do not depend on it.\n\nThis fixes a problem with GCC 4.4.0 and -O3 where the optimizer would\nconsider \"consumed_bytes > consumed_bytes + bytes\" as a constant\nexpression, and never execute the die()-call.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n builtin/index-pack.c     |    2 +-\n builtin/unpack-objects.c |    2 +-\n git-compat-util.h        |    9 +++++++++\n 3 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 2e680d7..e243d9d 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -161,7 +161,7 @@ static void use(int bytes)\n \tinput_offset += bytes;\n \n \t/* make sure off_t is sufficiently large not to wrap */\n-\tif (consumed_bytes > consumed_bytes + bytes)\n+\tif (signed_add_overflows(consumed_bytes, bytes))\n \t\tdie(\"pack too large for current definition of off_t\");\n \tconsumed_bytes += bytes;\n }\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 685566e..f63973c 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -83,7 +83,7 @@ static void use(int bytes)\n \toffset += bytes;\n \n \t/* make sure off_t is sufficiently large not to wrap */\n-\tif (consumed_bytes > consumed_bytes + bytes)\n+\tif (signed_add_overflows(consumed_bytes, bytes))\n \t\tdie(\"pack too large for current definition of off_t\");\n \tconsumed_bytes += bytes;\n }\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 81883e7..dfb5565 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -28,6 +28,15 @@\n #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))\n #define bitsizeof(x)  (CHAR_BIT * sizeof(x))\n \n+/*\n+ * Signed integer overflow is undefined in C, so here's a helper macro\n+ * to detect if the sum of two integers will overflow. The bitsize to\n+ * overflow at is taken from the first parameter, which must be zero\n+ * or positive.\n+ */\n+#define signed_add_overflows(a, b) \\\n+    ((b) > ((INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a))) - (a)))\n+\n #ifdef __GNUC__\n #define TYPEOF(x) (__typeof__(x))\n #else\n-- \n1.7.3.1.51.g3f36d\n"},{"id":"152613","messageId":"20101004223331.GB9994@burratino","threadId":"25346","inReplyTo":"1286227522-5468-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v2] do not depend on signed integer overflow","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-04T22:33:31Z","receivedAt":"2010-10-04T22:33:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Erik Faye-Lund wrote:\n\n> +++ b/git-compat-util.h\n> @@ -28,6 +28,15 @@\n>  #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))\n>  #define bitsizeof(x)  (CHAR_BIT * sizeof(x))\n>  \n> +/*\n> + * Signed integer overflow is undefined in C, so here's a helper macro\n> + * to detect if the sum of two integers will overflow. The bitsize to\n> + * overflow at is taken from the first parameter, which must be zero\n> + * or positive.\n> + */\n> +#define signed_add_overflows(a, b) \\\n> +    ((b) > ((INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a))) - (a)))\n\nYes, I still like it.  This could be made closer to self-documenting\nlike so:\n\n#define maximum_signed_value_of_type(a) \\\n\t(INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a)))\n\n/*\n * Signed overflow is undefined in C, so here's a helper macro\n * to detect if the sum of two signed integers will overflow.\n *\n * Requires: a >= 0, typeof(a) equals typeof(b)\n */\n#define signed_add_overflows(a, b) \\\n\t((b) > maximum_signed_value_of_type(a) - (a))\n"},{"id":"152614","messageId":"AANLkTim-DRC6k7wg=orYzVS81JRD7D6_UOr+w1ti_Owr@mail.gmail.com","threadId":"25346","inReplyTo":"20101004223331.GB9994@burratino","subject":"Re: [PATCH v2] do not depend on signed integer overflow","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-10-04T22:46:36Z","receivedAt":"2010-10-04T22:46:36Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Oct 5, 2010 at 12:33 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Erik Faye-Lund wrote:\n>\n>> +++ b/git-compat-util.h\n>> @@ -28,6 +28,15 @@\n>>  #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))\n>>  #define bitsizeof(x)  (CHAR_BIT * sizeof(x))\n>>\n>> +/*\n>> + * Signed integer overflow is undefined in C, so here's a helper macro\n>> + * to detect if the sum of two integers will overflow. The bitsize to\n>> + * overflow at is taken from the first parameter, which must be zero\n>> + * or positive.\n>> + */\n>> +#define signed_add_overflows(a, b) \\\n>> +    ((b) > ((INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a))) - (a)))\n>\n> Yes, I still like it.  This could be made closer to self-documenting\n> like so:\n>\n> #define maximum_signed_value_of_type(a) \\\n>        (INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a)))\n>\n> /*\n>  * Signed overflow is undefined in C, so here's a helper macro\n>  * to detect if the sum of two signed integers will overflow.\n>  *\n>  * Requires: a >= 0, typeof(a) equals typeof(b)\n>  */\n> #define signed_add_overflows(a, b) \\\n>        ((b) > maximum_signed_value_of_type(a) - (a))\n>\n\nI like that. Thanks for the suggestion, I'll roll a new version.\n"}]}