Re: Warning in lib/stdlib.c

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]

On 2024-12-21 05:10, Eli Zaretskii wrote:

>    stdlib.c:20: warning: "_GL_STDLIB_INLINE" redefined

That's a violation of the C standard and so should be fixed.

The real culprit here is that Emacs config.h includes <stdlib.h>. It's 
generally not a good idea for config.h to include headers like 
<stdlib.h>, as that's brittle (as shown here).

I adjusted Gnulib to work around this particular problem and merged that 
into Emacs. However, the culprit may cause other lurking issues so I 
also installed the attached patches to Emacs so that config.h no longer 
includes <stdlib.h>.

In <https://bugs.gnu.org/52455#21> I just now proposed removing 
no-longer-working code that attempts to support unexec on macOS. That 
would have simplified the attached patches.

It would also simplify things to remove the HYBRID_MALLOC code but I 
don't know whether that stuff is still used.

[-- Attachment #2: 0001-Let-Gnulib-deal-with-MB_CUR_MAX.patch --]
[-- Type: text/x-patch, Size: 3125 bytes --]

From a51642ba5fad891239552fe7e746fa160e3f88ea Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 23 Dec 2024 14:03:06 -0800
Subject: [PATCH 1/2] Let Gnulib deal with MB_CUR_MAX

Recent Gnulib should work around the Android NDK 16
MB_CUR_MAX bug, so Emacs no longer needs to worry about it.
* configure.ac (gl_STDLIB_H):
Remove, to stop overriding Gnulib.
* configure.ac (__ctype_get_mb_cur_max)
(emacs_cv_broken_mb_cur_max, REPLACEMENT_MB_CUR_MAX):
* src/conf_post.h (MB_CUR_MAX):
Remove, as Gnulib does this now.
---
 configure.ac    | 33 ---------------------------------
 src/conf_post.h | 10 ----------
 2 files changed, 43 deletions(-)

diff --git a/configure.ac b/configure.ac
index 18cbb42b0ef..bfef9cd294d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1615,10 +1615,6 @@ AC_DEFUN
 AC_DEFUN([gt_TYPE_WINT_T],
   [GNULIBHEADERS_OVERRIDE_WINT_T=0
    AC_SUBST([GNULIBHEADERS_OVERRIDE_WINT_T])])
-# Emacs does not need precise checks for the Solaris 10 MB_CUR_MAX bug.
-AC_DEFUN_ONCE([gl_STDLIB_H],
-  [AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
-   gl_NEXT_HEADERS([stdlib.h])])
 # Emacs does not need to check for off64_t.
 AC_DEFUN([gl_TYPE_OFF64_T],
   [HAVE_OFF64_T=1
@@ -7646,35 +7642,6 @@ AC_DEFUN
 
 AC_SUBST([WINDOW_SYSTEM_OBJ])
 
-# Some systems have MB_CUR_MAX defined to a call to
-# __ctype_get_mb_cur_max, but do not have __ctype_get_mb_cur_max in
-# libc.  Check for that situation and define MB_CUR_MAX to something
-# sane.
-
-AC_CHECK_FUNC([__ctype_get_mb_cur_max])
-
-AC_CACHE_CHECK([whether MB_CUR_MAX is defined to function that won't link],
-  [emacs_cv_broken_mb_cur_max],
-  [AC_EGREP_CPP(__ctype_get_mb_cur_max, [
-#include <stdlib.h>
-#ifndef MB_CUR_MAX
-#define MB_CUR_MAX -1
-#endif
-static int foo = MB_CUR_MAX;
-], [AS_IF([test "$ac_cv_func___ctype_get_mb_cur_max" = "yes"],
-     [emacs_cv_broken_mb_cur_max=no],
-     [emacs_cv_broken_mb_cur_max=yes])],
-  [emacs_cv_broken_mb_cur_max=no])])
-
-AS_IF([test "$emacs_cv_broken_mb_cur_max" = "yes"],
- # Define this to 4, which is right for Android.
- [AS_CASE([$opsys], [android],
-   [AC_DEFINE([REPLACEMENT_MB_CUR_MAX], [4],
-      [Define to MB_CUR_MAX if stdlib.h is broken.])],
-   [AC_MSG_ERROR([MB_CUR_MAX does not work on your system.
-Please modify configure.ac to set an appropriate value, then
-send your change to bug-gnu-emacs@gnu.org])])])
-
 AH_TOP([/* GNU Emacs site configuration template file.
 
 Copyright (C) 1988-2024 Free Software Foundation, Inc.
diff --git a/src/conf_post.h b/src/conf_post.h
index 9d3f5711bef..6aa2c901cc7 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -462,16 +462,6 @@ #define VFORK() vfork ()
 # define UNINIT /* empty */
 #endif
 
-/* MB_CUR_MAX is often broken on systems which copy-paste LLVM
-   headers, so replace its definition with a working one if
-   necessary.  */
-
-#ifdef REPLACEMENT_MB_CUR_MAX
-#include <stdlib.h>
-#undef MB_CUR_MAX
-#define MB_CUR_MAX REPLACEMENT_MB_CUR_MAX
-#endif /* REPLACEMENT_MB_CUR_MAX */
-
 /* Emacs does not need glibc strftime behavior for AM and PM
    indicators.  */
 #define REQUIRE_GNUISH_STRFTIME_AM_PM false
-- 
2.45.2


[-- Attachment #3: 0002-Don-t-include-stdlib.h-from-conf_post.h.patch --]
[-- Type: text/x-patch, Size: 5698 bytes --]

From 577714e3fe722625236ce060f53b5d76f7933454 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 26 Dec 2024 15:44:34 -0800
Subject: [PATCH 2/2] =?UTF-8?q?Don=E2=80=99t=20include=20stdlib.h=20from?=
 =?UTF-8?q?=20conf=5Fpost.h?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is brittle, as evinced by the recent problem with lib/stdlib.c.
* src/conf_post.h: Move potential inclusion of stdlib.h and
redefinitions of malloc, free, realloc, aligned_alloc, and calloc
from here ...
* src/lisp.h: ... to here.  Do not redefine the symbols
if UNEXMACOS_C is defined.
* src/unexmacosx.c: Do not undef malloc, realloc, free.
(UNEXMACOS_C): New symbol, to prevent lisp.h from defining them.
---
 src/conf_post.h  | 49 ------------------------------------------------
 src/gmalloc.c    |  2 +-
 src/lisp.h       | 35 ++++++++++++++++++++++++++++++++++
 src/unexmacosx.c |  7 +------
 4 files changed, 37 insertions(+), 56 deletions(-)

diff --git a/src/conf_post.h b/src/conf_post.h
index 6aa2c901cc7..e204a85b973 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -93,55 +93,6 @@ Copyright (C) 1988, 1993-1994, 1999-2002, 2004-2024 Free Software
 # define ADDRESS_SANITIZER false
 #endif
 
-#ifdef emacs
-/* We include stdlib.h here, because Gnulib's stdlib.h might redirect
-   'free' to its replacement, and we want to avoid that in unexec
-   builds.  Including it here will render its inclusion after config.h
-   a no-op.  */
-# if (defined DARWIN_OS && defined HAVE_UNEXEC) || defined HYBRID_MALLOC
-#  include <stdlib.h>
-# endif
-#endif
-
-#if defined DARWIN_OS && defined emacs && defined HAVE_UNEXEC
-# undef malloc
-# define malloc unexec_malloc
-# undef realloc
-# define realloc unexec_realloc
-# undef free
-# define free unexec_free
-
-extern void *unexec_malloc (size_t);
-extern void *unexec_realloc (void *, size_t);
-extern void unexec_free (void *);
-
-#endif
-
-/* If HYBRID_MALLOC is defined (e.g., on Cygwin), emacs will use
-   gmalloc before dumping and the system malloc after dumping.
-   hybrid_malloc and friends, defined in gmalloc.c, are wrappers that
-   accomplish this.  */
-#ifdef HYBRID_MALLOC
-#ifdef emacs
-#undef malloc
-#define malloc hybrid_malloc
-#undef realloc
-#define realloc hybrid_realloc
-#undef aligned_alloc
-#define aligned_alloc hybrid_aligned_alloc
-#undef calloc
-#define calloc hybrid_calloc
-#undef free
-#define free hybrid_free
-
-extern void *hybrid_malloc (size_t);
-extern void *hybrid_calloc (size_t, size_t);
-extern void hybrid_free (void *);
-extern void *hybrid_aligned_alloc (size_t, size_t);
-extern void *hybrid_realloc (void *, size_t);
-#endif	/* emacs */
-#endif	/* HYBRID_MALLOC */
-
 /* We have to go this route, rather than the old hpux9 approach of
    renaming the functions via macros.  The system's stdlib.h has fully
    prototyped declarations, which yields a conflicting definition of
diff --git a/src/gmalloc.c b/src/gmalloc.c
index 1faf6506167..d6c5d351925 100644
--- a/src/gmalloc.c
+++ b/src/gmalloc.c
@@ -62,7 +62,7 @@ #define USE_PTHREAD
    grealloc... via the macros that follow).  The dumped emacs,
    however, will use the system malloc, realloc....  In other source
    files, malloc, realloc... are renamed hybrid_malloc,
-   hybrid_realloc... via macros in conf_post.h.  hybrid_malloc and
+   hybrid_realloc... via macros in lisp.h.  hybrid_malloc and
    friends are wrapper functions defined later in this file.  */
 #undef malloc
 #undef realloc
diff --git a/src/lisp.h b/src/lisp.h
index bf6b023fc2a..f3566ad2973 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4930,10 +4930,45 @@ #define safe_calln(...) \
 
 /* Defined in unexmacosx.c.  */
 #if defined DARWIN_OS && defined HAVE_UNEXEC
+/* Redirect calls to malloc, realloc and free to a macOS zone memory allocator.
+   FIXME: Either also redirect unexec_aligned_alloc and unexec_calloc,
+   or fix this comment to explain why those two redirections are not needed.  */
 extern void unexec_init_emacs_zone (void);
 extern void *unexec_malloc (size_t);
 extern void *unexec_realloc (void *, size_t);
 extern void unexec_free (void *);
+# ifndef UNEXMACOSX_C
+#  include <stdlib.h>
+#  undef malloc
+#  undef realloc
+#  undef free
+#  define malloc unexec_malloc
+#  define realloc unexec_realloc
+#  define free unexec_free
+# endif
+#endif
+
+/* Defined in gmalloc.c.  */
+#ifdef HYBRID_MALLOC
+/* Redirect calls to malloc and friends to a hybrid allocator that
+   uses gmalloc before dumping and the system malloc after dumping.
+   This can be useful on Cygwin, for example.  */
+extern void *hybrid_aligned_alloc (size_t, size_t);
+extern void *hybrid_calloc (size_t, size_t);
+extern void *hybrid_malloc (size_t);
+extern void *hybrid_realloc (void *, size_t);
+extern void hybrid_free (void *);
+# include <stdlib.h>
+# undef aligned_alloc
+# undef calloc
+# undef malloc
+# undef realloc
+# undef free
+# define aligned_alloc hybrid_aligned_alloc
+# define calloc hybrid_calloc
+# define malloc hybrid_malloc
+# define realloc hybrid_realloc
+# define free hybrid_free
 #endif
 
 /* The definition of Lisp_Module_Function depends on emacs-module.h,
diff --git a/src/unexmacosx.c b/src/unexmacosx.c
index 7b2326441b4..d6c7686607f 100644
--- a/src/unexmacosx.c
+++ b/src/unexmacosx.c
@@ -87,15 +87,10 @@
 
 #include <config.h>
 
-/* Although <config.h> redefines malloc to unexec_malloc, etc., this
-   file wants stdlib.h to declare the originals.  */
-#undef malloc
-#undef realloc
-#undef free
-
 #include <stdlib.h>
 
 #include "unexec.h"
+#define UNEXMACOSX_C /* Tell lisp.h we want the system malloc, etc.  */
 #include "lisp.h"
 #include "sysstdio.h"
 
-- 
2.45.2