diff mbox series

[v7,2/8] Remove __morecore and __default_morecore

Message ID 20210708174325.3243721-3-siddhesh@sourceware.org
State New
Headers show
Series malloc hooks removal | expand

Commit Message

Siddhesh Poyarekar July 8, 2021, 5:43 p.m. UTC
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(-)

Comments

Florian Weimer July 9, 2021, 6:27 a.m. UTC | #1
* 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
Siddhesh Poyarekar July 9, 2021, 7:02 a.m. UTC | #2
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
Florian Weimer July 9, 2021, 7:46 a.m. UTC | #3
* 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 mbox series

Patch

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