bug#27416: [PROPOSED] Simplify malloc replacement on glibc

bug#27416: [PROPOSED] Simplify malloc replacement on glibc

[Paul Eggert]

This avoids the need to build lib/e-gettime.o etc. on glibc
platforms, as plain lib/gettime.o will do.
* configure.ac (HAVE___LIBC_MALLOC): New symbol.
* lib/Makefile.in (all): Don’t build libegnu.a if HAVE___LIBC_MALLOC.
* lib/gnulib.mk.in: Regenerate.
* src/Makefile.in (HAVE___LIBC_MALLOC): New macro.
(LIBEGNU_ARCHIVE): Use it, so that libegnu.a is not needed
if HAVE___LIBC_MALLOC.
* src/conf_post.h (malloc, realloc, aligned_alloc, calloc, free):
Do not define as macros if HAVE___LIBC_MALLOC, since they are
now defined as functions in that case.
* src/gmalloc.c (hybrid_malloc, hybrid_realloc, hybrid_calloc)
(hybrid_aligned_alloc, hybrid_free): Define to plain malloc etc.,
if HAVE___LIBC_MALLOC, since the hybrid functions replace the
standard ones in that case.
(malloc, realloc, calloc, free): Define to __libc_malloc etc.,
if HAVE___LIBC_MALLOC, as that is the way to call the standard ones
in that case.
(aligned_alloc): No need to declare if HAVE___LIBC_MALLOC,
since we don’t replace it.
(hybrid_aligned_alloc): Do not call aligned_alloc if
HAVE___LIBC_MALLOC, since there is no __libc_aligned_alloc.
Use posix_memalign instead.
---
 configure.ac     |  3 +++
 lib/Makefile.in  |  2 +-
 lib/gnulib.mk.in |  1 +
 src/Makefile.in  |  4 +++-
 src/conf_post.h  |  9 ++++-----
 src/gmalloc.c    | 16 ++++++++++++++--
 6 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index ef61107..7dafd49 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2214,6 +2214,7 @@ AC_DEFUN
 
 GMALLOC_OBJ=
 HYBRID_MALLOC=
+HAVE___LIBC_MALLOC=
 if test "${system_malloc}" = "yes"; then
   AC_DEFINE([SYSTEM_MALLOC], 1,
     [Define to 1 to use the system memory allocator, even if it is not
@@ -2230,6 +2231,7 @@ AC_DEFUN
   GNU_MALLOC_reason=" (only before dumping)"
   GMALLOC_OBJ=gmalloc.o
   VMLIMIT_OBJ=
+  AC_CHECK_FUNCS([__libc_malloc], [HAVE___LIBC_MALLOC=1])
 else
   test "$doug_lea_malloc" != "yes" && GMALLOC_OBJ=gmalloc.o
   VMLIMIT_OBJ=vm-limit.o
@@ -2249,6 +2251,7 @@ AC_DEFUN
   fi
 fi
 AC_SUBST([HYBRID_MALLOC])
+AC_SUBST([HAVE___LIBC_MALLOC])
 AC_SUBST(GMALLOC_OBJ)
 AC_SUBST(VMLIMIT_OBJ)
 
diff --git a/lib/Makefile.in b/lib/Makefile.in
index ee41ea3..cd18da7 100644
--- a/lib/Makefile.in
+++ b/lib/Makefile.in
@@ -90,7 +90,7 @@ .c.o:
 e-%.o: %.c
 	$(AM_V_CC)$(CC) -c $(CPPFLAGS) $(ALL_CFLAGS) -Demacs -o $@ $<
 
-all: libgnu.a $(if $(HYBRID_MALLOC),libegnu.a)
+all: libgnu.a $(if $(HYBRID_MALLOC),$(if $(HAVE___LIBC_MALLOC),,libegnu.a))
 
 libgnu.a: $(libgnu_a_OBJECTS)
 	$(AM_V_at)rm -f $@
diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
index 509089e..4afe4b3 100644
--- a/lib/gnulib.mk.in
+++ b/lib/gnulib.mk.in
@@ -522,6 +522,7 @@ HAVE_WCHAR_T = @HAVE_WCHAR_T@
 HAVE_WINSOCK2_H = @HAVE_WINSOCK2_H@
 HAVE_XSERVER = @HAVE_XSERVER@
 HAVE__EXIT = @HAVE__EXIT@
+HAVE___LIBC_MALLOC = @HAVE___LIBC_MALLOC@
 HYBRID_MALLOC = @HYBRID_MALLOC@
 IMAGEMAGICK_CFLAGS = @IMAGEMAGICK_CFLAGS@
 IMAGEMAGICK_LIBS = @IMAGEMAGICK_LIBS@
diff --git a/src/Makefile.in b/src/Makefile.in
index 2be24ac..f1b1aeb 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -257,6 +257,7 @@ XDBE_CFLAGS =
 WIDGET_OBJ=@WIDGET_OBJ@
 
 HYBRID_MALLOC = @HYBRID_MALLOC@
+HAVE___LIBC_MALLOC = @HAVE___LIBC_MALLOC@
 
 ## cygw32.o if CYGWIN, otherwise empty.
 CYGWIN_OBJ=@CYGWIN_OBJ@
@@ -584,7 +585,8 @@ globals.h:
 
 $(ALLOBJS): globals.h
 
-LIBEGNU_ARCHIVE = $(lib)/lib$(if $(HYBRID_MALLOC),e)gnu.a
+LIBEGNU_ARCHIVE = \
+  $(lib)/lib$(if $(HYBRID_MALLOC),$(if $(HAVE___LIBC_MALLOC),,e))gnu.a
 
 $(LIBEGNU_ARCHIVE): $(config_h)
 	$(MAKE) -C $(lib) all
diff --git a/src/conf_post.h b/src/conf_post.h
index e1d6a93..af10397 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -105,17 +105,16 @@ typedef bool bool_bf;
 
 /* 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
+   This is done via wrappers defined in gmalloc.c, whose names are
+   'malloc' etc. if HAVE___LIBC_MALLOC is defined, and 'hybrid_malloc'
+   etc. otherwise.  */
+#if defined HYBRID_MALLOC && !defined HAVE___LIBC_MALLOC && defined emacs
 #define malloc hybrid_malloc
 #define realloc hybrid_realloc
 #define aligned_alloc hybrid_aligned_alloc
 #define calloc hybrid_calloc
 #define free hybrid_free
 #endif
-#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
diff --git a/src/gmalloc.c b/src/gmalloc.c
index 49f1faf..9de0079 100644
--- a/src/gmalloc.c
+++ b/src/gmalloc.c
@@ -1709,13 +1709,25 @@ valloc (size_t size)
 #undef aligned_alloc
 #undef free
 
+#ifdef HAVE___LIBC_MALLOC
+# define hybrid_malloc malloc
+# define hybrid_realloc realloc
+# define hybrid_calloc calloc
+# define hybrid_aligned_alloc aligned_alloc
+# define hybrid_free free
+# define malloc __libc_malloc
+# define realloc __libc_realloc
+# define calloc __libc_calloc
+# define free __libc_free
+#endif
+
 #ifdef HYBRID_MALLOC
 /* Declare system malloc and friends.  */
 extern void *malloc (size_t size);
 extern void *realloc (void *ptr, size_t size);
 extern void *calloc (size_t nmemb, size_t size);
 extern void free (void *ptr);
-#ifdef HAVE_ALIGNED_ALLOC
+#if defined HAVE_ALIGNED_ALLOC && !defined HAVE___LIBC_MALLOC
 extern void *aligned_alloc (size_t alignment, size_t size);
 #elif defined HAVE_POSIX_MEMALIGN
 extern int posix_memalign (void **memptr, size_t alignment, size_t size);
@@ -1759,7 +1771,7 @@ hybrid_aligned_alloc (size_t alignment, size_t size)
   if (!DUMPED)
     return galigned_alloc (alignment, size);
   /* The following is copied from alloc.c */
-#ifdef HAVE_ALIGNED_ALLOC
+#if defined HAVE_ALIGNED_ALLOC && !defined HAVE___LIBC_MALLOC
   return aligned_alloc (alignment, size);
 #else  /* HAVE_POSIX_MEMALIGN */
   void *p;
-- 
2.9.4

bug#27416: [PROPOSED] Simplify malloc replacement on glibc

[npostavs]

tags 27416 + patch
severity 27416 wishlist
quit

Paul Eggert <eggert@cs.ucla.edu> writes:

> This avoids the need to build lib/e-gettime.o etc. on glibc
> platforms, as plain lib/gettime.o will do.

Could you add something like the summary you posted in
http://lists.gnu.org/archive/html/emacs-devel/2017-06/msg00351.html to
the commit message, I found the explanation here a bit confusing until I
read that.

    [Replace] Emacs's hybrid malloc implementation on GNUish platforms
    with a hybrid implementation that redefines malloc instead of
    re-#defining it.  That is, instead of defining a function
    hybrid_malloc that can call either gmalloc or the system malloc (and
    using the macro "#define malloc hybrid_malloc" in Emacs code), Emacs
    defines a function malloc that can call either gmalloc or the system
    malloc (via the latter’s alternate name ‘__libc_malloc’), without
    using a macro.

    [T]he Emacs build process no longer needs to compile library files
    twice.  For example, it can simply build lib/gettime.o from
    lib/gettime.c, rather than having to build both lib/gettime.o and
    lib/e-gettime.o.  This is because there is only one malloc symbol
    used by Emacs code, not two.

    The hybrid malloc approach would not change on non-GNUish platforms.

bug#27416: [PROPOSED] Simplify malloc replacement on glibc

[Paul Eggert]

After looking into this some more, I decided not to pursue this. The 
patch has a typo, and when I fixed the typo and debugged Emacs I found 
that the patch sometimes made Emacs crashed. Apparently the problem is 
that the dynamic linker uses gmalloc once the patch is made, and gmalloc 
is not compatible with what the dynamic linker expects. Oh well. Closing.