Paul Eggert wrote:
> > Well, if we go for some VARIABLE=VALUE option that is not visible through
> > './configure --help', it could be the name of some cache variable:
> > gl_cv_realloc_sanitize=yes
> >
> > Would that be OK?
>
> Yes, I think so.
Implemented through these two patches.
The first patch does the realloc modification. The second patch then fixes
a test-reallocarray failure on Solaris:
FAIL: test-reallocarray
=======================
Stack trace:
0x4071b1 print_stack_trace_to
../../gllib/stack-trace-impl.h:33
0x4071b1 rpl_abort
../../gllib/abort-debug.c:36
0x407314 rpl_realloc
../../gllib/realloc.c:53
0x4070fa main
../../gltests/test-reallocarray.c:53
../../build-aux/test-driver: line 121: 16106: Abort(coredump)
FAIL test-reallocarray (exit status: 262)
How to use this option:
$ gl_cv_func_realloc_sanitize=yes ../configure ...
I tested it
- with a testdir of all of gnulib,
- with GNU gettext
and found no other occurrence of the undefined behaviour case.
That's probably because
- C strings allocations are at least 1 byte large,
- Well-optimized programs avoid memory allocations for 0 bytes of data.
2024-10-21 Bruno Haible <[email protected]>
reallocarray: Don't assume unportable behaviour of realloc.
* lib/reallocarray.c (reallocarray): Handle the nbytes==0 case
explicitly.
* modules/reallocarray (Depends-on): Remove realloc-gnu. Add
malloc-posix, realloc-posix.
realloc: Optionally check for undefined behaviour.
* m4/realloc.m4 (gl_FUNC_REALLOC_SANITIZED): New macro.
(gl_FUNC_REALLOC_POSIX): Require it. If a sanitized realloc is
requested, define NEED_SANITIZED_REALLOC and compile realloc.c.
(gl_FUNC_REALLOC_GNU): Require it. If a sanitized realloc is requested,
don't compile realloc.c a second time.
* lib/realloc.c (rpl_realloc): If NEED_SANITIZED_REALLOC is defined,
abort in the case of undefined behaviour.
From 12cd6eca2058105f81c3a5dc739e82f965ca18bf Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email protected]>
Date: Mon, 21 Oct 2024 15:29:40 +0200
Subject: [PATCH 1/2] realloc: Optionally check for undefined behaviour.
* m4/realloc.m4 (gl_FUNC_REALLOC_SANITIZED): New macro.
(gl_FUNC_REALLOC_POSIX): Require it. If a sanitized realloc is
requested, define NEED_SANITIZED_REALLOC and compile realloc.c.
(gl_FUNC_REALLOC_GNU): Require it. If a sanitized realloc is requested,
don't compile realloc.c a second time.
* lib/realloc.c (rpl_realloc): If NEED_SANITIZED_REALLOC is defined,
abort in the case of undefined behaviour.
---
ChangeLog | 11 +++++++++++
lib/realloc.c | 11 +++++++++++
m4/realloc.m4 | 27 ++++++++++++++++++++++++---
3 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 0b7094e650..4e5e61b654 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2024-10-21 Bruno Haible <[email protected]>
+
+ realloc: Optionally check for undefined behaviour.
+ * m4/realloc.m4 (gl_FUNC_REALLOC_SANITIZED): New macro.
+ (gl_FUNC_REALLOC_POSIX): Require it. If a sanitized realloc is
+ requested, define NEED_SANITIZED_REALLOC and compile realloc.c.
+ (gl_FUNC_REALLOC_GNU): Require it. If a sanitized realloc is requested,
+ don't compile realloc.c a second time.
+ * lib/realloc.c (rpl_realloc): If NEED_SANITIZED_REALLOC is defined,
+ abort in the case of undefined behaviour.
+
2024-10-19 Paul Eggert <[email protected]>
realloc: more improvements for realloc (p, 0)
diff --git a/lib/realloc.c b/lib/realloc.c
index 0573139625..5cfe3bc747 100644
--- a/lib/realloc.c
+++ b/lib/realloc.c
@@ -42,8 +42,19 @@ rpl_realloc (void *p, size_t n)
if (n == 0)
{
+#if NEED_SANITIZED_REALLOC
+ /* ISO C 23 ยง 7.24.3.7.(3) says that this case is undefined behaviour.
+ Let the programmer know that it occurred.
+ When the undefined-behaviour sanitizers report this case, i.e. when
+ <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117233> and
+ <https://github.com/llvm/llvm-project/issues/113065>
+ have been closed and new releases of GCC and clang have been made,
+ we can remove this code here. */
+ abort ();
+#else
free (p);
return NULL;
+#endif
}
if (xalloc_oversized (n, 1))
diff --git a/m4/realloc.m4 b/m4/realloc.m4
index e25f329c50..f4d78f9622 100644
--- a/m4/realloc.m4
+++ b/m4/realloc.m4
@@ -1,11 +1,23 @@
# realloc.m4
-# serial 32
+# serial 33
dnl Copyright (C) 2007, 2009-2024 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.
dnl This file is offered as-is, without any warranty.
+# An an experimental option, the user can request a sanitized realloc()
+# implementation, i.e. one that aborts upon undefined behaviour,
+# by setting
+# gl_cv_func_realloc_sanitize=yes
+# at configure time.
+AC_DEFUN([gl_FUNC_REALLOC_SANITIZED],
+[
+ AC_CACHE_CHECK([whether realloc should abort upon undefined behaviour],
+ [gl_cv_func_realloc_sanitize],
+ [test -n "$gl_cv_func_realloc_sanitize" || gl_cv_func_realloc_sanitize=no])
+])
+
# This is adapted with modifications from upstream Autoconf here:
# https://git.savannah.gnu.org/cgit/autoconf.git/tree/lib/autoconf/functions.m4?id=v2.70#n1455
AC_DEFUN([_AC_FUNC_REALLOC_IF],
@@ -51,7 +63,9 @@ AC_DEFUN([gl_FUNC_REALLOC_GNU]
dnl gets defined already before this macro gets invoked. This helps
dnl if !(__VEC__ || __AIXVEC), and doesn't hurt otherwise.
- if test $REPLACE_REALLOC_FOR_REALLOC_GNU = 0; then
+ AC_REQUIRE([gl_FUNC_REALLOC_SANITIZED])
+ if test "$gl_cv_func_realloc_sanitize" = no \
+ && test $REPLACE_REALLOC_FOR_REALLOC_GNU = 0; then
_AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC_FOR_REALLOC_GNU=1])
fi
])# gl_FUNC_REALLOC_GNU
@@ -65,7 +79,14 @@ AC_DEFUN([gl_FUNC_REALLOC_POSIX]
[
AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
- if test $REPLACE_MALLOC_FOR_MALLOC_POSIX = 1; then
+ AC_REQUIRE([gl_FUNC_REALLOC_SANITIZED])
+ if test "$gl_cv_func_realloc_sanitize" != no; then
REPLACE_REALLOC_FOR_REALLOC_POSIX=1
+ AC_DEFINE([NEED_SANITIZED_REALLOC], [1],
+ [Define to 1 if realloc should abort upon undefined behaviour.])
+ else
+ if test $REPLACE_MALLOC_FOR_MALLOC_POSIX = 1; then
+ REPLACE_REALLOC_FOR_REALLOC_POSIX=1
+ fi
fi
])
--
2.34.1
>From b3791b2bb9bdf03b2f99be5629d017a40382e1ee Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email protected]>
Date: Mon, 21 Oct 2024 17:20:37 +0200
Subject: [PATCH 2/2] reallocarray: Don't assume unportable behaviour of
realloc.
* lib/reallocarray.c (reallocarray): Handle the nbytes==0 case
explicitly.
* modules/reallocarray (Depends-on): Remove realloc-gnu. Add
malloc-posix, realloc-posix.
---
ChangeLog | 6 ++++++
lib/reallocarray.c | 14 +++++++++++++-
modules/reallocarray | 3 ++-
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 4e5e61b654..a9f4da9556 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2024-10-21 Bruno Haible <[email protected]>
+ reallocarray: Don't assume unportable behaviour of realloc.
+ * lib/reallocarray.c (reallocarray): Handle the nbytes==0 case
+ explicitly.
+ * modules/reallocarray (Depends-on): Remove realloc-gnu. Add
+ malloc-posix, realloc-posix.
+
realloc: Optionally check for undefined behaviour.
* m4/realloc.m4 (gl_FUNC_REALLOC_SANITIZED): New macro.
(gl_FUNC_REALLOC_POSIX): Require it. If a sanitized realloc is
diff --git a/lib/reallocarray.c b/lib/reallocarray.c
index 09711a0ede..e0377d0870 100644
--- a/lib/reallocarray.c
+++ b/lib/reallocarray.c
@@ -33,6 +33,18 @@ reallocarray (void *ptr, size_t nmemb, size_t size)
return NULL;
}
- /* Rely on the semantics of GNU realloc. */
+ /* Avoid calling realloc (ptr, 0), since that is undefined behaviour in
+ ISO C 23 and since the GNU libc behaviour may possibly change. */
+ if (nbytes == 0)
+ {
+ void *new_ptr = malloc (1);
+ if (new_ptr == NULL)
+ /* errno is set here. */
+ return NULL;
+ free (ptr);
+ return new_ptr;
+ }
+
+ /* Call realloc, setting errno to ENOMEM on failure. */
return realloc (ptr, nbytes);
}
diff --git a/modules/reallocarray b/modules/reallocarray
index 380434870e..dace5e636b 100644
--- a/modules/reallocarray
+++ b/modules/reallocarray
@@ -8,7 +8,8 @@ m4/reallocarray.m4
Depends-on:
extensions
-realloc-gnu [test $HAVE_REALLOCARRAY = 0 || test $REPLACE_REALLOCARRAY = 1]
+malloc-posix [test $HAVE_REALLOCARRAY = 0 || test $REPLACE_REALLOCARRAY = 1]
+realloc-posix [test $HAVE_REALLOCARRAY = 0 || test $REPLACE_REALLOCARRAY = 1]
stdckdint [test $HAVE_REALLOCARRAY = 0 || test $REPLACE_REALLOCARRAY = 1]
stdlib
--
2.34.1
