* ptrdiff_t overflow checks for malloc-posix etc.
@ 2021-04-18 2:02 Paul Eggert
2021-04-18 11:59 ` Bruno Haible
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Paul Eggert @ 2021-04-18 2:02 UTC (permalink / raw)
To: Gnulib bugs
[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]
I installed the attached patches into Gnulib to make its malloc
replacements ptrdiff_t safe. This should help us move in a direction
where we can use idx_t (which is signed and therefore safer) for sizes
and indexes, instead of using size_t.
In creating these patches I found a reasonable amount of cruft of which
I tried to clean up some (see ChangeLog entry). If I went too far please
let me know and I’ll work to unclean it.
I initially attempted to come up with new modules malloc-ptrdiff_t, etc.
but ran into complexity issues with all the possible combinations the
various malloc modules. So instead, I simply added the fixes to
malloc-posix, realloc-posix, and realloc-posix, where they will
automatically percolate into malloc-gnu etc.
Come to think of it, why do we have both malloc-gnu and malloc-posix
modules (and similarly for calloc and realloc)? Was it because GNU
realloc was incompatible with C99 realloc, so we needed realloc-gnu vs
realloc-posix modules? If so, I suggest that we stop worrying about it,
as that worry is now obsolete - C17 allows the GNU behavior.
In other words, I suggest that we remove malloc-posix, realloc-posix and
calloc-posix, or failing that simply make them obsolete compatibility
aliases for malloc-gnu etc. This would simplify the configuration of
malloc-using code, and any runtime cost would surely be insignificant
(and would occur only on older or non-GNU hosts).
The first attached patch does the heavy lifting; the second shows how
the xalloc module can be simplified because of the malloc etc. fixes.
Other simplifications are possible elsewhere; one step at a time.
[-- Attachment #2: 0001-malloc-etc.-check-for-ptrdiff_t-overflow.patch --]
[-- Type: text/x-patch, Size: 31145 bytes --]
From 1ca8ecafc9066c9c0fb4b1030088969a11dce3af Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 17 Apr 2021 18:44:25 -0700
Subject: [PATCH 1/2] malloc, etc.: check for ptrdiff_t overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
In glibc 2.30 and later, malloc, realloc and calloc reject
attempts to create objects larger than PTRDIFF_MAX bytes.
This patch changes malloc-gnu etc. to support this behavior
on non-GNU hosts. It also makes this change for malloc-posix etc.
since it’s a safety measure that ought to be in POSIX (perhaps
we can talk them into that...).
In writing this patch I found a complicated set of code that had
accumulated over the years, some written by yours truly. I got
rid of the code I couldn’t see the need for nowadays. Among other
things, the GNU realloc behavior is no longer incompatible with
the C standard, because in C17 the latter was relaxed to allow the
former. If I went too far in cleaning up, the old stuff can be
resurrected.
This change is mostly for 32-bit platforms, since practical 64-bit
platforms cannot create objects larger than PTRDIFF_MAX bytes anyway.
* doc/posix-functions/calloc.texi:
* doc/posix-functions/malloc.texi:
* doc/posix-functions/realloc.texi:
Mention ptrdiff_t issues, and go into more detail about what
the gnu extension module does.
* doc/posix-functions/realloc.texi: Fix now-obsolete commentary
about C99 vs glibc, as C17 allows the glibc behavior and POSIX
will follow suit when it gets around to it.
* lib/calloc.c, lib/malloc.c, lib/realloc.c:
Simplify by always supplying a GNU-compatible version,
as that suffices for correctness and is good enough for performance.
Include xalloc-oversized.h, and use xalloc_oversized to
check for ptrdiff_t overflow.
(NEED_CALLOC_GNU, NEED_MALLOC_GNU, NEED_REALLOC_GNU): Remove.
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF):
* m4/malloc.m4 (_AC_FUNC_MALLOC_IF):
* m4/realloc.m4 (_AC_FUNC_REALLOC_IF):
Don’t start with a newline. Fix message to match behavior.
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF): Don’t test for size_t overflow,
as the ptrdiff_t test is good enough.
* m4/calloc.m4 (gl_FUNC_CALLOC_GNU):
* m4/malloc.m4 (gl_FUNC_MALLOC_GNU):
* m4/realloc.m4 (gl_FUNC_REALLOC_GNU):
Do not define HAVE_CALLOC_GNU, HAVE_MALLOC_GNU, HAVE_REALLOC_GNU.
It’s not worth the aggravation of maintaining these, as they
are confusing (they don’t really mean GNU-compatible anyway).
Don’t bother testing for GNU behavior if we have already decided
to replace the function, since the replacement is always GNUish.
* m4/calloc.m4 (gl_FUNC_CALLOC_POSIX):
* m4/realloc.m4 (gl_FUNC_REALLOC_POSIX):
Defer to gl_FUNC_MALLOC_POSIX.
* m4/malloc.m4 (gl_FUNC_MALLOC_PTRDIFF, gl_CHECK_MALLOC_PTRDIFF):
New macros.
(gl_FUNC_MALLOC_POSIX): Use them to check for ptrdiff_t overflow.
* modules/calloc-gnu, modules/malloc-gnu, modules/realloc-gnu:
Remove no-longer-needed module indicators.
* modules/calloc-posix, modules/malloc-posix, modules/realloc-posix:
Depend on xalloc-oversized.
* modules/malloc-posix: Require gl_FUNC_MALLOC_POSIX instead of
calling it directly, so that other code can require it.
* modules/realloc-posix: Depend on free-posix and malloc-posix.
---
ChangeLog | 62 +++++++++++++++++++++++++
doc/posix-functions/calloc.texi | 17 +++++--
doc/posix-functions/malloc.texi | 17 ++++---
doc/posix-functions/realloc.texi | 37 ++++++++++-----
lib/calloc.c | 37 +++++----------
lib/malloc.c | 32 +++++--------
lib/realloc.c | 55 +++++++---------------
m4/calloc.m4 | 67 +++++++--------------------
m4/malloc.m4 | 79 ++++++++++++++++++++++++--------
m4/realloc.m4 | 32 ++++---------
modules/calloc-gnu | 1 -
modules/calloc-posix | 1 +
modules/malloc-gnu | 1 -
modules/malloc-posix | 4 +-
modules/realloc-gnu | 1 -
modules/realloc-posix | 4 +-
16 files changed, 243 insertions(+), 204 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 1e4c95524..825201fe2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,65 @@
+2021-04-17 Paul Eggert <eggert@cs.ucla.edu>
+
+ malloc, etc.: check for ptrdiff_t overflow
+ In glibc 2.30 and later, malloc, realloc and calloc reject
+ attempts to create objects larger than PTRDIFF_MAX bytes.
+ This patch changes malloc-gnu etc. to support this behavior
+ on non-GNU hosts. It also makes this change for malloc-posix etc.
+ since it’s a safety measure that ought to be in POSIX (perhaps
+ we can talk them into that...).
+
+ In writing this patch I found a complicated set of code that had
+ accumulated over the years, some written by yours truly. I got
+ rid of the code I couldn’t see the need for nowadays. Among other
+ things, the GNU realloc behavior is no longer incompatible with
+ the C standard, because in C17 the latter was relaxed to allow the
+ former. If I went too far in cleaning up, the old stuff can be
+ resurrected.
+
+ This change is mostly for 32-bit platforms, since practical 64-bit
+ platforms cannot create objects larger than PTRDIFF_MAX bytes anyway.
+ * doc/posix-functions/calloc.texi:
+ * doc/posix-functions/malloc.texi:
+ * doc/posix-functions/realloc.texi:
+ Mention ptrdiff_t issues, and go into more detail about what
+ the gnu extension module does.
+ * doc/posix-functions/realloc.texi: Fix now-obsolete commentary
+ about C99 vs glibc, as C17 allows the glibc behavior and POSIX
+ will follow suit when it gets around to it.
+ * lib/calloc.c, lib/malloc.c, lib/realloc.c:
+ Simplify by always supplying a GNU-compatible version,
+ as that suffices for correctness and is good enough for performance.
+ Include xalloc-oversized.h, and use xalloc_oversized to
+ check for ptrdiff_t overflow.
+ (NEED_CALLOC_GNU, NEED_MALLOC_GNU, NEED_REALLOC_GNU): Remove.
+ * m4/calloc.m4 (_AC_FUNC_CALLOC_IF):
+ * m4/malloc.m4 (_AC_FUNC_MALLOC_IF):
+ * m4/realloc.m4 (_AC_FUNC_REALLOC_IF):
+ Don’t start with a newline. Fix message to match behavior.
+ * m4/calloc.m4 (_AC_FUNC_CALLOC_IF): Don’t test for size_t overflow,
+ as the ptrdiff_t test is good enough.
+ * m4/calloc.m4 (gl_FUNC_CALLOC_GNU):
+ * m4/malloc.m4 (gl_FUNC_MALLOC_GNU):
+ * m4/realloc.m4 (gl_FUNC_REALLOC_GNU):
+ Do not define HAVE_CALLOC_GNU, HAVE_MALLOC_GNU, HAVE_REALLOC_GNU.
+ It’s not worth the aggravation of maintaining these, as they
+ are confusing (they don’t really mean GNU-compatible anyway).
+ Don’t bother testing for GNU behavior if we have already decided
+ to replace the function, since the replacement is always GNUish.
+ * m4/calloc.m4 (gl_FUNC_CALLOC_POSIX):
+ * m4/realloc.m4 (gl_FUNC_REALLOC_POSIX):
+ Defer to gl_FUNC_MALLOC_POSIX.
+ * m4/malloc.m4 (gl_FUNC_MALLOC_PTRDIFF, gl_CHECK_MALLOC_PTRDIFF):
+ New macros.
+ (gl_FUNC_MALLOC_POSIX): Use them to check for ptrdiff_t overflow.
+ * modules/calloc-gnu, modules/malloc-gnu, modules/realloc-gnu:
+ Remove no-longer-needed module indicators.
+ * modules/calloc-posix, modules/malloc-posix, modules/realloc-posix:
+ Depend on xalloc-oversized.
+ * modules/malloc-posix: Require gl_FUNC_MALLOC_POSIX instead of
+ calling it directly, so that other code can require it.
+ * modules/realloc-posix: Depend on free-posix and malloc-posix.
+
2021-04-17 Bruno Haible <bruno@clisp.org>
stdio: Fix build error in some configurations (regression 2021-04-11).
diff --git a/doc/posix-functions/calloc.texi b/doc/posix-functions/calloc.texi
index 22c51be02..9ba40c0d4 100644
--- a/doc/posix-functions/calloc.texi
+++ b/doc/posix-functions/calloc.texi
@@ -12,11 +12,22 @@ Portability problems fixed by Gnulib:
Upon failure, the function does not set @code{errno} to @code{ENOMEM} on
some platforms:
mingw, MSVC 14.
-@end itemize
-Portability problems not fixed by Gnulib:
-@itemize
+@item
+On some platforms, @code{calloc (n, s)} can succeed even if
+multiplying @code{n} by @code{s} would exceed @code{PTRDIFF_MAX} or
+@code{SIZE_MAX}. Although failing to check for exceeding
+@code{PTRDIFF_MAX} is arguably allowed by POSIX it can lead to
+undefined behavior later, so @code{calloc-posix} does not allow
+going over the limit.
@end itemize
Extension: Gnulib provides a module @samp{calloc-gnu} that substitutes a
@code{calloc} implementation that behaves more like the glibc implementation.
+It fixes this portability problem:
+
+@itemize
+@item
+On some platforms, @code{calloc (0, s)} and @code{calloc (n, 0)}
+return @code{NULL} on success.
+@end itemize
diff --git a/doc/posix-functions/malloc.texi b/doc/posix-functions/malloc.texi
index 6d5995078..829517311 100644
--- a/doc/posix-functions/malloc.texi
+++ b/doc/posix-functions/malloc.texi
@@ -12,14 +12,19 @@ Portability problems fixed by Gnulib:
Upon failure, the function does not set @code{errno} to @code{ENOMEM} on
some platforms:
mingw, MSVC 14.
-@end itemize
-Portability problems not fixed by Gnulib:
-@itemize
-@item @code{malloc (0)} always returns a NULL pointer on some platforms:
-AIX 5.1.
+@item
+On some platforms, @code{malloc (n)} can succeed even if @code{n}
+exceeds @code{PTRDIFF_MAX}. Although this behavior is arguably
+allowed by POSIX it can lead to behavior not defined by POSIX later,
+so @code{malloc-posix} does not allow going over the limit.
@end itemize
Extension: Gnulib provides a module @samp{malloc-gnu} that substitutes a
@code{malloc} implementation that behaves more like the glibc implementation,
-regarding the result of @code{malloc (0)}.
+by fixing this portability problem:
+
+@itemize
+@item
+On some platforms, @code{malloc (0)} returns @code{NULL} on success.
+@end itemize
diff --git a/doc/posix-functions/realloc.texi b/doc/posix-functions/realloc.texi
index 1f0b18e95..282e36051 100644
--- a/doc/posix-functions/realloc.texi
+++ b/doc/posix-functions/realloc.texi
@@ -12,24 +12,37 @@ Portability problems fixed by Gnulib:
Upon failure, the function does not set @code{errno} to @code{ENOMEM} on
some platforms:
mingw, MSVC 14.
-@end itemize
-Portability problems not fixed by Gnulib:
-@itemize
@item
-It is not portable to call @code{realloc} with a size of 0. With a
+On some platforms, @code{realloc (p, n)} can succeed even if @code{n}
+exceeds @code{PTRDIFF_MAX}. Although this behavior is arguably
+allowed by POSIX it can lead to behavior not defined by POSIX later,
+so @code{realloc-posix} does not allow going over the limit.
+@end itemize
+
+Without the @samp{realloc-gnu} module described below, it is not portable
+to call @code{realloc} with a size of 0. With a
NULL pointer argument, this is the same ambiguity as @code{malloc (0)}
on whether a unique zero-size object is created. With a non-NULL
-pointer argument, C99 requires that if @code{realloc (p, 0)} returns
-@code{NULL} then @code{p} is still valid. Among implementations that
-obey C99, behavior varies on whether @code{realloc (p, 0)} always
+pointer argument @code{p}, C17 says that it is implementation-defined
+whether @code{realloc (p, 0)} frees @code{p}.
+Behavior varies on whether @code{realloc (p, 0)} always frees @code{p}
+and successfully returns a null pointer, or always
fails and leaves @code{p} valid, or usually succeeds and returns a
-unique zero-size object; either way, a program not suspecting these
+unique zero-size object; a program not suspecting these variations in
semantics will leak memory (either the still-valid @code{p}, or the
-non-NULL return value). Meanwhile, several implementations violate
-C99, by always calling @code{free (p)} but returning NULL:
-glibc, Cygwin
-@end itemize
+non-NULL return value).
Extension: Gnulib provides a module @samp{realloc-gnu} that substitutes a
@code{realloc} implementation that behaves more like the glibc implementation.
+It fixes these portability problems:
+
+@itemize
+@item
+On some platforms, @code{realloc (NULL, 0)} returns @code{NULL} on success.
+
+@item
+On some platforms, @code{realloc (p, 0)} with non-null @code{p}
+might not free @code{p}, or might clobber @code{errno},
+or might not return @code{NULL}.
+@end itemize
diff --git a/lib/calloc.c b/lib/calloc.c
index 5ab1975cf..c9da08047 100644
--- a/lib/calloc.c
+++ b/lib/calloc.c
@@ -19,49 +19,34 @@
#include <config.h>
-/* The gnulib module 'calloc-gnu' defines HAVE_CALLOC_GNU. */
-#if GNULIB_CALLOC_GNU && !HAVE_CALLOC_GNU
-# define NEED_CALLOC_GNU 1
-#endif
-
/* Specification. */
#include <stdlib.h>
#include <errno.h>
+#include "xalloc-oversized.h"
+
/* Call the system's calloc below. */
#undef calloc
-/* Allocate and zero-fill an NxS-byte block of memory from the heap.
- If N or S is zero, allocate and zero-fill a 1-byte block. */
+/* Allocate and zero-fill an NxS-byte block of memory from the heap,
+ even if N or S is zero. */
void *
rpl_calloc (size_t n, size_t s)
{
- void *result;
-
-#if NEED_CALLOC_GNU
if (n == 0 || s == 0)
+ n = s = 1;
+
+ if (xalloc_oversized (n, s))
{
- n = 1;
- s = 1;
- }
- else
- {
- /* Defend against buggy calloc implementations that mishandle
- size_t overflow. */
- size_t bytes = n * s;
- if (bytes / s != n)
- {
- errno = ENOMEM;
- return NULL;
- }
+ errno = ENOMEM;
+ return NULL;
}
-#endif
- result = calloc (n, s);
+ void *result = calloc (n, s);
-#if !HAVE_CALLOC_POSIX
+#if !HAVE_MALLOC_POSIX
if (result == NULL)
errno = ENOMEM;
#endif
diff --git a/lib/malloc.c b/lib/malloc.c
index 272b8b9f4..a900046ec 100644
--- a/lib/malloc.c
+++ b/lib/malloc.c
@@ -20,43 +20,35 @@
#define _GL_USE_STDLIB_ALLOC 1
#include <config.h>
-/* The gnulib module 'malloc-gnu' defines HAVE_MALLOC_GNU. */
-#if GNULIB_MALLOC_GNU && !HAVE_MALLOC_GNU
-# define NEED_MALLOC_GNU 1
-#endif
-
#include <stdlib.h>
-/* A function definition is only needed if NEED_MALLOC_GNU is defined above
- or if the module 'malloc-posix' requests it. */
-#if NEED_MALLOC_GNU || (GNULIB_MALLOC_POSIX && !HAVE_MALLOC_POSIX)
+#include <errno.h>
-# include <errno.h>
+#include "xalloc-oversized.h"
/* Call the system's malloc below. */
# undef malloc
-/* Allocate an N-byte block of memory from the heap.
- If N is zero, allocate a 1-byte block. */
+/* Allocate an N-byte block of memory from the heap, even if N is 0. */
void *
rpl_malloc (size_t n)
{
- void *result;
-
-# if NEED_MALLOC_GNU
if (n == 0)
n = 1;
-# endif
- result = malloc (n);
+ if (xalloc_oversized (n, 1))
+ {
+ errno = ENOMEM;
+ return NULL;
+ }
+
+ void *result = malloc (n);
-# if !HAVE_MALLOC_POSIX
+#if !HAVE_MALLOC_POSIX
if (result == NULL)
errno = ENOMEM;
-# endif
+#endif
return result;
}
-
-#endif
diff --git a/lib/realloc.c b/lib/realloc.c
index c3e3cdfc5..c0d94b439 100644
--- a/lib/realloc.c
+++ b/lib/realloc.c
@@ -21,66 +21,43 @@
#define _GL_USE_STDLIB_ALLOC 1
#include <config.h>
-/* The gnulib module 'realloc-gnu' defines HAVE_REALLOC_GNU. */
-#if GNULIB_REALLOC_GNU && !HAVE_REALLOC_GNU
-# define NEED_REALLOC_GNU 1
-#endif
-
-/* Infer the properties of the system's malloc function.
- The gnulib module 'malloc-gnu' defines HAVE_MALLOC_GNU. */
-#if GNULIB_MALLOC_GNU && HAVE_MALLOC_GNU
-# define SYSTEM_MALLOC_GLIBC_COMPATIBLE 1
-#endif
-
#include <stdlib.h>
-/* A function definition is only needed if NEED_REALLOC_GNU is defined above
- or if the module 'realloc-posix' requests it. */
-#if NEED_REALLOC_GNU || (GNULIB_REALLOC_POSIX && !HAVE_REALLOC_POSIX)
+#include <errno.h>
-# include <errno.h>
+#include "xalloc-oversized.h"
-/* Call the system's malloc and realloc below. */
-# undef malloc
-# undef realloc
+/* Call the system's realloc below. */
+#undef realloc
/* Change the size of an allocated block of memory P to N bytes,
- with error checking. If N is zero, change it to 1. If P is NULL,
- use malloc. */
+ with error checking. If P is NULL, use malloc. Otherwise if N is zero,
+ free P and return NULL. */
void *
rpl_realloc (void *p, size_t n)
{
- void *result;
+ if (p == NULL)
+ return malloc (n);
-# if NEED_REALLOC_GNU
if (n == 0)
{
- n = 1;
-
- /* In theory realloc might fail, so don't rely on it to free. */
free (p);
- p = NULL;
+ return NULL;
}
-# endif
- if (p == NULL)
+ if (xalloc_oversized (n, 1))
{
-# if GNULIB_REALLOC_GNU && !NEED_REALLOC_GNU && !SYSTEM_MALLOC_GLIBC_COMPATIBLE
- if (n == 0)
- n = 1;
-# endif
- result = malloc (n);
+ errno = ENOMEM;
+ return NULL;
}
- else
- result = realloc (p, n);
-# if !HAVE_REALLOC_POSIX
+ void *result = realloc (p, n);
+
+#if !HAVE_MALLOC_POSIX
if (result == NULL)
errno = ENOMEM;
-# endif
+#endif
return result;
}
-
-#endif
diff --git a/m4/calloc.m4 b/m4/calloc.m4
index eeeb033d8..143f3bc3f 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -1,4 +1,4 @@
-# calloc.m4 serial 23
+# calloc.m4 serial 24
# Copyright (C) 2004-2021 Free Software Foundation, Inc.
# This file is free software; the Free Software Foundation
@@ -14,12 +14,10 @@
# _AC_FUNC_CALLOC_IF([IF-WORKS], [IF-NOT])
# -------------------------------------
-# If 'calloc (0, 0)' is properly handled, run IF-WORKS, otherwise, IF-NOT.
+# If calloc is compatible with GNU calloc, run IF-WORKS, otherwise, IF-NOT.
AC_DEFUN([_AC_FUNC_CALLOC_IF],
-[
- AC_REQUIRE([AC_TYPE_SIZE_T])dnl
- AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
- AC_CACHE_CHECK([for GNU libc compatible calloc],
+[ AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
+ AC_CACHE_CHECK([whether calloc (0, n) and calloc (n, 0) return nonnull],
[ac_cv_func_calloc_0_nonnull],
[if test $cross_compiling != yes; then
ac_cv_func_calloc_0_nonnull=yes
@@ -35,32 +33,6 @@ AC_DEFUN([_AC_FUNC_CALLOC_IF],
]])],
[],
[ac_cv_func_calloc_0_nonnull=no])
- AC_RUN_IFELSE(
- [AC_LANG_PROGRAM(
- [AC_INCLUDES_DEFAULT],
- [[int result;
- typedef struct { char c[8]; } S8;
- size_t n = (size_t) -1 / sizeof (S8) + 2;
- S8 * volatile s = calloc (n, sizeof (S8));
- if (s)
- {
- s[0].c[0] = 1;
- if (s[n - 1].c[0])
- result = 0;
- else
- result = 2;
- }
- else
- result = 3;
- free (s);
- return result;
- ]])],
- dnl The exit code of this program is 0 if calloc() succeeded with a
- dnl wrap-around bug (which it shouldn't), 2 if calloc() succeeded in
- dnl a non-flat address space, 3 if calloc() failed, or 1 if some leak
- dnl sanitizer terminated the program as a result of the calloc() call.
- [ac_cv_func_calloc_0_nonnull=no],
- [])
else
case "$host_os" in
# Guess yes on glibc systems.
@@ -82,38 +54,31 @@ AC_DEFUN([_AC_FUNC_CALLOC_IF],
$2
;;
esac
-])# AC_FUNC_CALLOC
+])
# gl_FUNC_CALLOC_GNU
# ------------------
-# Report whether 'calloc (0, 0)' is properly handled, and replace calloc if
-# needed.
+# Replace calloc if it is not compatible with GNU libc.
AC_DEFUN([gl_FUNC_CALLOC_GNU],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
- _AC_FUNC_CALLOC_IF(
- [AC_DEFINE([HAVE_CALLOC_GNU], [1],
- [Define to 1 if your system has a GNU libc compatible 'calloc'
- function, and to 0 otherwise.])],
- [AC_DEFINE([HAVE_CALLOC_GNU], [0])
- REPLACE_CALLOC=1
- ])
+ AC_REQUIRE([gl_FUNC_CALLOC_POSIX])
+ test $REPLACE_CALLOC = 1 || _AC_FUNC_CALLOC_IF([], [REPLACE_CALLOC=1])
])# gl_FUNC_CALLOC_GNU
-
# gl_FUNC_CALLOC_POSIX
# --------------------
# Test whether 'calloc' is POSIX compliant (sets errno to ENOMEM when it
-# fails), and replace calloc if it is not.
+# fails, and doesn't mess up with ptrdiff_t or size_t overflow),
+# and replace calloc if it is not.
AC_DEFUN([gl_FUNC_CALLOC_POSIX],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
- AC_REQUIRE([gl_CHECK_MALLOC_POSIX])
- if test $gl_cv_func_malloc_posix = yes; then
- AC_DEFINE([HAVE_CALLOC_POSIX], [1],
- [Define if the 'calloc' function is POSIX compliant.])
- else
- REPLACE_CALLOC=1
- fi
+ AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
+ REPLACE_CALLOC=$REPLACE_MALLOC
+ dnl Although in theory we should also test for size_t overflow,
+ dnl in practice testing for ptrdiff_t overflow suffices
+ dnl since PTRDIFF_MAX <= SIZE_MAX on all known Gnulib porting targets.
+ dnl A separate size_t test would slow down 'configure'.
])
diff --git a/m4/malloc.m4 b/m4/malloc.m4
index 32ab42ec0..503da2cf8 100644
--- a/m4/malloc.m4
+++ b/m4/malloc.m4
@@ -1,4 +1,4 @@
-# malloc.m4 serial 22
+# malloc.m4 serial 23
dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -7,9 +7,8 @@ dnl with or without modifications, as long as this notice is preserved.
# This is adapted with modifications from upstream Autoconf here:
# https://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=04be2b7a29d65d9a08e64e8e56e594c91749598c
AC_DEFUN([_AC_FUNC_MALLOC_IF],
-[
- AC_REQUIRE([AC_CANONICAL_HOST])dnl for cross-compiles
- AC_CACHE_CHECK([for GNU libc compatible malloc],
+[ AC_REQUIRE([AC_CANONICAL_HOST])dnl for cross-compiles
+ AC_CACHE_CHECK([whether malloc (0) returns nonnull],
[ac_cv_func_malloc_0_nonnull],
[AC_RUN_IFELSE(
[AC_LANG_PROGRAM(
@@ -44,47 +43,89 @@ AC_DEFUN([_AC_FUNC_MALLOC_IF],
# gl_FUNC_MALLOC_GNU
# ------------------
-# Test whether 'malloc (0)' is handled like in GNU libc, and replace malloc if
-# it is not.
+# Replace malloc if it is not compatible with GNU libc.
AC_DEFUN([gl_FUNC_MALLOC_GNU],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
- dnl _AC_FUNC_MALLOC_IF is defined in Autoconf.
- _AC_FUNC_MALLOC_IF(
- [AC_DEFINE([HAVE_MALLOC_GNU], [1],
- [Define to 1 if your system has a GNU libc compatible 'malloc'
- function, and to 0 otherwise.])],
- [AC_DEFINE([HAVE_MALLOC_GNU], [0])
- REPLACE_MALLOC=1
+ AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
+ test $REPLACE_MALLOC = 1 || _AC_FUNC_MALLOC_IF([], [REPLACE_MALLOC=1])
+])
+
+# gl_FUNC_MALLOC_PTRDIFF
+# ----------------------
+# Test whether malloc (N) reliably fails when N exceeds PTRDIFF_MAX,
+# and replace malloc otherwise.
+AC_DEFUN([gl_FUNC_MALLOC_PTRDIFF],
+[
+ AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+ AC_REQUIRE([gl_CHECK_MALLOC_PTRDIFF])
+ test "$gl_cv_malloc_ptrdiff" = yes || REPLACE_MALLOC=1
+])
+
+# Test whether malloc, realloc, calloc refuse to create objects
+# larger than what can be expressed in ptrdiff_t.
+# Set gl_cv_func_malloc_gnu to yes or no accordingly.
+AC_DEFUN([gl_CHECK_MALLOC_PTRDIFF],
+[
+ AC_CACHE_CHECK([whether malloc is ptrdiff_t safe],
+ [gl_cv_malloc_ptrdiff],
+ [AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[#include <stdint.h>
+ ]],
+ [[/* 64-bit ptrdiff_t is so wide that no practical platform
+ can exceed it. */
+ #define WIDE_PTRDIFF (PTRDIFF_MAX >> 31 >> 31 != 0)
+
+ /* On rare machines where size_t fits in ptrdiff_t there
+ is no problem. */
+ #define NARROW_SIZE (SIZE_MAX <= PTRDIFF_MAX)
+
+ /* glibc 2.30 and later malloc refuses to exceed ptrdiff_t
+ bounds even on 32-bit platforms. We don't know which
+ non-glibc systems are safe. */
+ #define KNOWN_SAFE (2 < __GLIBC__ + (30 <= __GLIBC_MINOR__))
+
+ #if WIDE_PTRDIFF || NARROW_SIZE || KNOWN_SAFE
+ return 0;
+ #else
+ #error "malloc might not be ptrdiff_t safe"
+ syntax error
+ #endif
+ ]])],
+ [gl_cv_malloc_ptrdiff=yes],
+ [gl_cv_malloc_ptrdiff=no])
])
])
# gl_FUNC_MALLOC_POSIX
# --------------------
# Test whether 'malloc' is POSIX compliant (sets errno to ENOMEM when it
-# fails), and replace malloc if it is not.
+# fails, and doesn't mess up with ptrdiff_t overflow), and replace
+# malloc if it is not.
AC_DEFUN([gl_FUNC_MALLOC_POSIX],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+ AC_REQUIRE([gl_FUNC_MALLOC_PTRDIFF])
AC_REQUIRE([gl_CHECK_MALLOC_POSIX])
- if test $gl_cv_func_malloc_posix = yes; then
+ if test "$gl_cv_func_malloc_posix" = yes; then
AC_DEFINE([HAVE_MALLOC_POSIX], [1],
- [Define if the 'malloc' function is POSIX compliant.])
+ [Define if malloc, realloc, and calloc set errno on allocation failure.])
else
REPLACE_MALLOC=1
fi
])
-# Test whether malloc, realloc, calloc are POSIX compliant,
+# Test whether malloc, realloc, calloc set errno on failure.
# Set gl_cv_func_malloc_posix to yes or no accordingly.
AC_DEFUN([gl_CHECK_MALLOC_POSIX],
[
- AC_CACHE_CHECK([whether malloc, realloc, calloc are POSIX compliant],
+ AC_CACHE_CHECK([whether malloc, realloc, calloc set errno on failure],
[gl_cv_func_malloc_posix],
[
dnl It is too dangerous to try to allocate a large amount of memory:
dnl some systems go to their knees when you do that. So assume that
- dnl all Unix implementations of the function are POSIX compliant.
+ dnl all Unix implementations of the function set errno on failure.
AC_COMPILE_IFELSE(
[AC_LANG_PROGRAM(
[[]],
diff --git a/m4/realloc.m4 b/m4/realloc.m4
index a80a02a6b..4939516a0 100644
--- a/m4/realloc.m4
+++ b/m4/realloc.m4
@@ -1,4 +1,4 @@
-# realloc.m4 serial 20
+# realloc.m4 serial 21
dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -7,9 +7,8 @@ dnl with or without modifications, as long as this notice is preserved.
# This is adapted with modifications from upstream Autoconf here:
# https://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=04be2b7a29d65d9a08e64e8e56e594c91749598c
AC_DEFUN([_AC_FUNC_REALLOC_IF],
-[
- AC_REQUIRE([AC_CANONICAL_HOST])dnl for cross-compiles
- AC_CACHE_CHECK([for GNU libc compatible realloc],
+[ AC_REQUIRE([AC_CANONICAL_HOST])dnl for cross-compiles
+ AC_CACHE_CHECK([whether realloc (0, 0) returns nonnull],
[ac_cv_func_realloc_0_nonnull],
[AC_RUN_IFELSE(
[AC_LANG_PROGRAM(
@@ -44,33 +43,22 @@ AC_DEFUN([_AC_FUNC_REALLOC_IF],
# gl_FUNC_REALLOC_GNU
# -------------------
-# Test whether 'realloc (0, 0)' is handled like in GNU libc, and replace
-# realloc if it is not.
+# Replace realloc if it is not compatible with GNU libc.
AC_DEFUN([gl_FUNC_REALLOC_GNU],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
- dnl _AC_FUNC_REALLOC_IF is defined in Autoconf.
- _AC_FUNC_REALLOC_IF(
- [AC_DEFINE([HAVE_REALLOC_GNU], [1],
- [Define to 1 if your system has a GNU libc compatible 'realloc'
- function, and to 0 otherwise.])],
- [AC_DEFINE([HAVE_REALLOC_GNU], [0])
- REPLACE_REALLOC=1
- ])
+ AC_REQUIRE([gl_FUNC_REALLOC_POSIX])
+ test $REPLACE_REALLOC = 1 || _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
])# gl_FUNC_REALLOC_GNU
# gl_FUNC_REALLOC_POSIX
# ---------------------
# Test whether 'realloc' is POSIX compliant (sets errno to ENOMEM when it
-# fails), and replace realloc if it is not.
+# fails, and doesn't mess up with ptrdiff_t overflow),
+# and replace realloc if it is not.
AC_DEFUN([gl_FUNC_REALLOC_POSIX],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
- AC_REQUIRE([gl_CHECK_MALLOC_POSIX])
- if test $gl_cv_func_malloc_posix = yes; then
- AC_DEFINE([HAVE_REALLOC_POSIX], [1],
- [Define if the 'realloc' function is POSIX compliant.])
- else
- REPLACE_REALLOC=1
- fi
+ AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
+ REPLACE_REALLOC=$REPLACE_MALLOC
])
diff --git a/modules/calloc-gnu b/modules/calloc-gnu
index ffc8b50ef..2aa2dd1c0 100644
--- a/modules/calloc-gnu
+++ b/modules/calloc-gnu
@@ -13,7 +13,6 @@ gl_FUNC_CALLOC_GNU
if test $REPLACE_CALLOC = 1; then
AC_LIBOBJ([calloc])
fi
-gl_MODULE_INDICATOR([calloc-gnu])
Makefile.am:
diff --git a/modules/calloc-posix b/modules/calloc-posix
index cead843ae..dc9cadd5c 100644
--- a/modules/calloc-posix
+++ b/modules/calloc-posix
@@ -8,6 +8,7 @@ m4/malloc.m4
Depends-on:
stdlib
+xalloc-oversized [test $REPLACE_REALLOC = 1]
configure.ac:
gl_FUNC_CALLOC_POSIX
diff --git a/modules/malloc-gnu b/modules/malloc-gnu
index 0bfa92d75..f8bcdae55 100644
--- a/modules/malloc-gnu
+++ b/modules/malloc-gnu
@@ -17,7 +17,6 @@ gl_FUNC_MALLOC_GNU
if test $REPLACE_MALLOC = 1; then
AC_LIBOBJ([malloc])
fi
-gl_MODULE_INDICATOR([malloc-gnu])
Makefile.am:
diff --git a/modules/malloc-posix b/modules/malloc-posix
index 1a2d72c5a..bafcf3801 100644
--- a/modules/malloc-posix
+++ b/modules/malloc-posix
@@ -7,14 +7,14 @@ m4/malloc.m4
Depends-on:
stdlib
+xalloc-oversized [test $REPLACE_MALLOC = 1]
configure.ac:
-gl_FUNC_MALLOC_POSIX
+AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
if test $REPLACE_MALLOC = 1; then
AC_LIBOBJ([malloc])
fi
gl_STDLIB_MODULE_INDICATOR([malloc-posix])
-gl_MODULE_INDICATOR([malloc-posix])
Makefile.am:
diff --git a/modules/realloc-gnu b/modules/realloc-gnu
index 760edcda1..7752d8c6a 100644
--- a/modules/realloc-gnu
+++ b/modules/realloc-gnu
@@ -17,7 +17,6 @@ gl_FUNC_REALLOC_GNU
if test $REPLACE_REALLOC = 1; then
AC_LIBOBJ([realloc])
fi
-gl_MODULE_INDICATOR([realloc-gnu])
Makefile.am:
diff --git a/modules/realloc-posix b/modules/realloc-posix
index 7f481102b..a30356f31 100644
--- a/modules/realloc-posix
+++ b/modules/realloc-posix
@@ -8,6 +8,9 @@ m4/malloc.m4
Depends-on:
stdlib
+free-posix [test $REPLACE_REALLOC = 1]
+malloc-posix [test $REPLACE_REALLOC = 1]
+xalloc-oversized [test $REPLACE_REALLOC = 1]
configure.ac:
gl_FUNC_REALLOC_POSIX
@@ -15,7 +18,6 @@ if test $REPLACE_REALLOC = 1; then
AC_LIBOBJ([realloc])
fi
gl_STDLIB_MODULE_INDICATOR([realloc-posix])
-gl_MODULE_INDICATOR([realloc-posix])
Makefile.am:
--
2.27.0
[-- Attachment #3: 0002-xalloc-adjust-to-malloc-ptrdiff_t-change.patch --]
[-- Type: text/x-patch, Size: 3657 bytes --]
From 7415a0fa4abc1b8e8af37afd5b5408c1d5dfff2e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 17 Apr 2021 18:48:38 -0700
Subject: [PATCH 2/2] xalloc: adjust to malloc ptrdiff_t change
* lib/xmalloc.c (HAVE_GNU_CALLOC, HAVE_GNU_MALLOC, HAVE_GNU_REALLOC):
Remove.
(xmalloc, xrealloc, xcalloc): Simplify by assuming GNU behavior.
* modules/xalloc (Depends-on): Add calloc-gnu, malloc-gnu,
realloc-gnu.
---
ChangeLog | 7 +++++++
lib/xmalloc.c | 41 ++++-------------------------------------
modules/xalloc | 3 +++
3 files changed, 14 insertions(+), 37 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 825201fe2..03221a292 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2021-04-17 Paul Eggert <eggert@cs.ucla.edu>
+ xalloc: adjust to malloc ptrdiff_t change
+ * lib/xmalloc.c (HAVE_GNU_CALLOC, HAVE_GNU_MALLOC, HAVE_GNU_REALLOC):
+ Remove.
+ (xmalloc, xrealloc, xcalloc): Simplify by assuming GNU behavior.
+ * modules/xalloc (Depends-on): Add calloc-gnu, malloc-gnu,
+ realloc-gnu.
+
malloc, etc.: check for ptrdiff_t overflow
In glibc 2.30 and later, malloc, realloc and calloc reject
attempts to create objects larger than PTRDIFF_MAX bytes.
diff --git a/lib/xmalloc.c b/lib/xmalloc.c
index 4a6589571..39ce893ad 100644
--- a/lib/xmalloc.c
+++ b/lib/xmalloc.c
@@ -27,34 +27,13 @@
#include <stdlib.h>
#include <string.h>
-/* 1 if calloc, malloc and realloc are known to be compatible with GNU.
- This matters if we are not also using the calloc-gnu, malloc-gnu
- and realloc-gnu modules, which define HAVE_CALLOC_GNU,
- HAVE_MALLOC_GNU and HAVE_REALLOC_GNU and support the GNU API even
- on non-GNU platforms. */
-#if defined HAVE_CALLOC_GNU || (defined __GLIBC__ && !defined __UCLIBC__)
-enum { HAVE_GNU_CALLOC = 1 };
-#else
-enum { HAVE_GNU_CALLOC = 0 };
-#endif
-#if defined HAVE_MALLOC_GNU || (defined __GLIBC__ && !defined __UCLIBC__)
-enum { HAVE_GNU_MALLOC = 1 };
-#else
-enum { HAVE_GNU_MALLOC = 0 };
-#endif
-#if defined HAVE_REALLOC_GNU || (defined __GLIBC__ && !defined __UCLIBC__)
-enum { HAVE_GNU_REALLOC = 1 };
-#else
-enum { HAVE_GNU_REALLOC = 0 };
-#endif
-
/* Allocate N bytes of memory dynamically, with error checking. */
void *
xmalloc (size_t n)
{
void *p = malloc (n);
- if (!p && (HAVE_GNU_MALLOC || n))
+ if (!p)
xalloc_die ();
return p;
}
@@ -65,15 +44,8 @@ xmalloc (size_t n)
void *
xrealloc (void *p, size_t n)
{
- if (!HAVE_GNU_REALLOC && !n && p)
- {
- /* The GNU and C99 realloc behaviors disagree here. Act like GNU. */
- free (p);
- return NULL;
- }
-
void *r = realloc (p, n);
- if (!r && (n || (HAVE_GNU_REALLOC && !p)))
+ if (!r && (!p || n))
xalloc_die ();
return r;
}
@@ -175,13 +147,8 @@ xzalloc (size_t n)
void *
xcalloc (size_t n, size_t s)
{
- void *p;
- /* 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)))
+ void *p = calloc (n, s);
+ if (!p)
xalloc_die ();
return p;
}
diff --git a/modules/xalloc b/modules/xalloc
index 5fa386a5d..000933e94 100644
--- a/modules/xalloc
+++ b/modules/xalloc
@@ -8,10 +8,13 @@ m4/xalloc.m4
Depends-on:
c99
+calloc-gnu
extern-inline
idx
intprops
+malloc-gnu
minmax
+realloc-gnu
stdint
xalloc-die
xalloc-oversized
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 2:02 ptrdiff_t overflow checks for malloc-posix etc Paul Eggert
@ 2021-04-18 11:59 ` Bruno Haible
2021-04-18 12:11 ` Bruno Haible
` (3 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Bruno Haible @ 2021-04-18 11:59 UTC (permalink / raw)
To: bug-gnulib; +Cc: Paul Eggert
> * doc/posix-functions/calloc.texi:
> * doc/posix-functions/malloc.texi:
> * doc/posix-functions/realloc.texi:
> Mention ptrdiff_t issues, and go into more detail about what
> the gnu extension module does.
The patch dropped the list of affected platforms. However this list is
important so that
- users can decide whether they want to pull in the module,
- I know on which platforms to conduct tests when preparing changes in this
area.
2021-04-18 Bruno Haible <bruno@clisp.org>
malloc-posix, realloc-posix, calloc-posix: Document affected platforms.
* doc/posix-functions/malloc.texi: Re-add platforms list.
* doc/posix-functions/realloc.texi: Likewise.
* doc/posix-functions/calloc.texi: Likewise.
diff --git a/doc/posix-functions/calloc.texi b/doc/posix-functions/calloc.texi
index 9ba40c0..57bec4d 100644
--- a/doc/posix-functions/calloc.texi
+++ b/doc/posix-functions/calloc.texi
@@ -28,6 +28,7 @@ It fixes this portability problem:
@itemize
@item
-On some platforms, @code{calloc (0, s)} and @code{calloc (n, 0)}
-return @code{NULL} on success.
+@code{calloc (0, s)} and @code{calloc (n, 0)} return @code{NULL} on success
+on some platforms:
+AIX 7.2.
@end itemize
diff --git a/doc/posix-functions/malloc.texi b/doc/posix-functions/malloc.texi
index 8295173..028f1dc 100644
--- a/doc/posix-functions/malloc.texi
+++ b/doc/posix-functions/malloc.texi
@@ -26,5 +26,6 @@ by fixing this portability problem:
@itemize
@item
-On some platforms, @code{malloc (0)} returns @code{NULL} on success.
+@code{malloc (0)} returns @code{NULL} on success on some platforms:
+AIX 7.2.
@end itemize
diff --git a/doc/posix-functions/realloc.texi b/doc/posix-functions/realloc.texi
index 282e360..009bdab 100644
--- a/doc/posix-functions/realloc.texi
+++ b/doc/posix-functions/realloc.texi
@@ -39,7 +39,8 @@ It fixes these portability problems:
@itemize
@item
-On some platforms, @code{realloc (NULL, 0)} returns @code{NULL} on success.
+@code{realloc (NULL, 0)} returns @code{NULL} on success on some platforms:
+AIX 7.2.
@item
On some platforms, @code{realloc (p, 0)} with non-null @code{p}
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 2:02 ptrdiff_t overflow checks for malloc-posix etc Paul Eggert
2021-04-18 11:59 ` Bruno Haible
@ 2021-04-18 12:11 ` Bruno Haible
2021-04-18 20:24 ` Paul Eggert
2021-04-18 12:13 ` Bruno Haible
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-04-18 12:11 UTC (permalink / raw)
To: bug-gnulib; +Cc: Paul Eggert
Paul Eggert wrote:
> Come to think of it, why do we have both malloc-gnu and malloc-posix
> modules (and similarly for calloc and realloc)?
Package owners had two decisions to make:
"Does my package ever call malloc (0)? - If yes, I need 'malloc-gnu'."
"Does my package attempt portability to native Windows? - If yes, I
need 'malloc-posix'."
> In other words, I suggest that we remove malloc-posix, realloc-posix and
> calloc-posix
Think about the effects on the package on non-glibc platforms, from the
perspective of the package owner.
For example, if my package never calls malloc (0) but desires portability
to native Windows, would I want that *every* malloc call on *all*
non-glibc platforms goes through hoops before it reaches the malloc()
function in libc?
Bruno
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 2:02 ptrdiff_t overflow checks for malloc-posix etc Paul Eggert
2021-04-18 11:59 ` Bruno Haible
2021-04-18 12:11 ` Bruno Haible
@ 2021-04-18 12:13 ` Bruno Haible
2021-04-18 19:47 ` Paul Eggert
2021-04-18 12:13 ` Bruno Haible
2021-05-09 16:46 ` Bruno Haible
4 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-04-18 12:13 UTC (permalink / raw)
To: bug-gnulib; +Cc: Paul Eggert
> In glibc 2.30 and later, malloc, realloc and calloc reject
> attempts to create objects larger than PTRDIFF_MAX bytes.
> This patch changes malloc-gnu etc. to support this behavior
> on non-GNU hosts.
How about extending the unit test (tests/test-malloc-gnu.c) accordingly?
Bruno
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 2:02 ptrdiff_t overflow checks for malloc-posix etc Paul Eggert
` (2 preceding siblings ...)
2021-04-18 12:13 ` Bruno Haible
@ 2021-04-18 12:13 ` Bruno Haible
2021-04-18 18:37 ` Paul Eggert
2021-05-09 16:46 ` Bruno Haible
4 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-04-18 12:13 UTC (permalink / raw)
To: bug-gnulib; +Cc: Paul Eggert
Hi Paul,
> * m4/realloc.m4 (_AC_FUNC_REALLOC_IF):
> Don’t start with a newline.
This is not very robust. We usually don't care about newlines or useless
indentation in the generated configure.ac any more. Therefore future
edits may reintroduce a newline here. It would be more robust to change
test $REPLACE_REALLOC = 1 || _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
to
if test $REPLACE_REALLOC = 0; then
_AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
fi
Bruno
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 12:13 ` Bruno Haible
@ 2021-04-18 18:37 ` Paul Eggert
2021-05-09 20:19 ` Bruno Haible
0 siblings, 1 reply; 17+ messages in thread
From: Paul Eggert @ 2021-04-18 18:37 UTC (permalink / raw)
To: Bruno Haible; +Cc: bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]
On 4/18/21 5:13 AM, Bruno Haible wrote:
>> * m4/realloc.m4 (_AC_FUNC_REALLOC_IF):
>> Don’t start with a newline.
>
> This is not very robust. We usually don't care about newlines or useless
> indentation in the generated configure.ac any more.
Yes, I made that change only because I ran into some shell syntax error
and the change fixed it. I don't recall what the error was and can't
reproduce it now, so the attached first patch reverts that change.
If we run into further problems in this area, though, perhaps the patch
should go back in. Autoconf (which _AC_FUNC_REALLOC_IF is taken from)
does care about leading newlines, and even if we have a different API in
Gnulib surely it's better to stick to the Autoconf API in
Autoconf-replacing macros.
> if test $REPLACE_REALLOC = 0; then
> _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
> fi
I confess I don't like the style as much: it makes the shell code a bit
less readable, at least to me. But it appears that this style isn't
needed anyway.
I noticed some other incompatibilities with Autoconf _AC_FUNC_REALLOC_IF
etc. and fixed them too in the attached first patch.
The second patch switches from AS_IF to AS_CASE as that's cleaner in the
Gnulib version.
[-- Attachment #2: 0001-malloc-gnu-etc.-sync-better-with-Autoconf.patch --]
[-- Type: text/x-patch, Size: 7334 bytes --]
From c08e261c39ef3840b1cb1ae7c04c669c469cd91d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Apr 2021 11:23:24 -0700
Subject: [PATCH 1/2] malloc-gnu, etc.: sync better with Autoconf
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF):
* m4/malloc.m4 (_AC_FUNC_MALLOC_IF):
* m4/realloc.m4 (_AC_FUNC_REALLOC_IF):
Avoid some unnecessary differences from Autoconf’s versions.
Separate our platforms into a different line so that it’s easier
to diff. Use AS_IF in case the args use AC_REQUIRE.
However, don’t bother with omitting the first newline, as
omitting the newline is not Gnulib style and the difference
doesn’t seem to matter here.
---
ChangeLog | 13 +++++++++++++
m4/calloc.m4 | 15 +++++----------
m4/malloc.m4 | 24 ++++++++++--------------
m4/realloc.m4 | 24 ++++++++++--------------
4 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 38311496b..fa630a758 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2021-04-18 Paul Eggert <eggert@cs.ucla.edu>
+
+ malloc-gnu, etc.: sync better with Autoconf
+ * m4/calloc.m4 (_AC_FUNC_CALLOC_IF):
+ * m4/malloc.m4 (_AC_FUNC_MALLOC_IF):
+ * m4/realloc.m4 (_AC_FUNC_REALLOC_IF):
+ Avoid some unnecessary differences from Autoconf’s versions.
+ Separate our platforms into a different line so that it’s easier
+ to diff. Use AS_IF in case the args use AC_REQUIRE.
+ However, don’t bother with omitting the first newline, as
+ omitting the newline is not Gnulib style and the difference
+ doesn’t seem to matter here.
+
2021-04-18 Bruno Haible <bruno@clisp.org>
malloc-posix, realloc-posix, calloc-posix: Document affected platforms.
diff --git a/m4/calloc.m4 b/m4/calloc.m4
index 143f3bc3f..776979d25 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -1,4 +1,4 @@
-# calloc.m4 serial 24
+# calloc.m4 serial 25
# Copyright (C) 2004-2021 Free Software Foundation, Inc.
# This file is free software; the Free Software Foundation
@@ -16,7 +16,8 @@
# -------------------------------------
# If calloc is compatible with GNU calloc, run IF-WORKS, otherwise, IF-NOT.
AC_DEFUN([_AC_FUNC_CALLOC_IF],
-[ AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
+[
+ AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
AC_CACHE_CHECK([whether calloc (0, n) and calloc (n, 0) return nonnull],
[ac_cv_func_calloc_0_nonnull],
[if test $cross_compiling != yes; then
@@ -46,14 +47,8 @@ AC_DEFUN([_AC_FUNC_CALLOC_IF],
esac
fi
])
- case "$ac_cv_func_calloc_0_nonnull" in
- *yes)
- $1
- ;;
- *)
- $2
- ;;
- esac
+ AS_IF([case $ac_cv_func_calloc_0_nonnull in *yes) :;; *) false;; esac],
+ [$1], [$2])
])
diff --git a/m4/malloc.m4 b/m4/malloc.m4
index 503da2cf8..9734dab05 100644
--- a/m4/malloc.m4
+++ b/m4/malloc.m4
@@ -1,20 +1,21 @@
-# malloc.m4 serial 23
+# malloc.m4 serial 24
dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
# This is adapted with modifications from upstream Autoconf here:
-# https://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=04be2b7a29d65d9a08e64e8e56e594c91749598c
+# https://git.savannah.gnu.org/cgit/autoconf.git/tree/lib/autoconf/functions.m4?id=v2.70#n949
AC_DEFUN([_AC_FUNC_MALLOC_IF],
-[ AC_REQUIRE([AC_CANONICAL_HOST])dnl for cross-compiles
+[
+ AC_REQUIRE([AC_CANONICAL_HOST])dnl for cross-compiles
AC_CACHE_CHECK([whether malloc (0) returns nonnull],
[ac_cv_func_malloc_0_nonnull],
[AC_RUN_IFELSE(
[AC_LANG_PROGRAM(
[[#include <stdlib.h>
]],
- [[char *p = malloc (0);
+ [[void *p = malloc (0);
int result = !p;
free (p);
return result;]])
@@ -23,22 +24,17 @@ AC_DEFUN([_AC_FUNC_MALLOC_IF],
[ac_cv_func_malloc_0_nonnull=no],
[case "$host_os" in
# Guess yes on platforms where we know the result.
- *-gnu* | gnu* | *-musl* | freebsd* | midnightbsd* | netbsd* | openbsd* \
- | hpux* | solaris* | cygwin* | mingw*)
+ *-gnu* | freebsd* | netbsd* | openbsd* | bitrig* \
+ | gnu* | *-musl* | midnightbsd* \
+ | hpux* | solaris* | cygwin* | mingw* | msys* )
ac_cv_func_malloc_0_nonnull="guessing yes" ;;
# If we don't know, obey --enable-cross-guesses.
*) ac_cv_func_malloc_0_nonnull="$gl_cross_guess_normal" ;;
esac
])
])
- case "$ac_cv_func_malloc_0_nonnull" in
- *yes)
- $1
- ;;
- *)
- $2
- ;;
- esac
+ AS_IF([case $ac_cv_func_malloc_0_nonnull in *yes) :;; *) false;; esac],
+ [$1], [$2])
])# _AC_FUNC_MALLOC_IF
# gl_FUNC_MALLOC_GNU
diff --git a/m4/realloc.m4 b/m4/realloc.m4
index 4939516a0..cde906192 100644
--- a/m4/realloc.m4
+++ b/m4/realloc.m4
@@ -1,20 +1,21 @@
-# realloc.m4 serial 21
+# realloc.m4 serial 22
dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
# This is adapted with modifications from upstream Autoconf here:
-# https://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=04be2b7a29d65d9a08e64e8e56e594c91749598c
+# https://git.savannah.gnu.org/cgit/autoconf.git/tree/lib/autoconf/functions.m4?id=v2.70#n1455
AC_DEFUN([_AC_FUNC_REALLOC_IF],
-[ AC_REQUIRE([AC_CANONICAL_HOST])dnl for cross-compiles
+[
+ AC_REQUIRE([AC_CANONICAL_HOST])dnl for cross-compiles
AC_CACHE_CHECK([whether realloc (0, 0) returns nonnull],
[ac_cv_func_realloc_0_nonnull],
[AC_RUN_IFELSE(
[AC_LANG_PROGRAM(
[[#include <stdlib.h>
]],
- [[char *p = realloc (0, 0);
+ [[void *p = realloc (0, 0);
int result = !p;
free (p);
return result;]])
@@ -23,22 +24,17 @@ AC_DEFUN([_AC_FUNC_REALLOC_IF],
[ac_cv_func_realloc_0_nonnull=no],
[case "$host_os" in
# Guess yes on platforms where we know the result.
- *-gnu* | gnu* | *-musl* | freebsd* | midnightbsd* | netbsd* | openbsd* \
- | hpux* | solaris* | cygwin* | mingw*)
+ *-gnu* | freebsd* | netbsd* | openbsd* | bitrig* \
+ | gnu* | *-musl* | midnightbsd* \
+ | hpux* | solaris* | cygwin* | mingw* | msys* )
ac_cv_func_realloc_0_nonnull="guessing yes" ;;
# If we don't know, obey --enable-cross-guesses.
*) ac_cv_func_realloc_0_nonnull="$gl_cross_guess_normal" ;;
esac
])
])
- case "$ac_cv_func_realloc_0_nonnull" in
- *yes)
- $1
- ;;
- *)
- $2
- ;;
- esac
+ AS_IF([case $ac_cv_func_realloc_0_nonnull in *yes) :;; *) false;; esac],
+ [$1], [$2])
])# AC_FUNC_REALLOC
# gl_FUNC_REALLOC_GNU
--
2.27.0
[-- Attachment #3: 0002-malloc-gnu-etc.-prefer-AS_CASE-to-woolly-AS_IF.patch --]
[-- Type: text/x-patch, Size: 2213 bytes --]
From 6a9be1acc3445f09d7dd875a14271bbb7cb05c0c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Apr 2021 11:31:02 -0700
Subject: [PATCH 2/2] malloc-gnu, etc.: prefer AS_CASE to woolly AS_IF
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF):
* m4/malloc.m4 (_AC_FUNC_MALLOC_IF):
* m4/realloc.m4 (_AC_FUNC_REALLOC_IF): Use AS_CASE.
---
ChangeLog | 5 +++++
m4/calloc.m4 | 3 +--
m4/malloc.m4 | 3 +--
m4/realloc.m4 | 3 +--
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index fa630a758..dd491f07b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2021-04-18 Paul Eggert <eggert@cs.ucla.edu>
+ malloc-gnu, etc.: prefer AS_CASE to woolly AS_IF
+ * m4/calloc.m4 (_AC_FUNC_CALLOC_IF):
+ * m4/malloc.m4 (_AC_FUNC_MALLOC_IF):
+ * m4/realloc.m4 (_AC_FUNC_REALLOC_IF): Use AS_CASE.
+
malloc-gnu, etc.: sync better with Autoconf
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF):
* m4/malloc.m4 (_AC_FUNC_MALLOC_IF):
diff --git a/m4/calloc.m4 b/m4/calloc.m4
index 776979d25..2f0abee03 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -47,8 +47,7 @@ AC_DEFUN([_AC_FUNC_CALLOC_IF],
esac
fi
])
- AS_IF([case $ac_cv_func_calloc_0_nonnull in *yes) :;; *) false;; esac],
- [$1], [$2])
+ AS_CASE([$ac_cv_func_calloc_0_nonnull], [*yes], [$1], [$2])
])
diff --git a/m4/malloc.m4 b/m4/malloc.m4
index 9734dab05..c09006df5 100644
--- a/m4/malloc.m4
+++ b/m4/malloc.m4
@@ -33,8 +33,7 @@ AC_DEFUN([_AC_FUNC_MALLOC_IF],
esac
])
])
- AS_IF([case $ac_cv_func_malloc_0_nonnull in *yes) :;; *) false;; esac],
- [$1], [$2])
+ AS_CASE([$ac_cv_func_malloc_0_nonnull], [*yes], [$1], [$2])
])# _AC_FUNC_MALLOC_IF
# gl_FUNC_MALLOC_GNU
diff --git a/m4/realloc.m4 b/m4/realloc.m4
index cde906192..8eb6b199f 100644
--- a/m4/realloc.m4
+++ b/m4/realloc.m4
@@ -33,8 +33,7 @@ AC_DEFUN([_AC_FUNC_REALLOC_IF],
esac
])
])
- AS_IF([case $ac_cv_func_realloc_0_nonnull in *yes) :;; *) false;; esac],
- [$1], [$2])
+ AS_CASE([$ac_cv_func_realloc_0_nonnull], [*yes], [$1], [$2])
])# AC_FUNC_REALLOC
# gl_FUNC_REALLOC_GNU
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 12:13 ` Bruno Haible
@ 2021-04-18 19:47 ` Paul Eggert
2021-04-18 20:23 ` Bruno Haible
0 siblings, 1 reply; 17+ messages in thread
From: Paul Eggert @ 2021-04-18 19:47 UTC (permalink / raw)
To: Bruno Haible; +Cc: bug-gnulib
On 4/18/21 5:13 AM, Bruno Haible wrote:
> How about extending the unit test (tests/test-malloc-gnu.c) accordingly?
Won't that raise the possibility of the tests being too expensive, in
case the C library actually attempts to allocate PTRDIFF_MAX + 1 bytes?
(I'm looking at you, 64-bit Hurd. :-)
I see we're already doing something similar with size_t in
test-reallocarray.c but I suspect that test isn't often run because
reallocarray isn't much used yet. Plus, that test won't use up much
resources even on typical buggy hosts, because size_t overflow wraps around.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 19:47 ` Paul Eggert
@ 2021-04-18 20:23 ` Bruno Haible
2021-04-18 22:33 ` Paul Eggert
0 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-04-18 20:23 UTC (permalink / raw)
To: Paul Eggert; +Cc: bug-gnulib
Hi Paul,
> > How about extending the unit test (tests/test-malloc-gnu.c) accordingly?
>
> Won't that raise the possibility of the tests being too expensive, in
> case the C library actually attempts to allocate PTRDIFF_MAX + 1 bytes?
In those cases where our code has a bug and the xalloc_oversized test in
malloc.c is not effective, yes, the test program may allocate a lot of memory.
If we put the test in a module that is marked as
Status:
privileged-test
normal users will never get to execute this test.
Bruno
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 12:11 ` Bruno Haible
@ 2021-04-18 20:24 ` Paul Eggert
2021-04-18 20:32 ` Bruno Haible
0 siblings, 1 reply; 17+ messages in thread
From: Paul Eggert @ 2021-04-18 20:24 UTC (permalink / raw)
To: Bruno Haible; +Cc: bug-gnulib
On 4/18/21 5:11 AM, Bruno Haible wrote:
> Paul Eggert wrote:
>> Come to think of it, why do we have both malloc-gnu and malloc-posix
>> modules (and similarly for calloc and realloc)?
>
> Package owners had two decisions to make:
> "Does my package ever call malloc (0)? - If yes, I need 'malloc-gnu'."
> "Does my package attempt portability to native Windows? - If yes, I
> need 'malloc-posix'."
Sure, but that sort of approach doesn't scale well. Now that we know the
issues better, that approach would require package owners to answer 11
questions:
A (for malloc, realloc, calloc). Does my package need errno to be set
when malloc/realloc/calloc fails?
B (for malloc, realloc, calloc). Does my package require
malloc/realloc/calloc to fail if it would create an object with more
than PTRDIFF_MAX bytes?
C (for malloc, realloc, calloc). Does my package require malloc/calloc
to return NULL if and only if it fails, and similarly for realloc (P, N)
when !P || N?
D (for realloc). Does my package require realloc (P, 0) with nonnull P
to free P and return NULL?
E (for calloc). Does my package require calloc (N, S) to fail when N * S
would exceed SIZE_MAX?
And I've probably forgotten one or more questions.
Should we have 11 different modules, one for each question? Surely not.
We shouldn't expect other packages to carefully select exactly which of
these 11 questions they need GNU compatibility with. Such a selection
will be error-prone and won't be well-tested if we get the selections wrong.
It's also not reasonable to test all 2**11 combinations of answers to
these questions. Some combinations will never occur, and others will be
rare and hard to be tested.
Instead, I suggest that we simply implement malloc, realloc and calloc
the GNU way on all platforms. Packages that rely on any GNU malloc
feature can simply require the malloc-gnu module without worrying about
individual questions.
Come to think of it, it might be better to have a single module (say,
alloc-gnu) check and fix all three functions (malloc, realloc, calloc)
as that'd be simpler yet.
> For example, if my package never calls malloc (0) but desires portability
> to native Windows, would I want that *every* malloc call on *all*
> non-glibc platforms goes through hoops before it reaches the malloc()
> function in libc?
I would want that, yes, because that'd be more reliable than what we
were doing. Any performance penalty on non-GNU platforms is surely
insignificant in real applications, and so is outweighed by the
reliability improvement. Admittedly it's hard to measure the reliability
improvement, but I've already fixed an unlikely bug or two as part of
this cleanup, and I won't be surprised to fix more.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 20:24 ` Paul Eggert
@ 2021-04-18 20:32 ` Bruno Haible
0 siblings, 0 replies; 17+ messages in thread
From: Bruno Haible @ 2021-04-18 20:32 UTC (permalink / raw)
To: Paul Eggert; +Cc: bug-gnulib
Paul Eggert wrote:
> > For example, if my package never calls malloc (0) but desires portability
> > to native Windows, would I want that *every* malloc call on *all*
> > non-glibc platforms goes through hoops before it reaches the malloc()
> > function in libc?
>
> I would want that, yes, because that'd be more reliable than what we
> were doing. Any performance penalty on non-GNU platforms is surely
> insignificant in real applications, and so is outweighed by the
> reliability improvement.
I don't disagree — as I can't measure reliability quantitatively.
Just wanted to give the reasoning that had led to the existing distinction
between 'malloc-posix' and 'malloc-gnu'.
Bruno
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 20:23 ` Bruno Haible
@ 2021-04-18 22:33 ` Paul Eggert
2021-04-19 0:04 ` Bruno Haible
0 siblings, 1 reply; 17+ messages in thread
From: Paul Eggert @ 2021-04-18 22:33 UTC (permalink / raw)
To: Bruno Haible; +Cc: bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 527 bytes --]
On 4/18/21 1:23 PM, Bruno Haible wrote:
> If we put the test in a module that is marked as
>
> Status:
> privileged-test
It shouldn't require root access to test.
I installed the attached instead, as this is simpler. If there are
problems with PTRDIFF_MAX + 1, the test program might thrash or maybe
even crash the kernel, but that's good enough since there shouldn't be
problems. These tests are not immune to arbitrary compiler optimization
tricks but it's not worth the trouble to try to bypass the trucks.
[-- Attachment #2: 0001-malloc-gnu-tests-etc.-test-ptrdiff_t-overflow.patch --]
[-- Type: text/x-patch, Size: 4364 bytes --]
From 4d58319de4759923a6661a7c05b08cbbd335285b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Apr 2021 15:29:54 -0700
Subject: [PATCH] malloc-gnu-tests, etc.: test ptrdiff_t overflow
* modules/calloc-gnu-tests (Depends-on):
* modules/malloc-gnu-tests (Depends-on):
* modules/realloc-gnu-tests (Depends-on): Add stdint.
* tests/test-calloc-gnu.c (main):
* tests/test-malloc-gnu.c (main):,
* tests/test-realloc-gnu.c (main): Test for ptrdiff_t overflow.
---
ChangeLog | 8 ++++++++
modules/calloc-gnu-tests | 1 +
modules/malloc-gnu-tests | 1 +
modules/realloc-gnu-tests | 1 +
tests/test-calloc-gnu.c | 14 +++++++++++++-
tests/test-malloc-gnu.c | 11 ++++++++++-
tests/test-realloc-gnu.c | 10 ++++++++++
7 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index dd491f07b..ab6045fd3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
2021-04-18 Paul Eggert <eggert@cs.ucla.edu>
+ malloc-gnu-tests, etc.: test ptrdiff_t overflow
+ * modules/calloc-gnu-tests (Depends-on):
+ * modules/malloc-gnu-tests (Depends-on):
+ * modules/realloc-gnu-tests (Depends-on): Add stdint.
+ * tests/test-calloc-gnu.c (main):
+ * tests/test-malloc-gnu.c (main):,
+ * tests/test-realloc-gnu.c (main): Test for ptrdiff_t overflow.
+
malloc-gnu, etc.: prefer AS_CASE to woolly AS_IF
* m4/calloc.m4 (_AC_FUNC_CALLOC_IF):
* m4/malloc.m4 (_AC_FUNC_MALLOC_IF):
diff --git a/modules/calloc-gnu-tests b/modules/calloc-gnu-tests
index 996db23b9..a4804fd28 100644
--- a/modules/calloc-gnu-tests
+++ b/modules/calloc-gnu-tests
@@ -2,6 +2,7 @@ Files:
tests/test-calloc-gnu.c
Depends-on:
+stdint
configure.ac:
diff --git a/modules/malloc-gnu-tests b/modules/malloc-gnu-tests
index 75f7e4f52..9a6f01cfa 100644
--- a/modules/malloc-gnu-tests
+++ b/modules/malloc-gnu-tests
@@ -2,6 +2,7 @@ Files:
tests/test-malloc-gnu.c
Depends-on:
+stdint
configure.ac:
diff --git a/modules/realloc-gnu-tests b/modules/realloc-gnu-tests
index 959d5d408..9d26260ba 100644
--- a/modules/realloc-gnu-tests
+++ b/modules/realloc-gnu-tests
@@ -2,6 +2,7 @@ Files:
tests/test-realloc-gnu.c
Depends-on:
+stdint
configure.ac:
diff --git a/tests/test-calloc-gnu.c b/tests/test-calloc-gnu.c
index 953bd778b..eb336e1a6 100644
--- a/tests/test-calloc-gnu.c
+++ b/tests/test-calloc-gnu.c
@@ -17,6 +17,7 @@
#include <config.h>
#include <stdlib.h>
+#include <stdint.h>
/* Return 8.
Usual compilers are not able to infer something about the return value. */
@@ -49,7 +50,7 @@ main ()
'volatile' is needed to defeat an incorrect optimization by clang 10,
see <https://bugs.llvm.org/show_bug.cgi?id=46055>. */
{
- void * volatile p = calloc ((size_t) -1 / 8 + 1, eight ());
+ void * volatile p = calloc (SIZE_MAX / 8 + 1, eight ());
if (p != NULL)
{
free (p);
@@ -57,5 +58,16 @@ main ()
}
}
+ /* Likewise for PTRDIFF_MAX. */
+ if (PTRDIFF_MAX / 8 < SIZE_MAX)
+ {
+ void * volatile p = calloc (PTRDIFF_MAX / 8 + 1, eight ());
+ if (p != NULL)
+ {
+ free (p);
+ return 2;
+ }
+ }
+
return 0;
}
diff --git a/tests/test-malloc-gnu.c b/tests/test-malloc-gnu.c
index 58a697f72..ce7e4fec2 100644
--- a/tests/test-malloc-gnu.c
+++ b/tests/test-malloc-gnu.c
@@ -17,6 +17,7 @@
#include <config.h>
#include <stdlib.h>
+#include <stdint.h>
int
main ()
@@ -25,7 +26,15 @@ main ()
char *p = malloc (0);
if (p == NULL)
return 1;
-
free (p);
+
+ /* Check that malloc (n) fails when n exceeds PTRDIFF_MAX. */
+ if (PTRDIFF_MAX < SIZE_MAX)
+ {
+ size_t n = PTRDIFF_MAX, n1 = n + 1;
+ if (malloc (n1) != NULL)
+ return 1;
+ }
+
return 0;
}
diff --git a/tests/test-realloc-gnu.c b/tests/test-realloc-gnu.c
index 296852049..9c7344f15 100644
--- a/tests/test-realloc-gnu.c
+++ b/tests/test-realloc-gnu.c
@@ -17,6 +17,7 @@
#include <config.h>
#include <stdlib.h>
+#include <stdint.h>
int
main ()
@@ -26,6 +27,15 @@ main ()
if (p == NULL)
return 1;
+ /* Check that realloc (p, n) fails when p is non-null and n exceeds
+ PTRDIFF_MAX. */
+ if (PTRDIFF_MAX < SIZE_MAX)
+ {
+ size_t n = PTRDIFF_MAX, n1 = n + 1;
+ if (realloc (p, n1) != NULL)
+ return 1;
+ }
+
free (p);
return 0;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 22:33 ` Paul Eggert
@ 2021-04-19 0:04 ` Bruno Haible
2021-04-19 4:20 ` Paul Eggert
0 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-04-19 0:04 UTC (permalink / raw)
To: Paul Eggert; +Cc: bug-gnulib
Paul Eggert wrote:
> I installed the attached instead, as this is simpler.
Thanks!
> If there are
> problems with PTRDIFF_MAX + 1, the test program might thrash or maybe
> even crash the kernel, but that's good enough since there shouldn't be
> problems.
Agree.
Bruno
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-19 0:04 ` Bruno Haible
@ 2021-04-19 4:20 ` Paul Eggert
0 siblings, 0 replies; 17+ messages in thread
From: Paul Eggert @ 2021-04-19 4:20 UTC (permalink / raw)
To: Bruno Haible; +Cc: bug-gnulib
[-- Attachment #1: Type: text/plain, Size: 389 bytes --]
On 4/18/21 5:04 PM, Bruno Haible wrote:
> Paul Eggert wrote:
>> I installed the attached instead, as this is simpler.
And I'm already starting to wonder whether testing for ptrdiff_t
overflow was such a good idea. I installed the attached further patch to
try to pacify GCC diagnostics about them.
If these ptrdiff_t tests turn out to be too much trouble we can always
withdraw them.
[-- Attachment #2: 0001-malloc-gnu-tests-pacify-Walloc-size-larger-than.patch --]
[-- Type: text/x-patch, Size: 2764 bytes --]
From 11e793833378de5a5978b296af1c0993f7305eda Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Apr 2021 21:18:00 -0700
Subject: [PATCH] malloc-gnu-tests: pacify -Walloc-size-larger-than
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* tests/test-malloc-gnu.c (main):
* tests/test-realloc-gnu.c (main): Hide true intentions from GCC,
to prevent diagnostics like “warning: argument 1 value
‘9223372036854775808’ exceeds maximum object size
9223372036854775807 [-Walloc-size-larger-than=]”.
---
ChangeLog | 7 +++++++
tests/test-malloc-gnu.c | 6 +++---
tests/test-realloc-gnu.c | 6 +++---
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 2c7edd59a..0146e1594 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2021-04-18 Paul Eggert <eggert@cs.ucla.edu>
+ malloc-gnu-tests: pacify -Walloc-size-larger-than
+ * tests/test-malloc-gnu.c (main):
+ * tests/test-realloc-gnu.c (main): Hide true intentions from GCC,
+ to prevent diagnostics like “warning: argument 1 value
+ ‘9223372036854775808’ exceeds maximum object size
+ 9223372036854775807 [-Walloc-size-larger-than=]”.
+
safe-alloc: fix pointer implementation
The old implementation assumed that all pointers use the same
internal representation, but the C standard doesn’t guarantee
diff --git a/tests/test-malloc-gnu.c b/tests/test-malloc-gnu.c
index ce7e4fec2..e1dfde452 100644
--- a/tests/test-malloc-gnu.c
+++ b/tests/test-malloc-gnu.c
@@ -20,7 +20,7 @@
#include <stdint.h>
int
-main ()
+main (int argc, char **argv)
{
/* Check that malloc (0) is not a NULL pointer. */
char *p = malloc (0);
@@ -31,8 +31,8 @@ main ()
/* Check that malloc (n) fails when n exceeds PTRDIFF_MAX. */
if (PTRDIFF_MAX < SIZE_MAX)
{
- size_t n = PTRDIFF_MAX, n1 = n + 1;
- if (malloc (n1) != NULL)
+ size_t one = argc != 12345;
+ if (malloc (PTRDIFF_MAX + one) != NULL)
return 1;
}
diff --git a/tests/test-realloc-gnu.c b/tests/test-realloc-gnu.c
index 9c7344f15..b62ee6bad 100644
--- a/tests/test-realloc-gnu.c
+++ b/tests/test-realloc-gnu.c
@@ -20,7 +20,7 @@
#include <stdint.h>
int
-main ()
+main (int argc, char **argv)
{
/* Check that realloc (NULL, 0) is not a NULL pointer. */
char *p = realloc (NULL, 0);
@@ -31,8 +31,8 @@ main ()
PTRDIFF_MAX. */
if (PTRDIFF_MAX < SIZE_MAX)
{
- size_t n = PTRDIFF_MAX, n1 = n + 1;
- if (realloc (p, n1) != NULL)
+ size_t one = argc != 12345;
+ if (realloc (p, PTRDIFF_MAX + one) != NULL)
return 1;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 2:02 ptrdiff_t overflow checks for malloc-posix etc Paul Eggert
` (3 preceding siblings ...)
2021-04-18 12:13 ` Bruno Haible
@ 2021-05-09 16:46 ` Bruno Haible
4 siblings, 0 replies; 17+ messages in thread
From: Bruno Haible @ 2021-05-09 16:46 UTC (permalink / raw)
To: bug-gnulib; +Cc: Paul Eggert
[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]
Paul Eggert wrote:
> I installed the attached patches into Gnulib to make its malloc
> replacements ptrdiff_t safe.
When testing m4-1.4.18b on IRIX 6.5, I get a test failure:
FAIL: test-reallocarray
Let's look in detail:
$ ./test-reallocarray ; echo $?
2
There is a call
p = realloc (NULL, 2*1073741824);
which returns NULL with errno being 0.
Since the 'reallocarray' module depends on 'realloc-gnu', and the
'realloc-gnu' and 'realloc-posix' documentation says:
Portability problems fixed by Gnulib:
@itemize
@item
Upon failure, the function does not set @code{errno} to @code{ENOMEM} on
some platforms:
mingw, MSVC 14.
@item
On some platforms, @code{realloc (p, n)} can succeed even if @code{n}
exceeds @code{PTRDIFF_MAX}. Although this behavior is arguably
allowed by POSIX it can lead to behavior not defined by POSIX later,
so @code{realloc-posix} does not allow going over the limit.
@end itemize
So, what the documentation implies and what the reallocarray unit test
verifies is that
realloc (NULL, n) where n > PTRDIFF_MAX
1) returns NULL and
2) sets errno to ENOMEM.
On IRIX (in n32 ABI), expectation 1) is fulfilled but 2) is not.
Likewise for malloc and calloc.
I'm adding two patches
- to make sure that the 'realloc-gnu' unit test already fails in this
situation,
- to fix 'realloc-gnu' on IRIX, so that it actually compiles the
replacement code lib/realloc.c.
2021-05-09 Bruno Haible <bruno@clisp.org>
malloc-gnu, realloc-gnu, calloc-gnu: Ensure errno gets set on IRIX.
* m4/malloc.m4 (gl_CHECK_MALLOC_POSIX): Require AC_CANONICAL_HOST. Set
gl_cv_func_malloc_posix to 'no' also on IRIX.
malloc-gnu, realloc-gnu, calloc-gnu tests: Verify errno is set.
* tests/test-malloc-gnu.c: Include <errno.h>.
(main): Verify that, when an allocation larger than PTRDIFF_MAX failed,
errno is ENOMEM.
* tests/test-realloc-gnu.c: Likewise.
* tests/test-calloc-gnu.c: Likewise.
[-- Attachment #2: 0001-malloc-gnu-realloc-gnu-calloc-gnu-tests-Verify-errno.patch --]
[-- Type: text/x-patch, Size: 2955 bytes --]
From 3189c490ec29a74e91e1b800d63fb7c9f9b47d2b Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sun, 9 May 2021 18:34:58 +0200
Subject: [PATCH 1/2] malloc-gnu, realloc-gnu, calloc-gnu tests: Verify errno
is set.
* tests/test-malloc-gnu.c: Include <errno.h>.
(main): Verify that, when an allocation larger than PTRDIFF_MAX failed,
errno is ENOMEM.
* tests/test-realloc-gnu.c: Likewise.
* tests/test-calloc-gnu.c: Likewise.
---
ChangeLog | 9 +++++++++
tests/test-calloc-gnu.c | 6 ++++--
tests/test-malloc-gnu.c | 4 +++-
tests/test-realloc-gnu.c | 4 +++-
4 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e84bc51..e69f447 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
2021-05-09 Bruno Haible <bruno@clisp.org>
+ malloc-gnu, realloc-gnu, calloc-gnu tests: Verify errno is set.
+ * tests/test-malloc-gnu.c: Include <errno.h>.
+ (main): Verify that, when an allocation larger than PTRDIFF_MAX failed,
+ errno is ENOMEM.
+ * tests/test-realloc-gnu.c: Likewise.
+ * tests/test-calloc-gnu.c: Likewise.
+
+2021-05-09 Bruno Haible <bruno@clisp.org>
+
getrandom: Fail with ENOSYS when the system has no randomness source.
* lib/getrandom.c (getrandom): When open() fails, set errno to ENOSYS.
diff --git a/tests/test-calloc-gnu.c b/tests/test-calloc-gnu.c
index b46e788..dbef019 100644
--- a/tests/test-calloc-gnu.c
+++ b/tests/test-calloc-gnu.c
@@ -17,6 +17,8 @@
#include <config.h>
#include <stdlib.h>
+
+#include <errno.h>
#include <stdint.h>
/* Return N.
@@ -56,10 +58,10 @@ main ()
for (size_t n = 2; n != 0; n <<= 1)
{
void *volatile p = calloc (PTRDIFF_MAX / n + 1, identity (n));
- if (p != NULL)
+ if (!(p == NULL && errno == ENOMEM))
return 2;
p = calloc (SIZE_MAX / n + 1, identity (n));
- if (p != NULL)
+ if (!(p == NULL && errno == ENOMEM))
return 3;
}
}
diff --git a/tests/test-malloc-gnu.c b/tests/test-malloc-gnu.c
index d8e7b04..13217c1 100644
--- a/tests/test-malloc-gnu.c
+++ b/tests/test-malloc-gnu.c
@@ -17,6 +17,8 @@
#include <config.h>
#include <stdlib.h>
+
+#include <errno.h>
#include <stdint.h>
int
@@ -33,7 +35,7 @@ main (int argc, char **argv)
{
size_t one = argc != 12345;
p = malloc (PTRDIFF_MAX + one);
- if (p != NULL)
+ if (!(p == NULL && errno == ENOMEM))
return 1;
}
diff --git a/tests/test-realloc-gnu.c b/tests/test-realloc-gnu.c
index f4c00c0..a366738 100644
--- a/tests/test-realloc-gnu.c
+++ b/tests/test-realloc-gnu.c
@@ -17,6 +17,8 @@
#include <config.h>
#include <stdlib.h>
+
+#include <errno.h>
#include <stdint.h>
int
@@ -33,7 +35,7 @@ main (int argc, char **argv)
{
size_t one = argc != 12345;
p = realloc (p, PTRDIFF_MAX + one);
- if (p != NULL)
+ if (!(p == NULL && errno == ENOMEM))
return 1;
}
--
2.7.4
[-- Attachment #3: 0002-malloc-gnu-realloc-gnu-calloc-gnu-Ensure-errno-gets-.patch --]
[-- Type: text/x-patch, Size: 3495 bytes --]
From bad7a098679f6bfd8573d14e1722765977436643 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sun, 9 May 2021 18:36:41 +0200
Subject: [PATCH 2/2] malloc-gnu, realloc-gnu, calloc-gnu: Ensure errno gets
set on IRIX.
* m4/malloc.m4 (gl_CHECK_MALLOC_POSIX): Require AC_CANONICAL_HOST. Set
gl_cv_func_malloc_posix to 'no' also on IRIX.
---
ChangeLog | 4 ++++
m4/malloc.m4 | 56 +++++++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e69f447..b24976a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2021-05-09 Bruno Haible <bruno@clisp.org>
+ malloc-gnu, realloc-gnu, calloc-gnu: Ensure errno gets set on IRIX.
+ * m4/malloc.m4 (gl_CHECK_MALLOC_POSIX): Require AC_CANONICAL_HOST. Set
+ gl_cv_func_malloc_posix to 'no' also on IRIX.
+
malloc-gnu, realloc-gnu, calloc-gnu tests: Verify errno is set.
* tests/test-malloc-gnu.c: Include <errno.h>.
(main): Verify that, when an allocation larger than PTRDIFF_MAX failed,
diff --git a/m4/malloc.m4 b/m4/malloc.m4
index c09006d..de1b2c6 100644
--- a/m4/malloc.m4
+++ b/m4/malloc.m4
@@ -1,4 +1,4 @@
-# malloc.m4 serial 24
+# malloc.m4 serial 25
dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -115,20 +115,54 @@ AC_DEFUN([gl_FUNC_MALLOC_POSIX],
# Set gl_cv_func_malloc_posix to yes or no accordingly.
AC_DEFUN([gl_CHECK_MALLOC_POSIX],
[
+ AC_REQUIRE([AC_CANONICAL_HOST])
AC_CACHE_CHECK([whether malloc, realloc, calloc set errno on failure],
[gl_cv_func_malloc_posix],
[
dnl It is too dangerous to try to allocate a large amount of memory:
dnl some systems go to their knees when you do that. So assume that
- dnl all Unix implementations of the function set errno on failure.
- AC_COMPILE_IFELSE(
- [AC_LANG_PROGRAM(
- [[]],
- [[#if defined _WIN32 && ! defined __CYGWIN__
- choke me
- #endif
- ]])],
- [gl_cv_func_malloc_posix=yes],
- [gl_cv_func_malloc_posix=no])
+ dnl all Unix implementations of the function set errno on failure,
+ dnl except on those platforms where we have seen 'test-malloc-gnu',
+ dnl 'test-realloc-gnu', 'test-calloc-gnu' fail.
+ case "$host_os" in
+ mingw*)
+ gl_cv_func_malloc_posix=no ;;
+ irix*)
+ dnl The three functions return NULL with errno unset when the
+ dnl argument is larger than PTRDIFF_MAX. Here is a test program:
+m4_divert_push([KILL])
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#define ptrdiff_t long
+#ifndef PTRDIFF_MAX
+# define PTRDIFF_MAX ((ptrdiff_t) ((1UL << (8 * sizeof (ptrdiff_t) - 1)) - 1))
+#endif
+
+int main ()
+{
+ void *p;
+
+ fprintf (stderr, "PTRDIFF_MAX = %lu\n", (unsigned long) PTRDIFF_MAX);
+
+ errno = 0;
+ p = malloc ((unsigned long) PTRDIFF_MAX + 1);
+ fprintf (stderr, "p=%p errno=%d\n", p, errno);
+
+ errno = 0;
+ p = calloc (PTRDIFF_MAX / 2 + 1, 2);
+ fprintf (stderr, "p=%p errno=%d\n", p, errno);
+
+ errno = 0;
+ p = realloc (NULL, (unsigned long) PTRDIFF_MAX + 1);
+ fprintf (stderr, "p=%p errno=%d\n", p, errno);
+
+ return 0;
+}
+m4_divert_pop([KILL])
+ gl_cv_func_malloc_posix=no ;;
+ *)
+ gl_cv_func_malloc_posix=yes ;;
+ esac
])
])
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-04-18 18:37 ` Paul Eggert
@ 2021-05-09 20:19 ` Bruno Haible
2021-05-09 23:00 ` Bruno Haible
0 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-05-09 20:19 UTC (permalink / raw)
To: Paul Eggert; +Cc: bug-gnulib
Hi Paul,
On 2021-04-18 you replied:
> > if test $REPLACE_REALLOC = 0; then
> > _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
> > fi
>
> I confess I don't like the style as much: it makes the shell code a bit
> less readable, at least to me. But it appears that this style isn't
> needed anyway.
Sorry, but the style
test $REPLACE_REALLOC = 1 || <some big macro invocation>
is buggy. Let's take, as example, the current GNU m4 snapshot. Its
configuration produces output like this:
...
checking for libsigsegv... yes
checking how to link with libsigsegv... /inst-x86_64-x32/lib/libsigsegv.a
yes
checking whether this system supports file names of any length... no
...
checking whether readlink truncates results correctly... yes
yes
checking for reallocarray... no
...
(or 'no' instead of 'yes' on some non-glibc systems).
When I look into the generated configure file I see this for the first
lonely 'yes':
--------------------------------------------------------------------------------
test $REPLACE_CALLOC = 1 ||
{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether calloc (0, n) and calloc (n, 0) return nonnull" >&5
printf %s "checking whether calloc (0, n) and calloc (n, 0) return nonnull... " >&6; }
if test ${ac_cv_func_calloc_0_nonnull+y}
...
fi
{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_func_calloc_0_nonnull" >&5
printf "%s\n" "$ac_cv_func_calloc_0_nonnull" >&6; }
case $ac_cv_func_calloc_0_nonnull in #(
*yes) :
;; #(
*) :
REPLACE_CALLOC=1 ;;
esac
--------------------------------------------------------------------------------
and this for the second lonely 'yes':
--------------------------------------------------------------------------------
test $REPLACE_REALLOC = 1 ||
{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether realloc (0, 0) returns nonnull" >&5
printf %s "checking whether realloc (0, 0) returns nonnull... " >&6; }
if test ${ac_cv_func_realloc_0_nonnull+y}
...
{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_func_realloc_0_nonnull" >&5
printf "%s\n" "$ac_cv_func_realloc_0_nonnull" >&6; }
case $ac_cv_func_realloc_0_nonnull in #(
*yes) :
;; #(
*) :
REPLACE_REALLOC=1 ;;
esac
--------------------------------------------------------------------------------
As you can see, this logic has caused the first two lines after the 'test'
to be skipped, but the next lines were executed although they shouldn't.
So, not only the configure output was wrong; also the logic which statements
of the configure file get executed was wrong.
I have to commit this fix. The alternative, to force some macros
expand into a statement group enclosed by { ... }, would be fragile.
With this fix, the configure output looks right:
...
checking for libsigsegv... yes
checking how to link with libsigsegv... /inst-x86_64-x32/lib/libsigsegv.a
checking whether calloc (0, n) and calloc (n, 0) return nonnull... yes
checking whether this system supports file names of any length... no
...
checking whether readlink truncates results correctly... yes
checking whether realloc (0, 0) returns nonnull... yes
checking for reallocarray... no
...
2021-05-09 Bruno Haible <bruno@clisp.org>
{malloc,realloc,calloc}-gnu: Fix autoconf macro (regression 2021-04-18).
* m4/malloc.m4 (gl_FUNC_MALLOC_GNU): Don't assume that
_AC_FUNC_MALLOC_IF expands to a single shell statement.
* m4/realloc.m4 (gl_FUNC_REALLOC_GNU): Don't assume that
_AC_FUNC_REALLOC_IF expands to a single shell statement.
* m4/calloc.m4 (gl_FUNC_CALLOC_GNU): Don't assume that
_AC_FUNC_CALLOC_IF expands to a single shell statement.
diff --git a/m4/calloc.m4 b/m4/calloc.m4
index 2f0abee..7575a69 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -1,4 +1,4 @@
-# calloc.m4 serial 25
+# calloc.m4 serial 26
# Copyright (C) 2004-2021 Free Software Foundation, Inc.
# This file is free software; the Free Software Foundation
@@ -58,7 +58,9 @@ AC_DEFUN([gl_FUNC_CALLOC_GNU],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
AC_REQUIRE([gl_FUNC_CALLOC_POSIX])
- test $REPLACE_CALLOC = 1 || _AC_FUNC_CALLOC_IF([], [REPLACE_CALLOC=1])
+ if test $REPLACE_CALLOC = 0; then
+ _AC_FUNC_CALLOC_IF([], [REPLACE_CALLOC=1])
+ fi
])# gl_FUNC_CALLOC_GNU
# gl_FUNC_CALLOC_POSIX
diff --git a/m4/malloc.m4 b/m4/malloc.m4
index de1b2c6..6fcd4ad 100644
--- a/m4/malloc.m4
+++ b/m4/malloc.m4
@@ -1,4 +1,4 @@
-# malloc.m4 serial 25
+# malloc.m4 serial 26
dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -43,7 +43,9 @@ AC_DEFUN([gl_FUNC_MALLOC_GNU],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
- test $REPLACE_MALLOC = 1 || _AC_FUNC_MALLOC_IF([], [REPLACE_MALLOC=1])
+ if test $REPLACE_MALLOC = 0; then
+ _AC_FUNC_MALLOC_IF([], [REPLACE_MALLOC=1])
+ fi
])
# gl_FUNC_MALLOC_PTRDIFF
diff --git a/m4/realloc.m4 b/m4/realloc.m4
index 8eb6b19..9925917 100644
--- a/m4/realloc.m4
+++ b/m4/realloc.m4
@@ -1,4 +1,4 @@
-# realloc.m4 serial 22
+# realloc.m4 serial 23
dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -43,7 +43,9 @@ AC_DEFUN([gl_FUNC_REALLOC_GNU],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
AC_REQUIRE([gl_FUNC_REALLOC_POSIX])
- test $REPLACE_REALLOC = 1 || _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
+ if test $REPLACE_REALLOC = 0; then
+ _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
+ fi
])# gl_FUNC_REALLOC_GNU
# gl_FUNC_REALLOC_POSIX
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-05-09 20:19 ` Bruno Haible
@ 2021-05-09 23:00 ` Bruno Haible
2021-05-10 8:45 ` Paul Eggert
0 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-05-09 23:00 UTC (permalink / raw)
To: Paul Eggert; +Cc: bug-gnulib
> 2021-05-09 Bruno Haible <bruno@clisp.org>
>
> {malloc,realloc,calloc}-gnu: Fix autoconf macro (regression 2021-04-18).
Even after this is fixed, I see a test failure (building the newest m4)
on AIX in 64-bit mode: test-calloc-gnu fails with exit code 1.
Looking at the details: The configure output has
checking whether calloc (0, n) and calloc (n, 0) return nonnull... no
which indicates that the configure script has noticed that it should
enable the calloc() replacement. But config.status has
S["REPLACE_MALLOC"]="1"
...
S["REPLACE_CALLOC"]="0"
...
S["REPLACE_REALLOC"]="1"
On AIX, all three functions ought to be replaced. Why is REPLACE_CALLOC zero?
Let's look at the assignments to REPLACE_CALLOC and the uses of REPLACE_MALLOC
in the configure script:
12111: REPLACE_CALLOC=0;
12115: REPLACE_MALLOC=0;
12192: test "$gl_cv_malloc_ptrdiff" = yes || REPLACE_MALLOC=1
12226: REPLACE_MALLOC=1
12232: REPLACE_CALLOC=$REPLACE_MALLOC
32513: if test $REPLACE_CALLOC = 0; then
32577: REPLACE_CALLOC=1 ;;
32582: if test $REPLACE_CALLOC = 1; then
32597: REPLACE_CALLOC=$REPLACE_MALLOC
32599: if test $REPLACE_CALLOC = 1; then
39364: if test $REPLACE_MALLOC = 0; then
39419: REPLACE_MALLOC=1 ;;
39424: if test $REPLACE_MALLOC = 1; then
39437: if test $REPLACE_MALLOC = 1; then
And when I execute the configure script with 'sh -x', I get these assignments
in sequence:
+ REPLACE_CALLOC=0
+ REPLACE_CALLOC=0
+ REPLACE_CALLOC=1
+ REPLACE_CALLOC=0
So, REPLACE_CALLOC gets set to 1 but then it gets set to 0 later.
There are two ways to fix this:
(a) Never set REPLACE_CALLOC to 0, except in the gl_STDLIB_H_DEFAULTS macro.
(b) Ensure that gl_FUNC_CALLOC_POSIX gets expanded only once, before
gl_FUNC_CALLOC_GNU.
I prefer (a), because that is the common practice already: The REPLACE_*
variables are getting set to 0 early, and then any macro can set it to 1
depending on whatever conditions.
This patch does it, and fixes the AIX 64-bit issue.
2021-05-09 Bruno Haible <bruno@clisp.org>
{realloc,calloc}-gnu: Fix autoconf macro (regression 2021-04-18).
* m4/realloc.m4 (gl_FUNC_REALLOC_POSIX): Don't reset REPLACE_REALLOC
to 0 if it is already 1 after gl_FUNC_REALLOC_GNU was executed.
* m4/calloc.m4 (gl_FUNC_CALLOC_POSIX): Don't reset REPLACE_CALLOC
to 0 if it is already 1 after gl_FUNC_CALLOC_GNU was executed.
diff --git a/m4/calloc.m4 b/m4/calloc.m4
index 7575a69..fe12b15 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -1,4 +1,4 @@
-# calloc.m4 serial 26
+# calloc.m4 serial 27
# Copyright (C) 2004-2021 Free Software Foundation, Inc.
# This file is free software; the Free Software Foundation
@@ -72,7 +72,9 @@ AC_DEFUN([gl_FUNC_CALLOC_POSIX],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
- REPLACE_CALLOC=$REPLACE_MALLOC
+ if test $REPLACE_MALLOC = 1; then
+ REPLACE_CALLOC=1
+ fi
dnl Although in theory we should also test for size_t overflow,
dnl in practice testing for ptrdiff_t overflow suffices
dnl since PTRDIFF_MAX <= SIZE_MAX on all known Gnulib porting targets.
diff --git a/m4/realloc.m4 b/m4/realloc.m4
index 9925917..0abc418 100644
--- a/m4/realloc.m4
+++ b/m4/realloc.m4
@@ -1,4 +1,4 @@
-# realloc.m4 serial 23
+# realloc.m4 serial 24
dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -57,5 +57,7 @@ AC_DEFUN([gl_FUNC_REALLOC_POSIX],
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
- REPLACE_REALLOC=$REPLACE_MALLOC
+ if test $REPLACE_MALLOC = 1; then
+ REPLACE_REALLOC=1
+ fi
])
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ptrdiff_t overflow checks for malloc-posix etc.
2021-05-09 23:00 ` Bruno Haible
@ 2021-05-10 8:45 ` Paul Eggert
0 siblings, 0 replies; 17+ messages in thread
From: Paul Eggert @ 2021-05-10 8:45 UTC (permalink / raw)
To: Bruno Haible; +Cc: bug-gnulib
Thanks for all those fixes (and the detailed reports!).
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-05-10 8:45 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-18 2:02 ptrdiff_t overflow checks for malloc-posix etc Paul Eggert
2021-04-18 11:59 ` Bruno Haible
2021-04-18 12:11 ` Bruno Haible
2021-04-18 20:24 ` Paul Eggert
2021-04-18 20:32 ` Bruno Haible
2021-04-18 12:13 ` Bruno Haible
2021-04-18 19:47 ` Paul Eggert
2021-04-18 20:23 ` Bruno Haible
2021-04-18 22:33 ` Paul Eggert
2021-04-19 0:04 ` Bruno Haible
2021-04-19 4:20 ` Paul Eggert
2021-04-18 12:13 ` Bruno Haible
2021-04-18 18:37 ` Paul Eggert
2021-05-09 20:19 ` Bruno Haible
2021-05-09 23:00 ` Bruno Haible
2021-05-10 8:45 ` Paul Eggert
2021-05-09 16:46 ` Bruno Haible
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).