[PATCH v4 05/10] glibc.malloc.check: Wean away from malloc hooks
Carlos O'Donell
carlos@redhat.com
Fri Jul 2 19:06:42 GMT 2021
On 7/2/21 7:38 AM, Siddhesh Poyarekar wrote:
> 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.
LGTM.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> Reviewed-by: DJ Delorie <dj@redhat.com>
> ---
> malloc/arena.c | 4 +--
> malloc/hooks.c | 63 ++++++++++++++++++++++++++++++++++++++++++-
> malloc/malloc-check.c | 30 ++++++---------------
> malloc/malloc.c | 9 +------
> 4 files changed, 73 insertions(+), 33 deletions(-)
>
> 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);
> }
>
> # 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
>
> #if HAVE_MALLOC_INIT_HOOK
> 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>
> +
> /* 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
> @@ -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;
> +
> /* Forward declarations. */
>
> #if HAVE_MALLOC_INIT_HOOK
> @@ -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;
> +}
> +
> #include "malloc-check.c"
>
> static __always_inline bool
> @@ -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;
> }
>
> @@ -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;
> }
>
> @@ -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;
> }
>
> @@ -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;
> }
>
> @@ -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;
> }
>
> @@ -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);
>
> /* Patch the dumped heap. We no longer try to integrate into the
> existing heap. Instead, we mark the existing chunks as mmapped.
> 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;
> -}
> -
> /* When memory is tagged, the checking data is stored in the user part
> of the chunk. We can't rely on the user not having modified the
> tags, so fetch the tag at each location before dereferencing
> @@ -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));
>
> for (size = CHUNK_HDR_SZ + memsize (p) - 1;
> (c = *SAFE_CHAR_OFFSET (p, size)) != magic;
> @@ -203,7 +189,7 @@ top_check (void)
> }
>
> static void *
> -malloc_check (size_t sz, const void *caller)
> +malloc_check (size_t sz)
> {
> void *victim;
> size_t nb;
> @@ -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)
> {
> mchunkptr p;
>
> @@ -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)
> {
> INTERNAL_SIZE_T chnb;
> void *newmem = 0;
> @@ -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);
>
> if (bytes == 0)
> {
> - free_check (oldmem, NULL);
> + free_check (oldmem);
> return NULL;
> }
>
> @@ -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);
>
> if (alignment < MINSIZE)
> alignment = MINSIZE;
> 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);
> -
> /* ------------------ MMAP support ------------------ */
>
>
> @@ -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);
>
> if (chunk_is_mmapped (p))
>
--
Cheers,
Carlos.
More information about the Libc-alpha
mailing list