On 12/15/2016 02:09 AM, Bruno Haible wrote:
So, the limiting factor is the pointer difference operator
ptr1 - ptr2 where sizeof (*ptr1,*ptr2) > 1.
Yes, it is the pointer difference operator. However, the problem occurs
even with size-1 array elements. For example:
#include <stdint.h>
#include <stddef.h>
#include <stdlib.h>
ptrdiff_t
diff (char *a, char *b)
{
return a - b;
}
int
main (void)
{
size_t n = PTRDIFF_MAX / 2 + 1;
size_t size = 2 * n;
char *x = malloc (size);
return 0 < diff (x + size, x);
}
'main' returns 0 on Fedora 24 (x86-64 or x86).
* We have no problem with code that only works with indices and never does
pointer differences or pointer comparisons.
I don't see a problem with pointer comparisons, just pointer differences.
* We have no problem with strings, because sizeof (char) == 1.
No, unfortunately large strings do not work, as one cannot reliably
compute differences of pointers to their elements.
One possibility would be to have two flavors of xalloc_oversized. One
flavor would check for both ptrdiff_t overflow and size_t overflow, for
programs that do pointer subtraction, and the other flavor
(yalloc_oversized, say?) would check only for size_t overflow, for
programs that never subtract pointers to the allocated storage. All
current functions like xnmalloc could have two flavors, so that xnmalloc
checks for both kinds of overflow and ynmalloc checks only for size_t
overflow. It's not clear to me whether it's worth going to all that
effort merely to support 3 GiB arrays in 32-bit applications. In the
meantime, I installed the patch I proposed yesterday, along with the
additional patches attached, which merely change the x* functions to
check for both kinds of overflow.
From 7c7c9a8c9fda03b8973fbc866bf6171cab97a8bb Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 15 Dec 2016 09:53:45 -0800
Subject: [PATCH 1/4] quotearg: pacify GCC better
* modules/quotearg (Depends-on): Add minmax, stdint.
* lib/quotearg.c: Include minmax.h, stdint.h.
(nslots): Now int, as there seems little point to going to extra
work merely to support the INT_MAX slot, which nobody ever uses.
(quotearg_n_options): Redo size-overflow checks to pacify GCC
and to catch (mostly-theoretical) ptrdiff_t problems too.
This can be done via one comparison.
---
ChangeLog | 11 +++++++++++
lib/quotearg.c | 23 +++++++++--------------
modules/quotearg | 2 ++
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 767e0c7..612d66c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2016-12-15 Paul Eggert <eggert@cs.ucla.edu>
+
+ quotearg: pacify GCC better
+ * modules/quotearg (Depends-on): Add minmax, stdint.
+ * lib/quotearg.c: Include minmax.h, stdint.h.
+ (nslots): Now int, as there seems little point to going to extra
+ work merely to support the INT_MAX slot, which nobody ever uses.
+ (quotearg_n_options): Redo size-overflow checks to pacify GCC
+ and to catch (mostly-theoretical) ptrdiff_t problems too.
+ This can be done via one comparison.
+
2016-12-14 Paul Eggert <eggert@cs.ucla.edu>
xalloc-oversized: check for PTRDIFF_MAX too
diff --git a/lib/quotearg.c b/lib/quotearg.c
index 730b4b3..07658b2 100644
--- a/lib/quotearg.c
+++ b/lib/quotearg.c
@@ -29,6 +29,7 @@
#include "quotearg.h"
#include "quote.h"
+#include "minmax.h"
#include "xalloc.h"
#include "c-strcaseeq.h"
#include "localcharset.h"
@@ -37,6 +38,7 @@
#include <errno.h>
#include <limits.h>
#include <stdbool.h>
+#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <wchar.h>
@@ -830,7 +832,7 @@ struct slotvec
/* Preallocate a slot 0 buffer, so that the caller can always quote
one small component of a "memory exhausted" message in slot 0. */
static char slot0[256];
-static unsigned int nslots = 1;
+static int nslots = 1;
static struct slotvec slotvec0 = {sizeof slot0, slot0};
static struct slotvec *slotvec = &slotvec0;
@@ -838,7 +840,7 @@ void
quotearg_free (void)
{
struct slotvec *sv = slotvec;
- unsigned int i;
+ int i;
for (i = 1; i < nslots; i++)
free (sv[i].val);
if (sv[0].val != slot0)
@@ -869,30 +871,23 @@ quotearg_n_options (int n, char const *arg, size_t argsize,
{
int e = errno;
- unsigned int n0 = n;
struct slotvec *sv = slotvec;
if (n < 0)
abort ();
- if (nslots <= n0)
+ if (nslots <= n)
{
- /* FIXME: technically, the type of n1 should be 'unsigned int',
- but that evokes an unsuppressible warning from gcc-4.0.1 and
- older. If gcc ever provides an option to suppress that warning,
- revert to the original type, so that the test in xalloc_oversized
- is once again performed only at compile time. */
- size_t n1 = n0 + 1;
bool preallocated = (sv == &slotvec0);
- if (xalloc_oversized (n1, sizeof *sv))
+ if (MIN (INT_MAX, MIN (PTRDIFF_MAX, SIZE_MAX) / sizeof *sv) <= n)
xalloc_die ();
- slotvec = sv = xrealloc (preallocated ? NULL : sv, n1 * sizeof *sv);
+ slotvec = sv = xrealloc (preallocated ? NULL : sv, (n + 1) * sizeof *sv);
if (preallocated)
*sv = slotvec0;
- memset (sv + nslots, 0, (n1 - nslots) * sizeof *sv);
- nslots = n1;
+ memset (sv + nslots, 0, (n + 1 - nslots) * sizeof *sv);
+ nslots = n + 1;
}
{
diff --git a/modules/quotearg b/modules/quotearg
index bd63761..109a16d 100644
--- a/modules/quotearg
+++ b/modules/quotearg
@@ -16,9 +16,11 @@ gettext-h
mbrtowc
mbsinit
memcmp
+minmax
quotearg-simple
localcharset
stdbool
+stdint
wchar
wctype-h
xalloc
--
2.7.4
From 83c7f0582c5111ce027f74e50fc7f1e813db02e1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 15 Dec 2016 09:56:53 -0800
Subject: [PATCH 2/4] malloca: do not exceed PTRDIFF_MAX
* lib/malloca.h: Include xalloc-oversized.
(nmalloca): Use xalloc_oversized instead of rolling our own.
* modules/malloca (Depends-on):
* modules/relocatable-prog-wrapper (Depends-on):
Add xalloc-oversized.
---
ChangeLog | 7 +++++++
lib/malloca.h | 13 ++++---------
modules/malloca | 1 +
modules/relocatable-prog-wrapper | 1 +
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 612d66c..07c4d65 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2016-12-15 Paul Eggert <eggert@cs.ucla.edu>
+ malloca: do not exceed PTRDIFF_MAX
+ * lib/malloca.h: Include xalloc-oversized.
+ (nmalloca): Use xalloc_oversized instead of rolling our own.
+ * modules/malloca (Depends-on):
+ * modules/relocatable-prog-wrapper (Depends-on):
+ Add xalloc-oversized.
+
quotearg: pacify GCC better
* modules/quotearg (Depends-on): Add minmax, stdint.
* lib/quotearg.c: Include minmax.h, stdint.h.
diff --git a/lib/malloca.h b/lib/malloca.h
index 46f27f0..c00dbed 100644
--- a/lib/malloca.h
+++ b/lib/malloca.h
@@ -21,6 +21,9 @@
#include <alloca.h>
#include <stddef.h>
#include <stdlib.h>
+#include <stdint.h>
+
+#include "xalloc-oversized.h"
#ifdef __cplusplus
@@ -73,15 +76,7 @@ extern void freea (void *p);
It allocates an array of N objects, each with S bytes of memory,
on the stack. S must be positive and N must be nonnegative.
The array must be freed using freea() before the function returns. */
-#if 1
-/* Cf. the definition of xalloc_oversized. */
-# define nmalloca(n, s) \
- ((n) > (size_t) (sizeof (ptrdiff_t) <= sizeof (size_t) ? -1 : -2) / (s) \
- ? NULL \
- : malloca ((n) * (s)))
-#else
-extern void * nmalloca (size_t n, size_t s);
-#endif
+#define nmalloca(n, s) (xalloc_oversized (n, s) ? NULL : malloca ((n) * (s)))
#ifdef __cplusplus
diff --git a/modules/malloca b/modules/malloca
index ca0be89..9d4b40b 100644
--- a/modules/malloca
+++ b/modules/malloca
@@ -12,6 +12,7 @@ m4/longlong.m4
Depends-on:
alloca-opt
stdint
+xalloc-oversized
verify
configure.ac:
diff --git a/modules/relocatable-prog-wrapper b/modules/relocatable-prog-wrapper
index 50444b1..db451ef 100644
--- a/modules/relocatable-prog-wrapper
+++ b/modules/relocatable-prog-wrapper
@@ -47,6 +47,7 @@ unistd
environ
string
verify
+xalloc-oversized
configure.ac:
gl_FUNC_READLINK_SEPARATE
--
2.7.4
From b2d30e9c7649397463aed49f12cc493c93afbd45 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 15 Dec 2016 09:59:21 -0800
Subject: [PATCH 3/4] xalloc: do not exceed PTRDIFF_MAX
* lib/xmalloc.c (xcalloc) [HAVE_GNU_CALLOC]: Do not omit
xalloc_oversized check, since objects larger than PTRDIFF_MAX
bytes have pointer-subtraction problems.
---
ChangeLog | 5 +++++
lib/xmalloc.c | 10 +++++-----
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 07c4d65..bee9a17 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2016-12-15 Paul Eggert <eggert@cs.ucla.edu>
+ xalloc: do not exceed PTRDIFF_MAX
+ * lib/xmalloc.c (xcalloc) [HAVE_GNU_CALLOC]: Do not omit
+ xalloc_oversized check, since objects larger than PTRDIFF_MAX
+ bytes have pointer-subtraction problems.
+
malloca: do not exceed PTRDIFF_MAX
* lib/malloca.h: Include xalloc-oversized.
(nmalloca): Use xalloc_oversized instead of rolling our own.
diff --git a/lib/xmalloc.c b/lib/xmalloc.c
index 429b50d..7d9c077 100644
--- a/lib/xmalloc.c
+++ b/lib/xmalloc.c
@@ -93,11 +93,11 @@ void *
xcalloc (size_t n, size_t s)
{
void *p;
- /* Test for overflow, since some calloc implementations don't have
- proper overflow checks. But omit overflow and size-zero tests if
- HAVE_GNU_CALLOC, since GNU calloc catches overflow and never
- returns NULL if successful. */
- if ((! HAVE_GNU_CALLOC && xalloc_oversized (n, s))
+ /* Test for overflow, since objects with size greater than
+ PTRDIFF_MAX cause pointer subtraction to go awry. Omit size-zero
+ tests if HAVE_GNU_CALLOC, since GNU calloc never returns NULL if
+ successful. */
+ if (xalloc_oversized (n, s)
|| (! (p = calloc (n, s)) && (HAVE_GNU_CALLOC || n != 0)))
xalloc_die ();
return p;
--
2.7.4
From bae65427b398824d58ef55bbc14b296ddaf22921 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 15 Dec 2016 10:00:22 -0800
Subject: [PATCH 4/4] safe-alloc: use xalloc-oversized
* lib/safe-alloc.c: Include xalloc-oversized.h.
(safe_alloc_oversized): Remove. All uses changed to xalloc_oversized.
* modules/safe-alloc (Depends-on): Add xalloc-oversized.
---
ChangeLog | 5 +++++
lib/safe-alloc.c | 25 ++++---------------------
modules/safe-alloc | 3 +++
3 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index bee9a17..5e9e801 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2016-12-15 Paul Eggert <eggert@cs.ucla.edu>
+ safe-alloc: use xalloc-oversized
+ * lib/safe-alloc.c: Include xalloc-oversized.h.
+ (safe_alloc_oversized): Remove. All uses changed to xalloc_oversized.
+ * modules/safe-alloc (Depends-on): Add xalloc-oversized.
+
xalloc: do not exceed PTRDIFF_MAX
* lib/xmalloc.c (xcalloc) [HAVE_GNU_CALLOC]: Do not omit
xalloc_oversized check, since objects larger than PTRDIFF_MAX
diff --git a/lib/safe-alloc.c b/lib/safe-alloc.c
index 7b2acf2..0f9e5a6 100644
--- a/lib/safe-alloc.c
+++ b/lib/safe-alloc.c
@@ -22,30 +22,13 @@
/* Specification. */
#include "safe-alloc.h"
+#include "xalloc-oversized.h"
+
#include <stdlib.h>
#include <stddef.h>
#include <errno.h>
-/* Return 1 if an array of N objects, each of size S, cannot exist due
- to size arithmetic overflow. S must be positive and N must be
- nonnegative. This is a macro, not a function, so that it
- works correctly even when SIZE_MAX < N.
-
- By gnulib convention, SIZE_MAX represents overflow in size
- calculations, so the conservative dividend to use here is
- SIZE_MAX - 1, since SIZE_MAX might represent an overflowed value.
- However, malloc (SIZE_MAX) fails on all known hosts where
- sizeof (ptrdiff_t) <= sizeof (size_t), so do not bother to test for
- exactly-SIZE_MAX allocations on such hosts; this avoids a test and
- branch when S is known to be 1.
-
- This is the same as xalloc_oversized from xalloc.h
-*/
-#define safe_alloc_oversized(n, s) \
- ((size_t) (sizeof (ptrdiff_t) <= sizeof (size_t) ? -1 : -2) / (s) < (n))
-
-
/**
* safe_alloc_alloc_n:
* @ptrptr: pointer to pointer for address of allocated memory
@@ -68,7 +51,7 @@ safe_alloc_alloc_n (void *ptrptr, size_t size, size_t count, int zeroed)
return 0;
}
- if (safe_alloc_oversized (count, size))
+ if (xalloc_oversized (count, size))
{
errno = ENOMEM;
return -1;
@@ -108,7 +91,7 @@ safe_alloc_realloc_n (void *ptrptr, size_t size, size_t count)
*(void **) ptrptr = NULL;
return 0;
}
- if (safe_alloc_oversized (count, size))
+ if (xalloc_oversized (count, size))
{
errno = ENOMEM;
return -1;
diff --git a/modules/safe-alloc b/modules/safe-alloc
index 94f799f..13e78de 100644
--- a/modules/safe-alloc
+++ b/modules/safe-alloc
@@ -6,6 +6,9 @@ lib/safe-alloc.h
lib/safe-alloc.c
m4/safe-alloc.m4
+Depends-on:
+xalloc-oversized
+
configure.ac:
gl_SAFE_ALLOC
--
2.7.4
