[PATCH] xmalloca, etc.: fix some xalloc-oversized issues

[Paul Eggert]

* lib/malloca.h (nmalloca):
* lib/xmalloca.h (xnmalloca): Convert S to ptrdiff_t to avoid
arithmetic overflow if N and S are both narrower than ptrdiff_t.
* lib/xalloc-oversized.h (xalloc_oversized):
Don’t say that args must be ptrdiff_t or size_t or wider.
The macro returns the correct answer even when that is not
the case, and it’s the caller’s responsibility to avoid
howlers like (xalloc_oversized (n, s) ? NULL : malloc (n * s))
when N and S are both narrower than ptrdiff_t and size_t.
Add a comment to that effect.
* lib/xmalloca.h: Include xalloc-oversized.h, since this file uses
xalloc_oversized.  Add comments about side effects and avoid
unnecessary parens.
* modules/xmalloca (Depends-on): Add xalloc-oversized.
---
 ChangeLog              | 16 ++++++++++++++++
 lib/malloca.h          |  4 ++--
 lib/xalloc-oversized.h | 18 +++++++++++++-----
 lib/xmalloca.h         | 10 ++++++----
 modules/xmalloca       |  1 +
 5 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c198830bf..63471166c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2021-04-24  Paul Eggert  <eggert@cs.ucla.edu>
 
+       xmalloca, etc.: fix some xalloc-oversized issues
+       * lib/malloca.h (nmalloca):
+       * lib/xmalloca.h (xnmalloca): Convert S to ptrdiff_t to avoid
+       arithmetic overflow if N and S are both narrower than ptrdiff_t.
+       * lib/xalloc-oversized.h (xalloc_oversized):
+       Don’t say that args must be ptrdiff_t or size_t or wider.
+       The macro returns the correct answer even when that is not
+       the case, and it’s the caller’s responsibility to avoid
+       howlers like (xalloc_oversized (n, s) ? NULL : malloc (n * s))
+       when N and S are both narrower than ptrdiff_t and size_t.
+       Add a comment to that effect.
+       * lib/xmalloca.h: Include xalloc-oversized.h, since this file uses
+       xalloc_oversized.  Add comments about side effects and avoid
+       unnecessary parens.
+       * modules/xmalloca (Depends-on): Add xalloc-oversized.
+
        reallocarray: check for ptrdiff_t overflow
        * doc/glibc-functions/reallocarray.texi (reallocarray):
        Mention ptrdiff_t overflow.
diff --git a/lib/malloca.h b/lib/malloca.h
index 16a156ba2..f9b30880d 100644
--- a/lib/malloca.h
+++ b/lib/malloca.h
@@ -77,9 +77,9 @@ extern void freea (void *p);
 /* nmalloca(N,S) is an overflow-safe variant of malloca (N * S).
    It allocates an array of N objects, each with S bytes of memory,
    on the stack.  S must be positive and N must be nonnegative.
-   Either N or S should be of type ptrdiff_t or size_t or wider.
    The array must be freed using freea() before the function returns.  */
-#define nmalloca(n, s) (xalloc_oversized (n, s) ? NULL : malloca ((n) * (s)))
+#define nmalloca(n, s) \
+  (xalloc_oversized (n, s) ? NULL : malloca ((n) * (ptrdiff_t) (s)))
 
 
 #ifdef __cplusplus
diff --git a/lib/xalloc-oversized.h b/lib/xalloc-oversized.h
index 6437a5d8b..62f1ae94b 100644
--- a/lib/xalloc-oversized.h
+++ b/lib/xalloc-oversized.h
@@ -30,11 +30,19 @@
 #define __xalloc_oversized(n, s) \
   ((size_t) (PTRDIFF_MAX < SIZE_MAX ? PTRDIFF_MAX : SIZE_MAX - 1) / (s) < (n))
 
-/* Return 1 if an array of N objects, each of size S, cannot exist reliably
-   because its total size in bytes exceeds MIN (PTRDIFF_MAX, SIZE_MAX - 1).
-   N must be nonnegative, S must be positive, and either N or S should be
-   of type ptrdiff_t or size_t or wider.  This is a macro, not a function,
-   so that it works even if an argument exceeds MAX (PTRDIFF_MAX, SIZE_MAX).  
*/
+/* Return 1 if and only if an array of N objects, each of size S,
+   cannot exist reliably because its total size in bytes would exceed
+   MIN (PTRDIFF_MAX, SIZE_MAX - 1).
+
+   N must be nonnegative and S must be positive.
+
+   Warning: (xalloc_oversized (N, S) ? NULL : malloc (N * S)) can
+   misbehave if N and S are both narrower than ptrdiff_t and size_t,
+   and can be rewritten as (xalloc_oversized (N, S) ?  NULL : malloc
+   (N * (ptrdiff_t) S)) or similarly with size_t.
+
+   This is a macro, not a function, so that it works even if an
+   argument exceeds MAX (PTRDIFF_MAX, SIZE_MAX).  */
 #if 7 <= __GNUC__ && !defined __clang__ && PTRDIFF_MAX < SIZE_MAX
 # define xalloc_oversized(n, s) \
    __builtin_mul_overflow_p (n, s, (ptrdiff_t) 1)
diff --git a/lib/xmalloca.h b/lib/xmalloca.h
index a87a336b6..c7815f617 100644
--- a/lib/xmalloca.h
+++ b/lib/xmalloca.h
@@ -20,6 +20,7 @@
 
 #include "malloca.h"
 #include "xalloc.h"
+#include "xalloc-oversized.h"
 
 
 #ifdef __cplusplus
@@ -29,7 +30,8 @@ extern "C" {
 
 /* xmalloca(N) is a checking safe variant of alloca(N).  It allocates N bytes
    of memory allocated on the stack, that must be freed using freea() before
-   the function returns.  Upon failure, it exits with an error message.  */
+   the function returns.  N should not have side effects.
+   Upon failure, it exits with an error message.  */
 #if HAVE_ALLOCA
 # define xmalloca(N) \
   ((N) < 4032 - (2 * sa_alignment_max - 1)                                   \
@@ -46,16 +48,16 @@ extern void * xmmalloca (size_t n);
 /* xnmalloca(N,S) is an overflow-safe variant of xmalloca (N * S).
    It allocates an array of N objects, each with S bytes of memory,
    on the stack.  S must be positive and N must be nonnegative,
-   and at least one of N and S should be ptrdiff_t or size_t or wider.
+   and S and N should not have side effects.
    The array must be freed using freea() before the function returns.
    Upon failure, it exits with an error message.  */
 #if HAVE_ALLOCA
 /* Rely on xmalloca (SIZE_MAX) calling xalloc_die ().  */
 # define xnmalloca(n, s) \
-    xmalloca (xalloc_oversized ((n), (s)) ? (size_t) (-1) : (n) * (s))
+    xmalloca (xalloc_oversized (n, s) ? (size_t) (-1) : (n) * (ptrdiff_t) (s))
 #else
 # define xnmalloca(n, s) \
-    xnmalloc ((n), (s))
+    xnmalloc (n, s)
 #endif
 
 
diff --git a/modules/xmalloca b/modules/xmalloca
index dac01f2d2..92b965b82 100644
--- a/modules/xmalloca
+++ b/modules/xmalloca
@@ -8,6 +8,7 @@ lib/xmalloca.c
 Depends-on:
 malloca
 xalloc
+xalloc-oversized
 
 configure.ac:
 
-- 
2.27.0