From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS17314 8.43.84.0/22 X-Spam-Status: No, score=-3.5 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,PDS_RDNS_DYNAMIC_FP,RCVD_IN_DNSWL_HI, RDNS_DYNAMIC,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (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 dcvr.yhbt.net (Postfix) with ESMTPS id EEEDF1F8C6 for ; Wed, 14 Jul 2021 07:05:43 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 71CF1397283F for ; Wed, 14 Jul 2021 07:05:42 +0000 (GMT) Received: from camel.birch.relay.mailchannels.net (camel.birch.relay.mailchannels.net [23.83.209.29]) by sourceware.org (Postfix) with ESMTPS id 52A50397282D; Wed, 14 Jul 2021 07:02:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 52A50397282D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org 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 18F3D760BE9; Wed, 14 Jul 2021 07:02:01 +0000 (UTC) Received: from pdx1-sub0-mail-a5.g.dreamhost.com (100-96-133-125.trex-nlb.outbound.svc.cluster.local [100.96.133.125]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 19A17760C72; Wed, 14 Jul 2021 07:02:00 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a5.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384) by 100.96.133.125 (trex/6.3.3); Wed, 14 Jul 2021 07:02:01 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Exultant-Juvenile: 5b40659f36750c46_1626246120900_21663748 X-MC-Loop-Signature: 1626246120900:3557396850 X-MC-Ingress-Time: 1626246120900 Received: from pdx1-sub0-mail-a5.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a5.g.dreamhost.com (Postfix) with ESMTP id B678E8AC84; Wed, 14 Jul 2021 00:01:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gotplt.org; h=subject:to :cc:references:from:message-id:date:mime-version:in-reply-to :content-type:content-transfer-encoding; s=gotplt.org; bh=WA/qdV NJoAdcJT/brytLxMIdL88=; b=nFFJFYoqvUAvz7z6zYRkgFpgFhLasXpVeCG1vl ohMDMO+sg54zzAGkVbBcvDJAzRjt4m5ZkZwPELM58ODHRnlUJUWGdsIer4USenYZ HuJM0epajcTg9d3rcO7o3XpYco0wKCad5KgvU5JhJsTw7g+cmA7s9nmeh5gaQ6w/ hASh8= Received: from [192.168.1.139] (unknown [1.186.101.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a5.g.dreamhost.com (Postfix) with ESMTPSA id 3E7F18AD00; Wed, 14 Jul 2021 00:01:56 -0700 (PDT) Subject: Re: [PATCH v8 03/10] Remove __morecore and __default_morecore To: Siddhesh Poyarekar , libc-alpha@sourceware.org References: <20210713073845.504356-1-siddhesh@sourceware.org> <20210713073845.504356-4-siddhesh@sourceware.org> X-DH-BACKEND: pdx1-sub0-mail-a5 From: Siddhesh Poyarekar Message-ID: <9925362a-d278-a0ad-f504-ae08ca93f628@gotplt.org> Date: Wed, 14 Jul 2021 12:31:50 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20210713073845.504356-4-siddhesh@sourceware.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: , Cc: fweimer@redhat.com, guillaume@morinfr.org Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" FYI, there's at least one use case[1] that is adversely affected by __morecore removal. I have closed it as WONTFIX citing that malloc does not always do the right thing with arbitrary morecore (and we don't even test it) anyway and it's a net win to remove them, but I mention it here too in the interest of a wider discussion. Guillaume, would you like to elaborate on the use case a bit more so that we know exactly what we're dealing with? Siddhesh [1] https://sourceware.org/bugzilla/show_bug.cgi?id=20646 On 7/13/21 1:08 PM, Siddhesh Poyarekar via Libc-alpha wrote: > Make the __morecore and __default_morecore symbols compat-only and > remove their declarations from the API. Also, include morecore.c > directly into malloc.c; this should ideally get merged into malloc in > a future cleanup. > --- > NEWS | 5 +++++ > include/stdlib.h | 3 --- > malloc/Makefile | 2 +- > malloc/arena.c | 12 ++---------- > malloc/hooks.c | 2 ++ > malloc/malloc.c | 7 +++---- > malloc/malloc.h | 8 -------- > malloc/morecore.c | 34 +++++++++++----------------------- > 8 files changed, 24 insertions(+), 49 deletions(-) > > diff --git a/NEWS b/NEWS > index 13ffe627da..18d9e65eb2 100644 > --- a/NEWS > +++ b/NEWS > @@ -113,6 +113,11 @@ Deprecated and removed features, and other changes affecting compatibility: > mtrace. Similar functionality can be achieved by using conditional > breakpoints within mtrace functions from within gdb. > > +* The __morecore and __after_morecore_hook malloc hooks and the default > + implementation __default_morecore have been removed from the API. Existing > + applications will continue to link against these symbols but the interfaces > + no longer have any effect on malloc. > + > Changes to build and runtime requirements: > > * On Linux, the shm_open, sem_open, and related functions now expect the > diff --git a/include/stdlib.h b/include/stdlib.h > index 1f6e1508e4..1c6f70b082 100644 > --- a/include/stdlib.h > +++ b/include/stdlib.h > @@ -306,9 +306,6 @@ libc_hidden_proto (__qfcvt_r) > # define MB_CUR_MAX (_NL_CURRENT_WORD (LC_CTYPE, _NL_CTYPE_MB_CUR_MAX)) > # endif > > -extern void *__default_morecore (ptrdiff_t) __THROW; > -libc_hidden_proto (__default_morecore) > - > struct abort_msg_s > { > unsigned int size; > diff --git a/malloc/Makefile b/malloc/Makefile > index d15729569b..020f781b59 100644 > --- a/malloc/Makefile > +++ b/malloc/Makefile > @@ -104,7 +104,7 @@ tests-exclude-mcheck = tst-mallocstate \ > tests-mcheck = $(filter-out $(tests-exclude-mcheck), $(tests)) > endif > > -routines = malloc morecore mcheck mtrace obstack reallocarray \ > +routines = malloc mcheck mtrace obstack reallocarray \ > scratch_buffer_dupfree \ > scratch_buffer_grow scratch_buffer_grow_preserve \ > scratch_buffer_set_array_size \ > diff --git a/malloc/arena.c b/malloc/arena.c > index 991fc21a7e..f1693ed48f 100644 > --- a/malloc/arena.c > +++ b/malloc/arena.c > @@ -274,14 +274,6 @@ next_env_entry (char ***position) > #endif > > > -#if defined(SHARED) || defined(USE_MTAG) > -static void * > -__failing_morecore (ptrdiff_t d) > -{ > - return (void *) MORECORE_FAILURE; > -} > -#endif > - > #ifdef SHARED > extern struct dl_open_hook *_dl_open_hook; > libc_hidden_proto (_dl_open_hook); > @@ -310,7 +302,7 @@ ptmalloc_init (void) > and that morecore does not support tagged regions, then > disable it. */ > if (__MTAG_SBRK_UNTAGGED) > - __morecore = __failing_morecore; > + __always_fail_morecore = true; > > mtag_enabled = true; > mtag_mmap_flags = __MTAG_MMAP_FLAGS; > @@ -323,7 +315,7 @@ ptmalloc_init (void) > generic sbrk implementation also enforces this, but it is not > used on Hurd. */ > if (!__libc_initial) > - __morecore = __failing_morecore; > + __always_fail_morecore = true; > #endif > > thread_arena = &main_arena; > diff --git a/malloc/hooks.c b/malloc/hooks.c > index 45c91d6502..4aa6dadcff 100644 > --- a/malloc/hooks.c > +++ b/malloc/hooks.c > @@ -20,6 +20,8 @@ > #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_34) > void weak_variable (*__after_morecore_hook) (void) = NULL; > compat_symbol (libc, __after_morecore_hook, __after_morecore_hook, GLIBC_2_0); > +void *(*__morecore)(ptrdiff_t); > +compat_symbol (libc, __morecore, __morecore, GLIBC_2_0); > #endif > > /* Hooks for debugging versions. The initial hooks just call the > diff --git a/malloc/malloc.c b/malloc/malloc.c > index 24e7854a0e..6e8fa9e424 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -384,12 +384,11 @@ __malloc_assert (const char *assertion, const char *file, unsigned int line, > #define TRIM_FASTBINS 0 > #endif > > - > /* Definition for getting more memory from the OS. */ > -#define MORECORE (*__morecore) > +#include "morecore.c" > + > +#define MORECORE (*__glibc_morecore) > #define MORECORE_FAILURE 0 > -void * __default_morecore (ptrdiff_t); > -void *(*__morecore)(ptrdiff_t) = __default_morecore; > > /* Memory tagging. */ > > diff --git a/malloc/malloc.h b/malloc/malloc.h > index 634b7db868..17ab9ee345 100644 > --- a/malloc/malloc.h > +++ b/malloc/malloc.h > @@ -76,14 +76,6 @@ extern void *valloc (size_t __size) __THROW __attribute_malloc__ > extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ > __wur __attr_dealloc_free; > > -/* Underlying allocation function; successive calls should return > - contiguous pieces of memory. */ > -extern void *(*__morecore) (ptrdiff_t __size) __MALLOC_DEPRECATED; > - > -/* Default value of `__morecore'. */ > -extern void *__default_morecore (ptrdiff_t __size) > -__THROW __attribute_malloc__ __MALLOC_DEPRECATED; > - > /* SVID2/XPG mallinfo structure */ > > struct mallinfo > diff --git a/malloc/morecore.c b/malloc/morecore.c > index 047228779b..8168ef158c 100644 > --- a/malloc/morecore.c > +++ b/malloc/morecore.c > @@ -15,39 +15,27 @@ > License along with the GNU C Library; if not, see > . */ > > -#ifndef _MALLOC_INTERNAL > -# define _MALLOC_INTERNAL > -# include > -#endif > - > -#ifndef __GNU_LIBRARY__ > -# define __sbrk sbrk > -#endif > - > -#ifdef __GNU_LIBRARY__ > -/* It is best not to declare this and cast its result on foreign operating > - systems with potentially hostile include files. */ > - > -# include > -# include > -extern void *__sbrk (ptrdiff_t increment) __THROW; > -libc_hidden_proto (__sbrk) > -#endif > - > -#ifndef NULL > -# define NULL 0 > +#if defined(SHARED) || defined(USE_MTAG) > +static bool __always_fail_morecore = false; > #endif > > /* Allocate INCREMENT more bytes of data space, > and return the start of data space, or NULL on errors. > If INCREMENT is negative, shrink data space. */ > void * > -__default_morecore (ptrdiff_t increment) > +__glibc_morecore (ptrdiff_t increment) > { > +#if defined(SHARED) || defined(USE_MTAG) > + if (__always_fail_morecore) > + return NULL; > +#endif > + > void *result = (void *) __sbrk (increment); > if (result == (void *) -1) > return NULL; > > return result; > } > -libc_hidden_def (__default_morecore) > +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_34) > +compat_symbol (libc, __glibc_morecore, __default_morecore, GLIBC_2_0); > +#endif >