Re: [PATCH] malloc: Remove __malloc_initialize_hook from the API [BZ #19564]

[Florian Weimer <fweimer at redhat dot com>]

Attached is a slightly more polished version of this patch.  I rebased 
it on current master, so it no longer depends on the patch which removes 
__malloc_check_init.

I still do not know a better way to get rid of the symbol.  This version 
of the patch also makes the symbol a compat symbol.  It turns out the 
preprocessor expansion does the right thing; the symbol name is 
translated before it is converted to a string.

eu-readelf output looks reasonable, no default version:

 1794: 000000000039b790      8 OBJECT  WEAK   DEFAULT       33 
__malloc_initialize_hook@GLIBC_2.2.5


Thanks,
Florian

malloc: Remove __malloc_initialize_hook from the API [BZ #19564]

__malloc_initialize_hook is interposed by application code, so
the usual approach to define a compatibility symbol does not work.
This commit adds a new mechanism based on #pragma GCC poison in
<stdc-predef.h>.

2016-06-07  Florian Weimer  <fweimer@redhat.com>

	[BZ #19564]
	Remove __malloc_initialize_hook from the API.
	* malloc/malloc.h (__malloc_initialize_hook): Remove.
	* include/stdc-predef.h (__malloc_initialize_hook): Poison with
	#pragma GCC poison.
	* malloc/malloc-hooks.h: New file.
	* malloc/arena.c (ptmalloc_init): Use old__malloc_initialize_hook.
	* malloc/malloc.c (HAVE_MALLOC_INIT_HOOK): New.
	(old__malloc_initialize_hook): Rename from
	__malloc_initialize_hook to evade poisoning.  Turn into compat
	symbol.
        * malloc/mcheck-init.c (old__malloc_initialize_hook): Rename from
	__malloc_initialize_hook.
	* malloc/Makefile (poisoned_apis, unpoisoned_api_defines): New.
	(mallo.c, mcheck-init.c): Compile with unpoisoned symbol.
	* manual/memory.texi (Hooks for Malloc): Remove
	__malloc_initialize_hook.  Adjust hook example.

diff --git a/include/stdc-predef.h b/include/stdc-predef.h
index f9f7f73..52cf8d1 100644
--- a/include/stdc-predef.h
+++ b/include/stdc-predef.h
@@ -57,4 +57,11 @@
 /* We do not support C11 <threads.h>.  */
 #define __STDC_NO_THREADS__		1
 
+/* Remove symbols from the API which can be interposed.  */
+#if defined (__GNUC__)
+# if __GNUC__ >= 4
+#  pragma GCC poison __malloc_initialize_hook
+# endif	 /* __GNUC__ >= 4 */
+#endif	/* __GNUC__ */
+
 #endif
diff --git a/malloc/Makefile b/malloc/Makefile
index fa1730e..91eb17d 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -115,6 +115,20 @@ endif
 
 include ../Rules
 
+# Support references to removed APIs.  We use #pragma GCC poison in
+# <stdc-predef.h> to make it difficult to reference them.  For select
+# source files, we work around this poisoning by defining a macro on
+# the command line (which is processed before <stdc-predef.h> and can
+# therefore use tokens poisoned later).
+poisoned_apis = \
+  __malloc_initialize_hook \
+
+unpoisoned_api_defines := \
+  $(foreach sym,$(poisoned_apis), \
+    $(patsubst %,-Dold%, $(sym))=$(sym))
+CPPFLAGS-malloc.c = $(unpoisoned_api_defines)
+CPPFLAGS-mcheck-init.c = $(unpoisoned_api_defines)
+
 CFLAGS-mcheck-init.c = $(PIC-ccflag)
 CFLAGS-obstack.c = $(uses-callbacks)
 
diff --git a/malloc/arena.c b/malloc/arena.c
index 1dd9dee..64a118c 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -340,9 +340,11 @@ ptmalloc_init (void)
       if (check_action != 0)
         __malloc_check_init ();
     }
-  void (*hook) (void) = atomic_forced_read (__malloc_initialize_hook);
+#if HAVE_MALLOC_INIT_HOOK
+  void (*hook) (void) = atomic_forced_read (old__malloc_initialize_hook);
   if (hook != NULL)
     (*hook)();
+#endif
   __malloc_initialized = 1;
 }
 
diff --git a/malloc/malloc-hooks.h b/malloc/malloc-hooks.h
new file mode 100644
index 0000000..3be391b
--- /dev/null
+++ b/malloc/malloc-hooks.h
@@ -0,0 +1,27 @@
+/* Internal declarations of malloc hooks no longer in the public API.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public License as
+   published by the Free Software Foundation; either version 2.1 of the
+   License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; see the file COPYING.LIB.  If
+   not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef _MALLOC_HOOKS_H
+#define _MALLOC_HOOKS_H
+
+/* These hooks are no longer part of the public API and are poisoned
+   in <stdc-predef.h>.  Their names here reflect the command-line
+   mapping which is used inside glibc to get past the poisoning.  */
+void (*old__malloc_initialize_hook) (void);
+
+#endif  /* _MALLOC_HOOKS_H */
diff --git a/malloc/malloc.c b/malloc/malloc.c
index ead9a21..29617e5 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -491,6 +491,15 @@ void *(*__morecore)(ptrdiff_t) = __default_morecore;
 #define HAVE_MREMAP 0
 #endif
 
+/* We may need to support __malloc_initialize_hook for backwards
+   compatibility.  */
+
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_24)
+# define HAVE_MALLOC_INIT_HOOK 1
+#else
+# define HAVE_MALLOC_INIT_HOOK 0
+#endif
+
 
 /*
   This version of malloc supports the standard SVID/XPG mallinfo
@@ -1839,7 +1848,12 @@ static void *realloc_hook_ini (void *ptr, size_t sz,
 static void *memalign_hook_ini (size_t alignment, size_t sz,
                                 const void *caller) __THROW;
 
-void weak_variable (*__malloc_initialize_hook) (void) = NULL;
+#if HAVE_MALLOC_INIT_HOOK
+void weak_variable (*old__malloc_initialize_hook) (void) = NULL;
+compat_symbol (libc, old__malloc_initialize_hook,
+	       old__malloc_initialize_hook, GLIBC_2_0);
+#endif
+
 void weak_variable (*__free_hook) (void *__ptr,
                                    const void *) = NULL;
 void *weak_variable (*__malloc_hook)
diff --git a/malloc/malloc.h b/malloc/malloc.h
index d95a315..54b1862 100644
--- a/malloc/malloc.h
+++ b/malloc/malloc.h
@@ -141,11 +141,6 @@ extern void *malloc_get_state (void) __THROW;
    malloc_get_state(). */
 extern int malloc_set_state (void *__ptr) __THROW;
 
-/* Called once when malloc is initialized; redefining this variable in
-   the application provides the preferred way to set up the hook
-   pointers. */
-extern void (*__MALLOC_HOOK_VOLATILE __malloc_initialize_hook) (void)
-__MALLOC_DEPRECATED;
 /* Hooks for debugging and user-defined versions. */
 extern void (*__MALLOC_HOOK_VOLATILE __free_hook) (void *__ptr,
                                                    const void *)
diff --git a/malloc/mcheck-init.c b/malloc/mcheck-init.c
index 8d63dd3..3218bb0 100644
--- a/malloc/mcheck-init.c
+++ b/malloc/mcheck-init.c
@@ -27,4 +27,4 @@ turn_on_mcheck (void)
   mcheck (NULL);
 }
 
-void (*__malloc_initialize_hook) (void) = turn_on_mcheck;
+void (*old__malloc_initialize_hook) (void) = turn_on_mcheck;
diff --git a/manual/memory.texi b/manual/memory.texi
index a3ecc0d..92f041a 100644
--- a/manual/memory.texi
+++ b/manual/memory.texi
@@ -1370,19 +1370,6 @@ should make sure to restore all the hooks to their previous value.  When
 coming back from the recursive call, all the hooks should be resaved
 since a hook might modify itself.
 
-@comment malloc.h
-@comment GNU
-@defvar __malloc_initialize_hook
-The value of this variable is a pointer to a function that is called
-once when the malloc implementation is initialized.  This is a weak
-variable, so it can be overridden in the application with a definition
-like the following:
-
-@smallexample
-void (*@var{__malloc_initialize_hook}) (void) = my_init_hook;
-@end smallexample
-@end defvar
-
 An issue to look out for is the time at which the malloc hook functions
 can be safely installed.  If the hook functions call the malloc-related
 functions recursively, it is necessary that malloc has already properly
@@ -1393,11 +1380,6 @@ are assigned to @emph{before} the very first @code{malloc} call has
 completed, because otherwise a chunk obtained from the ordinary,
 un-hooked malloc may later be handed to @code{__free_hook}, for example.
 
-In both cases, the problem can be solved by setting up the hooks from
-within a user-defined function pointed to by
-@code{__malloc_initialize_hook}---then the hooks will be set up safely
-at the right time.
-
 Here is an example showing how to use @code{__malloc_hook} and
 @code{__free_hook} properly.  It installs a function that prints out
 information every time @code{malloc} or @code{free} is called.  We just
@@ -1413,11 +1395,8 @@ static void my_init_hook (void);
 static void *my_malloc_hook (size_t, const void *);
 static void my_free_hook (void*, const void *);
 
-/* Override initializing hook from the C library. */
-void (*__malloc_initialize_hook) (void) = my_init_hook;
-
 static void
-my_init_hook (void)
+my_init (void)
 @{
   old_malloc_hook = __malloc_hook;
   old_free_hook = __free_hook;
@@ -1465,6 +1444,7 @@ my_free_hook (void *ptr, const void *caller)
 
 main ()
 @{
+  my_init ();
   @dots{}
 @}
 @end smallexample