[PATCH 2/8] malloc: Move malloc hook references to hooks.c

DJ Delorie dj@redhat.com
Thu Jun 24 21:10:38 GMT 2021


Siddhesh Poyarekar <siddhesh@sourceware.org> writes:
> Make the core malloc code hooks free and instead only have entry
> points for _*_debug_before inline functions.

> diff --git a/malloc/arena.c b/malloc/arena.c
> index 7eb110445e..1861d20006 100644
> --- a/malloc/arena.c
> +++ b/malloc/arena.c
> @@ -44,8 +44,6 @@
>  
>  /***************************************************************************/
>  
> -#define top(ar_ptr) ((ar_ptr)->top)
> -

Moved to malloc-internal.h; ok

> diff --git a/malloc/hooks.c b/malloc/hooks.c
> index 57a9b55788..4960aafd08 100644
> --- a/malloc/hooks.c
> +++ b/malloc/hooks.c
> @@ -21,35 +21,107 @@
>     corrupt pointer is detected: do nothing (0), print an error message
>     (1), or call abort() (2). */
>  
> -/* Hooks for debugging versions.  The initial hooks just call the
> -   initialization routine, then do the normal work. */
> +/* Define and initialize the hook variables.  These weak definitions must
> +   appear before any use of the variables in a function (arena.c uses one).  */
> +#ifndef weak_variable
> +/* In GNU libc we want the hook variables to be weak definitions to
> +   avoid a problem with Emacs.  */
> +# define weak_variable weak_function
> +#endif
> +
> +/* Forward declarations.  */
> +
> +#if HAVE_MALLOC_INIT_HOOK
> +void (*__malloc_initialize_hook) (void) __attribute__ ((nocommon));
> +compat_symbol (libc, __malloc_initialize_hook,
> +	       __malloc_initialize_hook, GLIBC_2_0);
> +#endif
> +
> +void weak_variable (*__free_hook) (void *__ptr,
> +                                   const void *) = NULL;
> +void *weak_variable (*__malloc_hook)
> +  (size_t __size, const void *) = NULL;
> +void *weak_variable (*__realloc_hook)
> +  (void *__ptr, size_t __size, const void *) = NULL;
> +void *weak_variable (*__memalign_hook)
> +  (size_t __alignment, size_t __size, const void *) = NULL;
> +
> +static void ptmalloc_init (void);

ok.

> -static void *
> -malloc_hook_ini (size_t sz, const void *caller)
> +#include "malloc-check.c"

Moved from later, ok.

> +static __always_inline bool
> +_malloc_debug_before (size_t bytes, void **victimp, const void *address)
>  {
> -  __malloc_hook = NULL;
> -  ptmalloc_init ();
> -  return __libc_malloc (sz);
> +  _Static_assert (PTRDIFF_MAX <= SIZE_MAX / 2,
> +                  "PTRDIFF_MAX is not more than half of SIZE_MAX");
> +
> +  void *(*hook) (size_t, const void *)
> +    = atomic_forced_read (__malloc_hook);
> +  if (__builtin_expect (hook != NULL, 0))
> +    {
> +      *victimp = (*hook)(bytes, address);
> +      return true;
> +    }
> +  return false;
>  }

Ok.

> -static void *
> -realloc_hook_ini (void *ptr, size_t sz, const void *caller)
> +static __always_inline bool
> +_free_debug_before (void *mem, const void *address)
>  {
> -  __malloc_hook = NULL;
> -  __realloc_hook = NULL;
> -  ptmalloc_init ();
> -  return __libc_realloc (ptr, sz);
> +  void (*hook) (void *, const void *)
> +    = atomic_forced_read (__free_hook);
> +  if (__builtin_expect (hook != NULL, 0))
> +    {
> +      (*hook)(mem, address);
> +      return true;
> +    }
> +  return false;
>  }

Ok.

> -static void *
> -memalign_hook_ini (size_t alignment, size_t sz, const void *caller)
> +static __always_inline bool
> +_realloc_debug_before (void *oldmem, size_t bytes, void **victimp,
> +			const void *address)
>  {
> -  __memalign_hook = NULL;
> -  ptmalloc_init ();
> -  return __libc_memalign (alignment, sz);
> +  void *(*hook) (void *, size_t, const void *) =
> +    atomic_forced_read (__realloc_hook);
> +  if (__builtin_expect (hook != NULL, 0))
> +    {
> +      *victimp = (*hook)(oldmem, bytes, address);
> +      return true;
> +    }
> +
> +  return false;
>  }

Ok.

> -#include "malloc-check.c"

Ok.

> +static __always_inline bool
> +_memalign_debug_before (size_t alignment, size_t bytes, void **victimp,
> +			 const void *address)
> +{
> +  void *(*hook) (size_t, size_t, const void *) =
> +    atomic_forced_read (__memalign_hook);
> +  if (__builtin_expect (hook != NULL, 0))
> +    {
> +      *victimp = (*hook)(alignment, bytes, address);
> +      return true;
> +    }
> +  return false;
> +}

Ok.

> +static __always_inline bool
> +_calloc_debug_before (size_t bytes, void **victimp, const void *address)
> +{
> +  void *(*hook) (size_t, const void *) =
> +    atomic_forced_read (__malloc_hook);
> +  if (__builtin_expect (hook != NULL, 0))
> +    {
> +      *victimp = (*hook)(bytes, address);
> +      if (*victimp != NULL)
> +	memset (*victimp, 0, bytes);
> +      return true;
> +    }
> +  return false;
> +}

Ok.

> diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h
> index 258f29584e..ee0f5697af 100644
> --- a/malloc/malloc-internal.h
> +++ b/malloc/malloc-internal.h
> @@ -61,6 +61,7 @@
>  /* The corresponding bit mask value.  */
>  #define MALLOC_ALIGN_MASK (MALLOC_ALIGNMENT - 1)
>  
> +#define top(ar_ptr) ((ar_ptr)->top)

Ok.

> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 0e2e1747e0..75ca6ec3f0 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -2013,30 +2013,6 @@ static void     malloc_consolidate (mstate);
>  # define weak_variable weak_function
>  #endif
>  
> -/* Forward declarations.  */
> -static void *malloc_hook_ini (size_t sz,
> -                              const void *caller) __THROW;
> -static void *realloc_hook_ini (void *ptr, size_t sz,
> -                               const void *caller) __THROW;
> -static void *memalign_hook_ini (size_t alignment, size_t sz,
> -                                const void *caller) __THROW;
> -
> -#if HAVE_MALLOC_INIT_HOOK
> -void (*__malloc_initialize_hook) (void) __attribute__ ((nocommon));
> -compat_symbol (libc, __malloc_initialize_hook,
> -	       __malloc_initialize_hook, GLIBC_2_0);
> -#endif
> -
> -void weak_variable (*__free_hook) (void *__ptr,
> -                                   const void *) = NULL;
> -void *weak_variable (*__malloc_hook)
> -  (size_t __size, const void *) = malloc_hook_ini;
> -void *weak_variable (*__realloc_hook)
> -  (void *__ptr, size_t __size, const void *)
> -  = realloc_hook_ini;
> -void *weak_variable (*__memalign_hook)
> -  (size_t __alignment, size_t __size, const void *)
> -  = memalign_hook_ini;
>  void weak_variable (*__after_morecore_hook) (void) = NULL;

Moved, ok.

> @@ -2065,6 +2041,9 @@ free_perturb (char *p, size_t n)
>  
>  #include <stap-probe.h>
>  
> +/* ----------------- Support for debugging hooks -------------------- */
> +#include "hooks.c"
> +
>  /* ------------------- Support for multiple arenas -------------------- */
>  #include "arena.c"
>  
> @@ -2425,10 +2404,6 @@ do_check_malloc_state (mstate av)
>  #endif
>  
>  
> -/* ----------------- Support for debugging hooks -------------------- */
> -#include "hooks.c"
> -
> -
>  /* ----------- Routines dealing with system allocation -------------- */

Ok.

> @@ -3225,13 +3200,12 @@ __libc_malloc (size_t bytes)
>    mstate ar_ptr;
>    void *victim;
>  
> -  _Static_assert (PTRDIFF_MAX <= SIZE_MAX / 2,
> -                  "PTRDIFF_MAX is not more than half of SIZE_MAX");
> +  if (__malloc_initialized < 0)
> +    ptmalloc_init ();
> +
> +  if (_malloc_debug_before (bytes, &victim, RETURN_ADDRESS (0)))
> +    return victim;
>  
> -  void *(*hook) (size_t, const void *)
> -    = atomic_forced_read (__malloc_hook);
> -  if (__builtin_expect (hook != NULL, 0))
> -    return (*hook)(bytes, RETURN_ADDRESS (0));

Ok.

> @@ -3292,13 +3266,11 @@ __libc_free (void *mem)
>    mstate ar_ptr;
>    mchunkptr p;                          /* chunk corresponding to mem */
>  
> -  void (*hook) (void *, const void *)
> -    = atomic_forced_read (__free_hook);
> -  if (__builtin_expect (hook != NULL, 0))
> -    {
> -      (*hook)(mem, RETURN_ADDRESS (0));
> -      return;
> -    }
> +  if (__malloc_initialized < 0)
> +    ptmalloc_init ();
> +
> +  if (_free_debug_before (mem, RETURN_ADDRESS (0)))
> +    return;

Ok.


> @@ -3351,10 +3323,11 @@ __libc_realloc (void *oldmem, size_t bytes)
>  
>    void *newp;             /* chunk to return */
>  
> -  void *(*hook) (void *, size_t, const void *) =
> -    atomic_forced_read (__realloc_hook);
> -  if (__builtin_expect (hook != NULL, 0))
> -    return (*hook)(oldmem, bytes, RETURN_ADDRESS (0));
> +  if (__malloc_initialized < 0)
> +    ptmalloc_init ();
> +
> +  if (_realloc_debug_before (oldmem, bytes, &newp, RETURN_ADDRESS (0)))
> +    return newp;

Ok.

> @@ -3490,6 +3463,9 @@ void *
>  __libc_memalign (size_t alignment, size_t bytes)
>  {
>    void *address = RETURN_ADDRESS (0);
> +  if (__malloc_initialized < 0)
> +    ptmalloc_init ();
> +
>    return _mid_memalign (alignment, bytes, address);
>  }

Ok.

> @@ -3499,10 +3475,8 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
>    mstate ar_ptr;
>    void *p;
>  
> -  void *(*hook) (size_t, size_t, const void *) =
> -    atomic_forced_read (__memalign_hook);
> -  if (__builtin_expect (hook != NULL, 0))
> -    return (*hook)(alignment, bytes, address);
> +  if (_memalign_debug_before (alignment, bytes, &p, address))
> +    return p;

Ok.

> @@ -3612,16 +3586,11 @@ __libc_calloc (size_t n, size_t elem_size)
>  
>    sz = bytes;
>  
> -  void *(*hook) (size_t, const void *) =
> -    atomic_forced_read (__malloc_hook);
> -  if (__builtin_expect (hook != NULL, 0))
> -    {
> -      mem = (*hook)(sz, RETURN_ADDRESS (0));
> -      if (mem == 0)
> -        return 0;
> +  if (__malloc_initialized < 0)
> +    ptmalloc_init ();
>  
> -      return memset (mem, 0, sz);
> -    }
> +  if (_calloc_debug_before (sz, &mem, RETURN_ADDRESS (0)))
> +    return mem;

Ok.

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>



More information about the Libc-alpha mailing list