diff mbox

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

Message ID 5c9da5b0-d548-0fa7-9264-c0fc5bd65039@redhat.com
State New
Headers show

Commit Message

Florian Weimer May 24, 2016, noon UTC
The attached patch continues with the removal of deprecated malloc 
functionality.  It depends on the patches posted before 
(“tst-rec-dlopen: Use custom malloc instead of hooks”, “malloc: Remove 
__malloc_check_init from the API”).

I tackled a single hook variable first, __malloc_initialize_hook.  It is 
the one that Emacs needs to use heap dumping with the system malloc.

I expect to remove the other hooks as well, so I did not update the 
manual with advice how to use the hooks without __malloc_initialize_hook.

Thanks,
Florian

Comments

DJ Delorie May 24, 2016, 5:26 p.m. UTC | #1
Is there any way we could mark this in the ABI itself so that attempts
to link against obsolete hooks only work if you've selected an older
ABI, but fail with a suitable warning if you link against a new ABI?  It
looks like a recompile would be needed to provide backwards
compatibility.  (assuming we want to provide this feature at all, but is
a link-warning-free-but-doesnt-do-anything solution better than "old
apps don't build" ?)

The new malloc-hooks.h doesn't match the poison list in stdc-predef.h.
I assume that's "not yet" but the patch should be internally consistent.
Florian Weimer May 24, 2016, 6:39 p.m. UTC | #2
On 05/24/2016 07:26 PM, DJ Delorie wrote:
>
> Is there any way we could mark this in the ABI itself so that attempts
> to link against obsolete hooks only work if you've selected an older
> ABI, but fail with a suitable warning if you link against a new ABI?

I don't see how.  The symbol is interposed from application code.  This 
means that the symbol won't have any linker magic attached to it.  We 
can't trigger on the existence of a symbol, either, I think, so tricks 
with libc_nonshared.a aren't an option, either.

I'd love to do something less drastic here, but I just don't see any 
other approach to remove the symbol from the API (but not the ABI).

> The new malloc-hooks.h doesn't match the poison list in stdc-predef.h.
> I assume that's "not yet" but the patch should be internally consistent.

Right, I'll make it consistent.

Thanks,
Florian
diff mbox

Patch

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

2016-05-10  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 (old__malloc_initialize_hook): Rename from
	__malloc_initialize_hook.
	* manual/memory.texi (Hooks for Malloc): Remove
	__malloc_initialize_hook.  Adjust hook example.
        * malloc/mcheck-init.c (old__malloc_initialize_hook): Rename from
	__malloc_initialize_hook.

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 32bff63..a7d25c3 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -336,9 +336,11 @@  ptmalloc_init (void)
     }
   if (s && s[0])
     __libc_mallopt (M_CHECK_ACTION, (int) (s[0] - '0'));
-  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..dd03fae
--- /dev/null
+++ b/malloc/malloc-hooks.h
@@ -0,0 +1,31 @@ 
+/* 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);
+void (*old__free_hook) (void *, const void *);
+void *(*old__malloc_hook)(size_t, const void *);
+void *(*old__realloc_hook)(void *, size_t, const void *);
+void *(*old__memalign_hook)(size_t, size_t, const void *);
+
+#endif  /* _MALLOC_HOOKS_H */
diff --git a/malloc/malloc.c b/malloc/malloc.c
index f9d8c55..4d63805 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
@@ -1830,7 +1839,10 @@  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;
+#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 2d513c0..8f9dc72 100644
--- a/malloc/malloc.h
+++ b/malloc/malloc.h
@@ -31,7 +31,6 @@ 
 # define __MALLOC_DEPRECATED __attribute_deprecated__
 #endif
 
-
 __BEGIN_DECLS
 
 /* Allocate SIZE bytes of memory.  */
@@ -141,11 +140,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