{"thread":{"id":"25342","subject":"[PATCH] do not depend on undefined signed integer overflow","startedAt":"2010-10-04T19:43:08Z","lastAt":"2010-10-04T20:48:48Z","messageCount":3,"participants":["Erik Faye-Lund","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"152583","messageId":"1286221388-4908-1-git-send-email-kusmabite@gmail.com","threadId":"25342","inReplyTo":null,"subject":"[PATCH] do not depend on undefined signed integer overflow","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-10-04T19:43:08Z","receivedAt":"2010-10-04T19:43:08Z","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..5298747 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 and some compilers take\n+ * advantage of this when optimizing, so here's a helper macro to force\n+ * wrapping when adding two integers. The size to wrap to is taken from\n+ * the first parameter.\n+ */\n+#define signed_add_overflows(a, b) \\\n+    ((b) > ((INTMAX_MAX >> (bitsizeof(uintmax_t) - bitsizeof(a))) - (a)))\n+\n #ifdef __GNUC__\n #define TYPEOF(x) (__typeof__(x))\n #else\n-- \n1.7.3.4.g28d92.dirty\n"},{"id":"152584","messageId":"AANLkTimx0PbyTPgwq8=VoE2Ldihf44c2UkDGYom4TAcf@mail.gmail.com","threadId":"25342","inReplyTo":"1286221388-4908-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH] do not depend on undefined signed integer overflow","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-10-04T19:47:01Z","receivedAt":"2010-10-04T19:47:01Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Oct 4, 2010 at 9:43 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> +/*\n> + * Signed integer overflow is undefined in C and some compilers take\n> + * advantage of this when optimizing, so here's a helper macro to force\n> + * wrapping when adding two integers. The size to wrap to is taken from\n> + * the first parameter.\n> + */\n> +#define signed_add_overflows(a, b) \\\n> +    ((b) > ((INTMAX_MAX >> (bitsizeof(uintmax_t) - bitsizeof(a))) - (a)))\n> +\n\n*Sigh*, I already see that this comment is out of date (I did a\ndifferent approach before this one). Sorry, I will fix up this comment\nfor the next round.\n"},{"id":"152591","messageId":"20101004204848.GI6466@burratino","threadId":"25342","inReplyTo":"1286221388-4908-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH] do not depend on undefined signed integer overflow","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-04T20:48:48Z","receivedAt":"2010-10-04T20:48:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Erik Faye-Lund wrote:\n\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 and some compilers take\n> + * advantage of this when optimizing\n[...]\n> + */\n> +#define signed_add_overflows(a, b) \\\n> +    ((b) > ((INTMAX_MAX >> (bitsizeof(uintmax_t) - bitsizeof(a))) - (a)))\n\nClever.  I like it.\n"}]}