[PATCH 3/8] glibc.malloc.check: Wean away from malloc hooks
DJ Delorie
dj@redhat.com
Thu Jun 24 21:43:33 GMT 2021
Siddhesh Poyarekar <siddhesh@sourceware.org> writes:
> Set up a new internal debugging hooks flag variable and migrate
> glibc.malloc.check to it so that it no longer uses the malloc hooks.
> diff --git a/malloc/hooks.c b/malloc/hooks.c
> index 4960aafd08..77855801c8 100644
> --- a/malloc/hooks.c
> +++ b/malloc/hooks.c
> @@ -21,6 +21,8 @@
> corrupt pointer is detected: do nothing (0), print an error message
> (1), or call abort() (2). */
>
> +# include <stdbool.h>
> +
Ok.
> @@ -29,6 +31,14 @@
> # define weak_variable weak_function
> #endif
>
> +/* The internal malloc debugging hooks. */
> +enum malloc_debug_hooks
> +{
> + MALLOC_NONE_HOOK = 0,
> + MALLOC_CHECK_HOOK = 1 << 0, /* MALLOC_CHECK_ or glibc.malloc.check. */
> +};
> +static unsigned __malloc_debugging_hooks;
> +
Ok.
> @@ -48,6 +58,24 @@ void *weak_variable (*__memalign_hook)
>
> static void ptmalloc_init (void);
>
> +static __always_inline bool
> +__is_malloc_debug_enabled (enum malloc_debug_hooks flag)
> +{
> + return __malloc_debugging_hooks & flag;
> +}
> +
> +static __always_inline void
> +__malloc_debug_enable (enum malloc_debug_hooks flag)
> +{
> + __malloc_debugging_hooks |= flag;
> +}
> +
> +static __always_inline void
> +__malloc_debug_disable (enum malloc_debug_hooks flag)
> +{
> + __malloc_debugging_hooks &= ~flag;
> +}
> +
Ok.
> @@ -63,6 +91,12 @@ _malloc_debug_before (size_t bytes, void **victimp, const void *address)
> *victimp = (*hook)(bytes, address);
> return true;
> }
> +
> + if (__glibc_unlikely (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)))
> + {
> + *victimp = malloc_check (bytes);
> + return true;
> + }
> return false;
> }
Ok. Too bad there isn't a way to tag the *function* as likely/unlikely,
rather than the *calls*.
> @@ -76,6 +110,12 @@ _free_debug_before (void *mem, const void *address)
> (*hook)(mem, address);
> return true;
> }
> +
> + if (__glibc_unlikely (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)))
> + {
> + free_check (mem);
> + return true;
> + }
> return false;
> }
Ok.
> @@ -91,6 +131,12 @@ _realloc_debug_before (void *oldmem, size_t bytes, void **victimp,
> return true;
> }
>
> + if (__glibc_unlikely (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)))
> + {
> + *victimp = realloc_check (oldmem, bytes);
> + return true;
> + }
> +
> return false;
> }
Ok.
> @@ -105,6 +151,13 @@ _memalign_debug_before (size_t alignment, size_t bytes, void **victimp,
> *victimp = (*hook)(alignment, bytes, address);
> return true;
> }
> +
> + if (__glibc_unlikely (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)))
> + {
> + *victimp = memalign_check (alignment, bytes);
> + return true;
> + }
> +
> return false;
> }
Ok.
> @@ -120,6 +173,14 @@ _calloc_debug_before (size_t bytes, void **victimp, const void *address)
> memset (*victimp, 0, bytes);
> return true;
> }
> +
> + if (__glibc_unlikely (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)))
> + {
> + *victimp = malloc_check (bytes);
> + if (*victimp != NULL)
> + memset (*victimp, 0, bytes);
> + return true;
> + }
> return false;
> }
Ok.
> @@ -195,7 +256,7 @@ malloc_set_state (void *msptr)
> __realloc_hook = NULL;
> __free_hook = NULL;
> __memalign_hook = NULL;
> - using_malloc_checking = 0;
> + __malloc_debug_disable (MALLOC_CHECK_HOOK);
Ok.
> diff --git a/malloc/malloc-check.c b/malloc/malloc-check.c
> index dcab880510..a85e519498 100644
> --- a/malloc/malloc-check.c
> +++ b/malloc/malloc-check.c
> @@ -18,20 +18,6 @@
> not, see <https://www.gnu.org/licenses/>. */
>
>
> -/* Whether we are using malloc checking. */
> -static int using_malloc_checking;
> -
> -/* Activate a standard set of debugging hooks. */
> -void
> -__malloc_check_init (void)
> -{
> - using_malloc_checking = 1;
> - __malloc_hook = malloc_check;
> - __free_hook = free_check;
> - __realloc_hook = realloc_check;
> - __memalign_hook = memalign_check;
> -}
> -
Ok.
> @@ -69,7 +55,7 @@ malloc_check_get_size (mchunkptr p)
> unsigned char c;
> unsigned char magic = magicbyte (p);
>
> - assert (using_malloc_checking == 1);
> + assert (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK));
Ok.
> @@ -203,7 +189,7 @@ top_check (void)
> }
>
> static void *
> -malloc_check (size_t sz, const void *caller)
> +malloc_check (size_t sz)
Ok.
> @@ -222,7 +208,7 @@ malloc_check (size_t sz, const void *caller)
> }
>
> static void
> -free_check (void *mem, const void *caller)
> +free_check (void *mem)
Ok.
> @@ -256,7 +242,7 @@ free_check (void *mem, const void *caller)
> }
>
> static void *
> -realloc_check (void *oldmem, size_t bytes, const void *caller)
> +realloc_check (void *oldmem, size_t bytes)
Ok.
> @@ -269,11 +255,11 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
> return NULL;
> }
> if (oldmem == 0)
> - return malloc_check (bytes, NULL);
> + return malloc_check (bytes);
Ok.
> if (bytes == 0)
> {
> - free_check (oldmem, NULL);
> + free_check (oldmem);
Ok.
> @@ -348,12 +334,12 @@ invert:
> }
>
> static void *
> -memalign_check (size_t alignment, size_t bytes, const void *caller)
> +memalign_check (size_t alignment, size_t bytes)
> {
> void *mem;
>
> if (alignment <= MALLOC_ALIGNMENT)
> - return malloc_check (bytes, NULL);
> + return malloc_check (bytes);
Ok
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 75ca6ec3f0..60753446a1 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -1124,13 +1124,6 @@ static void munmap_chunk(mchunkptr p);
> static mchunkptr mremap_chunk(mchunkptr p, size_t new_size);
> #endif
>
> -static void* malloc_check(size_t sz, const void *caller);
> -static void free_check(void* mem, const void *caller);
> -static void* realloc_check(void* oldmem, size_t bytes,
> - const void *caller);
> -static void* memalign_check(size_t alignment, size_t bytes,
> - const void *caller);
Ok.
> @@ -5078,7 +5071,7 @@ musable (void *mem)
>
> p = mem2chunk (mem);
>
> - if (__builtin_expect (using_malloc_checking == 1, 0))
> + if (__glibc_unlikely (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)))
> return malloc_check_get_size (p);
Ok.
> if (chunk_is_mmapped (p))
> diff --git a/malloc/arena.c b/malloc/arena.c
> index 1861d20006..357a3b0b30 100644
> --- a/malloc/arena.c
> +++ b/malloc/arena.c
> @@ -210,7 +210,7 @@ TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
> {
> int32_t value = (int32_t) valp->numval;
> if (value != 0)
> - __malloc_check_init ();
> + __malloc_debug_enable (MALLOC_CHECK_HOOK);
> }
Ok.
> # define TUNABLE_CALLBACK_FNDECL(__name, __type) \
> @@ -400,7 +400,7 @@ ptmalloc_init (void)
> }
> }
> if (s && s[0] != '\0' && s[0] != '0')
> - __malloc_check_init ();
> + __malloc_debug_enable (MALLOC_CHECK_HOOK);
> #endif
Ok.
LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
More information about the Libc-alpha
mailing list