Message ID | 20210708174325.3243721-3-siddhesh@sourceware.org |
---|---|
State | New |
Headers | show |
Series | malloc hooks removal | expand |
* Siddhesh Poyarekar: > 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 This fails to assemble on ARC (and elsewhere with GCC 9 and earlier): /tmp/ccEXL6bz.s: Assembler messages: /tmp/ccEXL6bz.s: Error: `__morecore@GLIBC_2.32' can't be versioned to common sym bol '__morecore' You need to add __attribute__ ((morecore)). I posted a generic fix for this: Force building with -fno-common <https://sourceware.org/pipermail/libc-alpha/2021-July/128716.html> But until then, every instance needs to be fixed manually. Thanks, Florian
On 7/9/21 11:57 AM, Florian Weimer via Libc-alpha wrote: > * Siddhesh Poyarekar: > >> 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 > > This fails to assemble on ARC (and elsewhere with GCC 9 and earlier): > > /tmp/ccEXL6bz.s: Assembler messages: > /tmp/ccEXL6bz.s: Error: `__morecore@GLIBC_2.32' can't be versioned to common sym > bol '__morecore' > > You need to add __attribute__ ((morecore)). I could do this (I know you mean nocommon there)... > I posted a generic fix for this: > > Force building with -fno-common > <https://sourceware.org/pipermail/libc-alpha/2021-July/128716.html> > > But until then, every instance needs to be fixed manually. ... or I could review your generic fix instead. It seems like a much better cleanup than having to specify the attribute on everything that changes. What do you think? Siddhesh
* Siddhesh Poyarekar: > On 7/9/21 11:57 AM, Florian Weimer via Libc-alpha wrote: >> * Siddhesh Poyarekar: >> >>> 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 >> This fails to assemble on ARC (and elsewhere with GCC 9 and >> earlier): >> /tmp/ccEXL6bz.s: Assembler messages: >> /tmp/ccEXL6bz.s: Error: `__morecore@GLIBC_2.32' can't be versioned to common sym >> bol '__morecore' >> You need to add __attribute__ ((morecore)). > > I could do this (I know you mean nocommon there)... > >> I posted a generic fix for this: >> Force building with -fno-common >> <https://sourceware.org/pipermail/libc-alpha/2021-July/128716.html> >> But until then, every instance needs to be fixed manually. > > ... or I could review your generic fix instead. It seems like a much > better cleanup than having to specify the attribute on everything that > changes. What do you think? Do we want this change for glibc 2.34 at this point? I wasn't sure. Carlos, I guess this is a question for you. Thanks, Florian
diff --git a/NEWS b/NEWS index 8e72946c3f..89280df45c 100644 --- a/NEWS +++ b/NEWS @@ -97,6 +97,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 37a9a4efab..8050707a84 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -98,7 +98,7 @@ tests-exclude-mcheck = tst-mallocstate \ tests-mcheck = $(filter-out $(tests-exclude-mcheck), $(tests)) -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 6340eb133b..767df0982e 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 <https://www.gnu.org/licenses/>. */ -#ifndef _MALLOC_INTERNAL -# define _MALLOC_INTERNAL -# include <malloc.h> -#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 <stddef.h> -# include <stdlib.h> -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