From patchwork Thu Jun 24 18:23:06 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 44002 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CDA5F39B042A for ; Thu, 24 Jun 2021 18:25:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CDA5F39B042A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1624559132; bh=10fo++mTnl4fj60/88YUrZSnhgRyC6G6s8UKFk955nk=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ERHcMrDhkJtF9P1V/VsiUbk10Sxv5TdENcf2sK+25x0/++Qf4mWSHCXt4XIpIDRdJ ofs3mJ6TUbSCLZ6I3pXtcK+r6SBvdPOfbumjidGBfW1Jb36n8lMPvMbW+vcTU2mimL e9+3O+WM/dnvk+By+FXTWe8W7ul1wqlFFmuL6H1M= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from camel.birch.relay.mailchannels.net (camel.birch.relay.mailchannels.net [23.83.209.29]) by sourceware.org (Postfix) with ESMTPS id D67FC38618B8 for ; Thu, 24 Jun 2021 18:23:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D67FC38618B8 X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 0A9A9182972; Thu, 24 Jun 2021 18:23:40 +0000 (UTC) Received: from pdx1-sub0-mail-a66.g.dreamhost.com (100-96-27-202.trex.outbound.svc.cluster.local [100.96.27.202]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 75A09182A2F; Thu, 24 Jun 2021 18:23:39 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a66.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384) by 100.96.27.202 (trex/6.3.3); Thu, 24 Jun 2021 18:23:40 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Drop-Spill: 7ab97bb264839079_1624559019847_921423781 X-MC-Loop-Signature: 1624559019847:258328739 X-MC-Ingress-Time: 1624559019846 Received: from pdx1-sub0-mail-a66.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a66.g.dreamhost.com (Postfix) with ESMTP id F3AC88DD5D; Thu, 24 Jun 2021 11:23:38 -0700 (PDT) Received: from rhbox.intra.reserved-bit.com (unknown [1.186.101.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a66.g.dreamhost.com (Postfix) with ESMTPSA id 616EC8CE56; Thu, 24 Jun 2021 11:23:35 -0700 (PDT) X-DH-BACKEND: pdx1-sub0-mail-a66 To: libc-alpha@sourceware.org Subject: [PATCH 2/8] malloc: Move malloc hook references to hooks.c Date: Thu, 24 Jun 2021 23:53:06 +0530 Message-Id: <20210624182312.236596-3-siddhesh@sourceware.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210624182312.236596-1-siddhesh@sourceware.org> References: <20210624182312.236596-1-siddhesh@sourceware.org> MIME-Version: 1.0 X-Spam-Status: No, score=-3494.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_ASCII_DIVIDERS, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Siddhesh Poyarekar via Libc-alpha From: Siddhesh Poyarekar Reply-To: Siddhesh Poyarekar Cc: fweimer@redhat.com Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" Make the core malloc code hooks free and instead only have entry points for _*_debug_before inline functions. This also introduces the first (albeit very constrained) breakage for malloc hooks behaviour. The hook variables no longer call ptmalloc_init, it is called directly on first invocation of an allocator function. This breaks debugging hooks that may depend on overriding the hook symbols with their own implementations due to which ptmalloc_init is never called. In other words, it breaks hooks users that use the malloc hooks to have their own implementation of malloc that is conflicting with glibc malloc, which is not the intended use of the hooks as documented. None of the three debugging hooks in glibc depend on this behaviour and hence continue to work as before. In any case, future patches that move the hooks out into their own layer ought to bring back this functionality. As a result, the breakage will not be visible to the user in the end. Reviewed-by: DJ Delorie --- malloc/arena.c | 2 - malloc/hooks.c | 110 ++++++++++++++++++++++++++++++++------- malloc/malloc-internal.h | 1 + malloc/malloc.c | 85 ++++++++++-------------------- 4 files changed, 119 insertions(+), 79 deletions(-) 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) - /* A heap is a single contiguous memory region holding (coalesceable) malloc_chunks. It is allocated with mmap() and always starts at an address aligned to HEAP_MAX_SIZE. */ 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); -static void * -malloc_hook_ini (size_t sz, const void *caller) +#include "malloc-check.c" + +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; } -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; } -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; } -#include "malloc-check.c" +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; +} + +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; +} #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_25) 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) /* Called in the parent process before a fork. */ void __malloc_fork_lock_parent (void) attribute_hidden; 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; /* This function is called from the arena shutdown hook, to free the @@ -2065,6 +2041,9 @@ free_perturb (char *p, size_t n) #include +/* ----------------- 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 -------------- */ /* @@ -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)); #if USE_TCACHE /* int_free also calls request2size, be careful to not pad twice. */ size_t tbytes; @@ -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; if (mem == 0) /* free(0) has no effect */ return; @@ -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; #if REALLOC_ZERO_BYTES_FREES if (bytes == 0 && oldmem != NULL) @@ -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); } @@ -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; /* If we need less alignment than we give anyway, just relay to malloc. */ if (alignment <= MALLOC_ALIGNMENT) @@ -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; MAYBE_INIT_TCACHE ();