* [PATCH v4] do not depend on signed integer overflow
@ 2010-10-05 7:24 Erik Faye-Lund
2010-10-05 13:28 ` Nicolas Pitre
2011-02-10 9:35 ` [PATCH] compat: helper for detecting unsigned overflow Jonathan Nieder
0 siblings, 2 replies; 5+ messages in thread
From: Erik Faye-Lund @ 2010-10-05 7:24 UTC (permalink / raw)
To: git; +Cc: jrnieder, peff, nico
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 <kusmabite@gmail.com>
---
builtin/index-pack.c | 2 +-
builtin/pack-objects.c | 2 +-
builtin/unpack-objects.c | 2 +-
git-compat-util.h | 12 ++++++++++++
4 files changed, 15 insertions(+), 3 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/pack-objects.c b/builtin/pack-objects.c
index 3756cf3..d5a8db1 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -431,7 +431,7 @@ static int write_one(struct sha1file *f,
written_list[nr_written++] = &e->idx;
/* make sure off_t is sufficiently large not to wrap */
- if (*offset > *offset + size)
+ if (signed_add_overflows(*offset, size))
die("pack too large for current definition of off_t");
*offset += size;
return 1;
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..2af8d3e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -28,6 +28,18 @@
#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
#define bitsizeof(x) (CHAR_BIT * sizeof(x))
+#define maximum_signed_value_of_type(a) \
+ (INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a)))
+
+/*
+ * Signed integer overflow is undefined in C, so here's a helper macro
+ * to detect if the sum of two 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))
+
#ifdef __GNUC__
#define TYPEOF(x) (__typeof__(x))
#else
--
1.7.3.1.51.ge462f.dirty
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4] do not depend on signed integer overflow
2010-10-05 7:24 [PATCH v4] do not depend on signed integer overflow Erik Faye-Lund
@ 2010-10-05 13:28 ` Nicolas Pitre
2011-02-10 9:35 ` [PATCH] compat: helper for detecting unsigned overflow Jonathan Nieder
1 sibling, 0 replies; 5+ messages in thread
From: Nicolas Pitre @ 2010-10-05 13:28 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, jrnieder, peff
On Tue, 5 Oct 2010, Erik Faye-Lund wrote:
> 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 <kusmabite@gmail.com>
Acked-by: Nicolas Pitre <nico@fluxnic.net>
> ---
> builtin/index-pack.c | 2 +-
> builtin/pack-objects.c | 2 +-
> builtin/unpack-objects.c | 2 +-
> git-compat-util.h | 12 ++++++++++++
> 4 files changed, 15 insertions(+), 3 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/pack-objects.c b/builtin/pack-objects.c
> index 3756cf3..d5a8db1 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -431,7 +431,7 @@ static int write_one(struct sha1file *f,
> written_list[nr_written++] = &e->idx;
>
> /* make sure off_t is sufficiently large not to wrap */
> - if (*offset > *offset + size)
> + if (signed_add_overflows(*offset, size))
> die("pack too large for current definition of off_t");
> *offset += size;
> return 1;
> 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..2af8d3e 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -28,6 +28,18 @@
> #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
> #define bitsizeof(x) (CHAR_BIT * sizeof(x))
>
> +#define maximum_signed_value_of_type(a) \
> + (INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a)))
> +
> +/*
> + * Signed integer overflow is undefined in C, so here's a helper macro
> + * to detect if the sum of two 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))
> +
> #ifdef __GNUC__
> #define TYPEOF(x) (__typeof__(x))
> #else
> --
> 1.7.3.1.51.ge462f.dirty
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] compat: helper for detecting unsigned overflow
2010-10-05 7:24 [PATCH v4] do not depend on signed integer overflow Erik Faye-Lund
2010-10-05 13:28 ` Nicolas Pitre
@ 2011-02-10 9:35 ` Jonathan Nieder
2011-02-10 12:11 ` Sverre Rabbelier
1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Nieder @ 2011-02-10 9:35 UTC (permalink / raw)
To: git
Cc: Erik Faye-Lund, Jeff King, Nicolas Pitre, Pierre Habouzit,
Ilari Liusvaara
Date: Sun, 10 Oct 2010 21:59:26 -0500
The idiom (a + b < a) works fine for detecting that an unsigned
integer has overflowed, but a more explicit
unsigned_add_overflows(a, b)
might be easier to read.
Define such a macro, expanding roughly to ((a) < UINT_MAX - (b)).
Because the expansion uses each argument only once outside of sizeof()
expressions, it is safe to use with arguments that have side effects.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
From the svn remote helper project. Sane?
git-compat-util.h | 6 ++++++
patch-delta.c | 2 +-
strbuf.c | 5 +++--
wrapper.c | 2 +-
4 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index d6d269f..9c23622 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -31,6 +31,9 @@
#define maximum_signed_value_of_type(a) \
(INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a)))
+#define maximum_unsigned_value_of_type(a) \
+ (UINTMAX_MAX >> (bitsizeof(uintmax_t) - bitsizeof(a)))
+
/*
* Signed integer overflow is undefined in C, so here's a helper macro
* to detect if the sum of two integers will overflow.
@@ -40,6 +43,9 @@
#define signed_add_overflows(a, b) \
((b) > maximum_signed_value_of_type(a) - (a))
+#define unsigned_add_overflows(a, b) \
+ ((b) > maximum_unsigned_value_of_type(a) - (a))
+
#ifdef __GNUC__
#define TYPEOF(x) (__typeof__(x))
#else
diff --git a/patch-delta.c b/patch-delta.c
index d218faa..56e0a5e 100644
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -48,7 +48,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size,
if (cmd & 0x20) cp_size |= (*data++ << 8);
if (cmd & 0x40) cp_size |= (*data++ << 16);
if (cp_size == 0) cp_size = 0x10000;
- if (cp_off + cp_size < cp_size ||
+ if (unsigned_add_overflows(cp_off, cp_size) ||
cp_off + cp_size > src_size ||
cp_size > size)
break;
diff --git a/strbuf.c b/strbuf.c
index 9b3c445..07e8883 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -63,7 +63,8 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc)
void strbuf_grow(struct strbuf *sb, size_t extra)
{
- if (sb->len + extra + 1 <= sb->len)
+ if (unsigned_add_overflows(extra, 1) ||
+ unsigned_add_overflows(sb->len, extra + 1))
die("you want to use way too much memory");
if (!sb->alloc)
sb->buf = NULL;
@@ -152,7 +153,7 @@ int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
const void *data, size_t dlen)
{
- if (pos + len < pos)
+ if (unsigned_add_overflows(pos, len))
die("you want to use way too much memory");
if (pos > sb->len)
die("`pos' is too far after the end of the buffer");
diff --git a/wrapper.c b/wrapper.c
index 55b074e..4c147d6 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -53,7 +53,7 @@ void *xmalloc(size_t size)
void *xmallocz(size_t size)
{
void *ret;
- if (size + 1 < size)
+ if (unsigned_add_overflows(size, 1))
die("Data too large to fit into virtual memory space.");
ret = xmalloc(size + 1);
((char*)ret)[size] = 0;
--
1.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] compat: helper for detecting unsigned overflow
2011-02-10 9:35 ` [PATCH] compat: helper for detecting unsigned overflow Jonathan Nieder
@ 2011-02-10 12:11 ` Sverre Rabbelier
2011-02-10 13:23 ` Joshua Juran
0 siblings, 1 reply; 5+ messages in thread
From: Sverre Rabbelier @ 2011-02-10 12:11 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Erik Faye-Lund, Jeff King, Nicolas Pitre, Pierre Habouzit,
Ilari Liusvaara
Heya,
On Thu, Feb 10, 2011 at 10:35, Jonathan Nieder <jrnieder@gmail.com> wrote:
> unsigned_add_overflows(a, b)
> Define such a macro, expanding roughly to ((a) < UINT_MAX - (b)).
> Because the expansion uses each argument only once outside of sizeof()
> expressions, it is safe to use with arguments that have side effects.
> +#define unsigned_add_overflows(a, b) \
> + ((b) > maximum_unsigned_value_of_type(a) - (a))
I'm confused, you say you won't use it twice, and then you do use it twice?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] compat: helper for detecting unsigned overflow
2011-02-10 12:11 ` Sverre Rabbelier
@ 2011-02-10 13:23 ` Joshua Juran
0 siblings, 0 replies; 5+ messages in thread
From: Joshua Juran @ 2011-02-10 13:23 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Jonathan Nieder, git, Erik Faye-Lund, Jeff King, Nicolas Pitre,
Pierre Habouzit, Ilari Liusvaara
On Feb 10, 2011, at 4:11 AM, Sverre Rabbelier wrote:
> On Thu, Feb 10, 2011 at 10:35, Jonathan Nieder <jrnieder@gmail.com>
> wrote:
>> unsigned_add_overflows(a, b)
>
>> Define such a macro, expanding roughly to ((a) < UINT_MAX - (b)).
>> Because the expansion uses each argument only once outside of
>> sizeof()
>> expressions, it is safe to use with arguments that have side effects.
>
>> +#define unsigned_add_overflows(a, b) \
>> + ((b) > maximum_unsigned_value_of_type(a) - (a))
>
> I'm confused, you say you won't use it twice, and then you do use it
> twice?
The author is asserting that maximum_unsigned_value_of_type() is a
function of sizeof a, which would have no runtime effect.
Josh
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-02-10 13:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-05 7:24 [PATCH v4] do not depend on signed integer overflow Erik Faye-Lund
2010-10-05 13:28 ` Nicolas Pitre
2011-02-10 9:35 ` [PATCH] compat: helper for detecting unsigned overflow Jonathan Nieder
2011-02-10 12:11 ` Sverre Rabbelier
2011-02-10 13:23 ` Joshua Juran
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.