From patchwork Fri Jul 2 02:35:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 1499846 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=ju0fY5hw; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4GGK6Y3YPtz9sV8 for ; Fri, 2 Jul 2021 12:39:33 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 037533858004 for ; Fri, 2 Jul 2021 02:39:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 037533858004 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1625193571; bh=Zkqu2HHNeImzx9mNgYLB/YHmlFn4QiApX/+8cnklOt0=; 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=ju0fY5hwQGeNyioMCyuh9v4wVVvKgC4k1BLWQZdYgxexLMqS366hG2q3DU6Z40YPZ QZiG/xCD+T/ekqkqjfEcc6ccMqXoAyvnNuubXqW0Fp+jMWM1WcNHr1DAtjh+bWyYLo Aa7sAbbFDL8iWCX3bpfNuyMLtFIUMFxjcbvjyIoQ= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from cross.elm.relay.mailchannels.net (cross.elm.relay.mailchannels.net [23.83.212.46]) by sourceware.org (Postfix) with ESMTPS id 31DAC3973025 for ; Fri, 2 Jul 2021 02:36:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 31DAC3973025 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 1F896361DFE; Fri, 2 Jul 2021 02:36:21 +0000 (UTC) Received: from pdx1-sub0-mail-a42.g.dreamhost.com (100-96-11-26.trex.outbound.svc.cluster.local [100.96.11.26]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id A8794361366; Fri, 2 Jul 2021 02:36:20 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a42.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384) by 100.96.11.26 (trex/6.3.3); Fri, 02 Jul 2021 02:36:21 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Well-Made-Lyrical: 4d5e579d7272f7b1_1625193380965_4003235483 X-MC-Loop-Signature: 1625193380964:2450378308 X-MC-Ingress-Time: 1625193380964 Received: from pdx1-sub0-mail-a42.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a42.g.dreamhost.com (Postfix) with ESMTP id 62FAC897DF; Fri, 2 Jul 2021 02:36:20 +0000 (UTC) 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-a42.g.dreamhost.com (Postfix) with ESMTPSA id AA909897E8; Fri, 2 Jul 2021 02:36:18 +0000 (UTC) X-DH-BACKEND: pdx1-sub0-mail-a42 To: libc-alpha@sourceware.org Subject: [PATCH v3 04/10] malloc: Move malloc hook references to hooks.c Date: Fri, 2 Jul 2021 08:05:40 +0530 Message-Id: <20210702023546.3081774-5-siddhesh@sourceware.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210702023546.3081774-1-siddhesh@sourceware.org> References: <20210702023546.3081774-1-siddhesh@sourceware.org> MIME-Version: 1.0 X-Spam-Status: No, score=-3494.1 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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+incoming=patchwork.ozlabs.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 ();