[PATCH] do not depend on undefined signed integer overflow

Subsystems: the rest

DORMANTno replies

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] do not depend on undefined signed integer overflow

From: Erik Faye-Lund <hidden>
Date: 2016-06-15 22:49:42

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 <redacted>
---
 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..5298747 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 and some compilers take
+ * advantage of this when optimizing, so here's a helper macro to force
+ * wrapping when adding two integers. The size to wrap to is taken from
+ * the first parameter.
+ */
+#define signed_add_overflows(a, b) \
+    ((b) > ((INTMAX_MAX >> (bitsizeof(uintmax_t) - bitsizeof(a))) - (a)))
+
 #ifdef __GNUC__
 #define TYPEOF(x) (__typeof__(x))
 #else
-- 
1.7.3.4.g28d92.dirty

Re: [PATCH] do not depend on undefined signed integer overflow

From: Erik Faye-Lund <hidden>
Date: 2016-06-15 22:49:42

On Mon, Oct 4, 2010 at 9:43 PM, Erik Faye-Lund [off-list ref] wrote:
+/*
+ * Signed integer overflow is undefined in C and some compilers take
+ * advantage of this when optimizing, so here's a helper macro to force
+ * wrapping when adding two integers. The size to wrap to is taken from
+ * the first parameter.
+ */
+#define signed_add_overflows(a, b) \
+    ((b) > ((INTMAX_MAX >> (bitsizeof(uintmax_t) - bitsizeof(a))) - (a)))
+
*Sigh*, I already see that this comment is out of date (I did a
different approach before this one). Sorry, I will fix up this comment
for the next round.

Re: [PATCH] do not depend on undefined signed integer overflow

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:42

Erik Faye-Lund wrote:
quoted hunk
--- 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 and some compilers take
+ * advantage of this when optimizing
[...]
+ */
+#define signed_add_overflows(a, b) \
+    ((b) > ((INTMAX_MAX >> (bitsizeof(uintmax_t) - bitsizeof(a))) - (a)))
Clever.  I like it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help