[PATCH v4] malloc: Improve malloc initialization

[PATCH v4] malloc: Improve malloc initialization

[Wilco Dijkstra]

v4: Remove remaining uses of __malloc_initialized.

Move malloc initialization to __libc_early_init.  Use a hidden __ptmalloc_init
for initialization and a weak call to avoid pulling in the system malloc in a
static binary.  All previous initialization checks can now be removed.

Passes regress, OK for commit?

---

diff --git a/elf/libc_early_init.c b/elf/libc_early_init.c
index 07202317534a4edbd2202c6e45770094cadb67f6..24b99d82fe28f4c324800dbc665fb0e9c702c228 100644
--- a/elf/libc_early_init.c
+++ b/elf/libc_early_init.c
@@ -24,6 +24,7 @@
 #include <pthread_early_init.h>
 #include <sys/single_threaded.h>
 #include <getrandom-internal.h>
+#include <malloc/malloc-internal.h>
 
 #ifdef SHARED
 _Bool __libc_initial;
@@ -32,6 +33,9 @@ _Bool __libc_initial;
 void
 __libc_early_init (_Bool initial)
 {
+  /* Initialize system malloc.  */
+  call_function_static_weak (__ptmalloc_init);
+
   /* Initialize ctype data.  */
   __ctype_init ();
 
diff --git a/malloc/arena.c b/malloc/arena.c
index 5672c699aa1a0306a13ebc54a08263f35024b3f1..90c526f23ba1b2d83a3479245179d1d93b09302d 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -113,9 +113,6 @@ static mstate free_list;
    acquired.  */
 __libc_lock_define_initialized (static, list_lock);
 
-/* Already initialized? */
-static bool __malloc_initialized = false;
-
 /**************************************************************************/
 
 
@@ -168,9 +165,6 @@ arena_for_chunk (mchunkptr ptr)
 void
 __malloc_fork_lock_parent (void)
 {
-  if (!__malloc_initialized)
-    return;
-
   /* We do not acquire free_list_lock here because we completely
      reconstruct free_list in __malloc_fork_unlock_child.  */
 
@@ -188,9 +182,6 @@ __malloc_fork_lock_parent (void)
 void
 __malloc_fork_unlock_parent (void)
 {
-  if (!__malloc_initialized)
-    return;
-
   for (mstate ar_ptr = &main_arena;; )
     {
       __libc_lock_unlock (ar_ptr->mutex);
@@ -204,9 +195,6 @@ __malloc_fork_unlock_parent (void)
 void
 __malloc_fork_unlock_child (void)
 {
-  if (!__malloc_initialized)
-    return;
-
   /* Push all arenas to the free list, except thread_arena, which is
      attached to the current thread.  */
   __libc_lock_init (free_list_lock);
@@ -259,14 +247,9 @@ TUNABLE_CALLBACK_FNDECL (set_hugetlb, size_t)
 static void tcache_key_initialize (void);
 #endif
 
-static void
-ptmalloc_init (void)
+void
+__ptmalloc_init (void)
 {
-  if (__malloc_initialized)
-    return;
-
-  __malloc_initialized = true;
-
 #if USE_TCACHE
   tcache_key_initialize ();
 #endif
diff --git a/malloc/malloc-check.c b/malloc/malloc-check.c
index c5265ecb91f1ae6eba9589f134158abba1126917..fbb030116c3cc7e0b3a577822f11467d97c82b8d 100644
--- a/malloc/malloc-check.c
+++ b/malloc/malloc-check.c
@@ -389,7 +389,7 @@ initialize_malloc_check (void)
 {
   /* This is the copy of the malloc initializer that we pulled in along with
      malloc-check.  This does not affect any of the libc malloc structures.  */
-  ptmalloc_init ();
+  __ptmalloc_init ();
   TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (set_mallopt_check));
   return __is_malloc_debug_enabled (MALLOC_CHECK_HOOK);
 }
diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h
index d88ed20c4622f9e03de6a26b4d8a3bb6fff7a83e..0f1b3a1d5d54a75d8289b5f7ff78c6985f751941 100644
--- a/malloc/malloc-internal.h
+++ b/malloc/malloc-internal.h
@@ -40,4 +40,7 @@ void __malloc_arena_thread_freeres (void) attribute_hidden;
 /* Activate a standard set of debugging hooks. */
 void __malloc_check_init (void) attribute_hidden;
 
+/* Initialize malloc.  */
+void __ptmalloc_init (void) attribute_hidden;
+
 #endif /* _MALLOC_INTERNAL_H */
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 9d860eac9cc923ef8f20218eb37f35b926f3ef82..6264f6be4d450686973ba690c3e602087ae0b42e 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1937,7 +1937,7 @@ static struct malloc_par mp_ =
 /*
    Initialize a malloc_state struct.
 
-   This is called from ptmalloc_init () or from _int_new_arena ()
+   This is called from __ptmalloc_init () or from _int_new_arena ()
    when creating a new arena.
  */
 
@@ -3344,9 +3344,6 @@ __libc_malloc2 (size_t bytes)
   mstate ar_ptr;
   void *victim;
 
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
   MAYBE_INIT_TCACHE ();
 
   if (SINGLE_THREAD_P)
@@ -3452,9 +3449,6 @@ __libc_realloc (void *oldmem, size_t bytes)
 
   void *newp;             /* chunk to return */
 
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
 #if REALLOC_ZERO_BYTES_FREES
   if (bytes == 0 && oldmem != NULL)
     {
@@ -3580,9 +3574,6 @@ libc_hidden_def (__libc_realloc)
 void *
 __libc_memalign (size_t alignment, size_t bytes)
 {
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
   void *address = RETURN_ADDRESS (0);
   return _mid_memalign (alignment, bytes, address);
 }
@@ -3593,9 +3584,6 @@ void *
 weak_function
 aligned_alloc (size_t alignment, size_t bytes)
 {
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
 /* Similar to memalign, but starting with ISO C17 the standard
    requires an error for alignments that are not supported by the
    implementation.  Valid alignments for the current implementation
@@ -3695,9 +3683,6 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
 void *
 __libc_valloc (size_t bytes)
 {
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
   void *address = RETURN_ADDRESS (0);
   size_t pagesize = GLRO (dl_pagesize);
   return _mid_memalign (pagesize, bytes, address);
@@ -3706,9 +3691,6 @@ __libc_valloc (size_t bytes)
 void *
 __libc_pvalloc (size_t bytes)
 {
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
   void *address = RETURN_ADDRESS (0);
   size_t pagesize = GLRO (dl_pagesize);
   size_t rounded_bytes;
@@ -3743,9 +3725,6 @@ __libc_calloc (size_t n, size_t elem_size)
 
   sz = bytes;
 
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
 #if USE_TCACHE
   size_t tc_idx = usize2tidx (bytes);
   if (tcache_available (tc_idx))
@@ -5208,9 +5187,6 @@ __malloc_trim (size_t s)
 {
   int result = 0;
 
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
   mstate ar_ptr = &main_arena;
   do
     {
@@ -5327,9 +5303,6 @@ __libc_mallinfo2 (void)
   struct mallinfo2 m;
   mstate ar_ptr;
 
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
   memset (&m, 0, sizeof (m));
   ar_ptr = &main_arena;
   do
@@ -5378,8 +5351,6 @@ __malloc_stats (void)
   mstate ar_ptr;
   unsigned int in_use_b = mp_.mmapped_mem, system_b = in_use_b;
 
-  if (!__malloc_initialized)
-    ptmalloc_init ();
   _IO_flockfile (stderr);
   int old_flags2 = stderr->_flags2;
   stderr->_flags2 |= _IO_FLAGS2_NOTCANCEL;
@@ -5560,8 +5531,6 @@ __libc_mallopt (int param_number, int value)
   mstate av = &main_arena;
   int res = 1;
 
-  if (!__malloc_initialized)
-    ptmalloc_init ();
   __libc_lock_lock (av->mutex);
 
   LIBC_PROBE (memory_mallopt, 2, param_number, value);
@@ -5777,11 +5746,14 @@ malloc_printerr (const char *str)
 }
 
 #if USE_TCACHE
+
+static volatile int dummy_var;
+
 static __attribute_noinline__ void
 malloc_printerr_tail (const char *str)
 {
   /* Ensure this cannot be a no-return function.  */
-  if (!__malloc_initialized)
+  if (dummy_var)
     return;
   malloc_printerr (str);
 }
@@ -5794,9 +5766,6 @@ __posix_memalign (void **memptr, size_t alignment, size_t size)
 {
   void *mem;
 
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
   /* Test whether the SIZE argument is valid.  It must be a power of
      two multiple of sizeof (void *).  */
   if (alignment % sizeof (void *) != 0
@@ -5837,11 +5806,6 @@ __malloc_info (int options, FILE *fp)
   size_t total_aspace = 0;
   size_t total_aspace_mprotect = 0;
 
-
-
-  if (!__malloc_initialized)
-    ptmalloc_init ();
-
   fputs ("<malloc version=\"1\">\n", fp);
 
   /* Iterate over all arenas currently in use.  */

Re: [PATCH v4] malloc: Improve malloc initialization

[Florian Weimer]

* Wilco Dijkstra:

> v4: Remove remaining uses of __malloc_initialized.
>
> Move malloc initialization to __libc_early_init.  Use a hidden __ptmalloc_init
> for initialization and a weak call to avoid pulling in the system malloc in a
> static binary.  All previous initialization checks can now be removed.
>
> Passes regress, OK for commit?

This version looks okay to me.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian

Re: [PATCH v4] malloc: Improve malloc initialization

[Tulio Magno Quites Machado Filho]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:

> v4: Remove remaining uses of __malloc_initialized.
>
> Move malloc initialization to __libc_early_init.  Use a hidden __ptmalloc_init
> for initialization and a weak call to avoid pulling in the system malloc in a
> static binary.  All previous initialization checks can now be removed.
>
> Passes regress, OK for commit?

I bisected crashes on 32-bit x86 to this commit.
It's still unclear to me what is causing the issue, but one thing caught
my attention: malloc() started to return addresses from mmap'ed areas
while it used to return addresses from the software break for this
particular program (dynamically linked llvm-objdump).

Before this patch, I was seeing this output from ltrace:

    brk@SYS(nil)                           = 0x893f000
    brk@SYS(0x8960000)                     = 0x8960000
    brk@SYS(0x8961000)                     = 0x8961000
    brk@SYS(0x8982000)                     = 0x8982000
    libstdc++.so.6->malloc(28)             = 0x8979070
    libLLVM.so.20.1->malloc(80064 <unfinished ...>
    brk@SYS(0x89ad000)                     = 0x89ad000
    <... malloc resumed> )                 = 0x8979090
    libstdc++.so.6->malloc(132)            = 0x8970910
    libstdc++.so.6->malloc(24)             = 0x898c960
    libstdc++.so.6->malloc(24)             = 0x898c980
    libstdc++.so.6->free(0x898c960)        = <void>
    libstdc++.so.6->free(0x898c980)        = <void>
    libLLVM.so.20.1->malloc(12)            = 0x8960da0
    libstdc++.so.6->malloc(296)            = 0x898c9a0
    libstdc++.so.6->malloc(52)             = 0x898cad0
    libstdc++.so.6->malloc(52)             = 0x898ce20
    libstdc++.so.6->malloc(52)             = 0x898ce60
    libLLVM.so.20.1->free(0x8960da0)       = <void>
    libstdc++.so.6->malloc(16)             = 0x898c980
    libstdc++.so.6->malloc(24)             = 0x898c960
    libstdc++.so.6->malloc(79)             = 0x898cea0
    libLLVM.so.20.1->malloc(168)           = 0x898cf00
    libstdc++.so.6->malloc(232)            = 0x898cfb0
    libstdc++.so.6->malloc(28)             = 0x898d0a0
    libstdc++.so.6->malloc(36)             = 0x898d0c0
    libstdc++.so.6->malloc(72)             = 0x898d0f0
    libstdc++.so.6->free(0x898d0a0)        = <void>
    libstdc++.so.6->free(0x898cfb0)        = <void>
    libLLVM.so.20.1->free(0x898cf00)       = <void>
    libstdc++.so.6->malloc(112)            = 0x898d140

Now I'm seeing:

    mmap2@SYS(0, 0x100000, 3, 34)          = 0xee830000
    libstdc++.so.6->malloc(28)             = 0xee86a070
    libLLVM.so.20.1->malloc(80064)         = 0xee86a090
    libstdc++.so.6->malloc(132)            = 0xee861910
    libstdc++.so.6->malloc(24)             = 0xee87d960
    libstdc++.so.6->malloc(24)             = 0xee87d980
    libstdc++.so.6->free(0xee87d960)       = <void>
    libstdc++.so.6->free(0xee87d980)       = <void>
    libLLVM.so.20.1->malloc(12)            = 0xee851da0
    libstdc++.so.6->malloc(296)            = 0xee87d9a0
    libstdc++.so.6->malloc(52)             = 0xee87dad0
    libstdc++.so.6->malloc(52)             = 0xee87de20
    libstdc++.so.6->malloc(52)             = 0xee87de60
    libLLVM.so.20.1->free(0xee851da0)      = <void>
    libstdc++.so.6->malloc(16)             = 0xee87d980
    libstdc++.so.6->malloc(24)             = 0xee87d960
    libstdc++.so.6->malloc(79)             = 0xee87dea0
    libLLVM.so.20.1->malloc(168)           = 0xee87df00
    libstdc++.so.6->malloc(232)            = 0xee87dfb0
    libstdc++.so.6->malloc(28)             = 0xee87e0a0

It looks like that even glibc memory allocations are not using the
software break anymore (see the first lines calling brk() without a call
from the other libraries).

-- 
Tulio Magno

Re: [PATCH v4] malloc: Improve malloc initialization

[Florian Weimer]

* Tulio Magno Quites Machado Filho:

> Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
>
>> v4: Remove remaining uses of __malloc_initialized.
>>
>> Move malloc initialization to __libc_early_init.  Use a hidden __ptmalloc_init
>> for initialization and a weak call to avoid pulling in the system malloc in a
>> static binary.  All previous initialization checks can now be removed.
>>
>> Passes regress, OK for commit?
>
> I bisected crashes on 32-bit x86 to this commit.
> It's still unclear to me what is causing the issue, but one thing caught
> my attention: malloc() started to return addresses from mmap'ed areas
> while it used to return addresses from the software break for this
> particular program (dynamically linked llvm-objdump).

That part (the switch to mmap) I can explain.  The order in
__libc_early_init is currently this:

  /* Initialize system malloc.  */
  call_function_static_weak (__ptmalloc_init);

  /* Initialize ctype data.  */
  __ctype_init ();

  /* Only the outer namespace is marked as single-threaded.  */
  __libc_single_threaded = initial;

So __libc_single_threaded is false when __ptmalloc_init is called, and
we disable the use of brk.

> It looks like that even glibc memory allocations are not using the
> software break anymore (see the first lines calling brk() without a call
> from the other libraries).

Still the switch to mmap is not supposed to result in crashes.  This
could point to a completely different bug.  What is the nature of the
crashes?

Thanks,
Florian

Re: [PATCH v4] malloc: Improve malloc initialization

[Wilco Dijkstra]

Hi Florian,

I believe it is the __libc_initial not set yet since ptmalloc_init() does: 

#if defined SHARED && IS_IN (libc)
  /* In case this libc copy is in a non-default namespace, never use
     brk.  Likewise if dlopened from statically linked program.  The
     generic sbrk implementation also enforces this, but it is not
     used on Hurd.  */
  if (!__libc_initial)
    __always_fail_morecore = true;
#endif

We can easily move the initialization.

However malloc will always switch to mmap due to:

  if (mp_.hp_pagesize > 0 && mp_.hp_pagesize <= heap_max_size ())
    {
      /* Force mmap for main arena instead of sbrk, so MAP_HUGETLB is always
         tried.  Also tune the mmap threshold, so allocation smaller than the
         large page will also try to use large pages by falling back
         to sysmalloc_mmap_fallback on sysmalloc.  */
      if (!TUNABLE_IS_INITIALIZED (mmap_threshold))
        do_set_mmap_threshold (mp_.hp_pagesize);
      __always_fail_morecore = true;
    }

So this must be a CPU without huge pages...

Is there actually an advantage in keeping the option to use sbrk() since
there are now quite a few reasons to not use it? Many years ago it was
slightly faster than mmap, but I'm not sure that is still true.

Cheers,
Wilco

Re: [PATCH v4] malloc: Improve malloc initialization

[Konrad Kleine]

Hi Florian,

On 23/05/2025 09:39, Florian Weimer wrote:
 > * Tulio Magno Quites Machado Filho:
 >
 >> Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
 >>
 >>> v4: Remove remaining uses of __malloc_initialized.
 >>>
 >>> Move malloc initialization to __libc_early_init.  Use a hidden 
__ptmalloc_init
 >>> for initialization and a weak call to avoid pulling in the system 
malloc in a
 >>> static binary.  All previous initialization checks can now be removed.
 >>>
 >>> Passes regress, OK for commit?
 >>
 >> I bisected crashes on 32-bit x86 to this commit.
 >> It's still unclear to me what is causing the issue, but one thing caught
 >> my attention: malloc() started to return addresses from mmap'ed areas
 >> while it used to return addresses from the software break for this
 >> particular program (dynamically linked llvm-objdump).
 >
 > That part (the switch to mmap) I can explain.  The order in
 > __libc_early_init is currently this:
 >
 >    /* Initialize system malloc.  */
 >    call_function_static_weak (__ptmalloc_init);
 >
 >    /* Initialize ctype data.  */
 >    __ctype_init ();
 >
 >    /* Only the outer namespace is marked as single-threaded.  */
 >    __libc_single_threaded = initial;
 >
 > So __libc_single_threaded is false when __ptmalloc_init is called, and
 > we disable the use of brk.
 >
 >> It looks like that even glibc memory allocations are not using the
 >> software break anymore (see the first lines calling brk() without a call
 >> from the other libraries).
 >
 > Still the switch to mmap is not supposed to result in crashes.  This
 > could point to a completely different bug.  What is the nature of the
 > crashes?

The nature of the crash is that it occurred on 32-bit rawhide when 
building LLVM and especially running this test from an LLVM-LIT file:

https://github.com/llvm/llvm-project/blob/5c3a99760274a06f8cb7e7247ce69c2fde5fbf2a/llvm/test/Object/macho-invalid.test#L287-L288

Here's the crash (segfault) when running in GDB:

$ gdb --quiet --args \
  
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/redhat-linux-build/bin/llvm-objdump 
\
     --macho \
     --private-headers \
  
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/test/Object/Inputs/macho-invalid-dylib-cmdsize-past-eof
Reading symbols from 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/redhat-linux-build/bin/llvm-objdump...
(gdb) r
Starting program: 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/redhat-linux-build/bin/llvm-objdump 
--macho --private-headers 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/test/Object/Inputs/macho-invalid-dylib-cmdsize-past-eof
Function(s) ^std::(move|forward|as_const|(__)?addressof) will be skipped 
when stepping.
Function(s) ^std::(shared|unique)_ptr<.*>::(get|operator) will be 
skipped when stepping.
Function(s) 
^std::(basic_string|vector|array|deque|(forward_)?list|(unordered_|flat_)?(multi)?(map|set)|span)<.*>::(c?r?(begin|end)|front|back|data|size|empty) 
will be skipped when stepping.
Function(s) ^std::(basic_string|vector|array|deque|span)<.*>::operator.] 
will be skipped when stepping.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
checkDylibCommand () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:760
760         if (P[i] == '\0')
Missing rpms, try: dnf --enablerepo='*debug*' install 
libstdc++-debuginfo-15.1.1-1.fc43.i686 
libgcc-debuginfo-15.1.1-1.fc43.i686 
glibc-debuginfo-2.41.9000-13.fc43.i686 
libffi-debuginfo-3.4.8-1.fc43.i686 
libedit-debuginfo-3.1-55.20250104cvs.fc42.i686 
zlib-ng-compat-debuginfo-2.2.4-2.fc43.i686 
libzstd-debuginfo-1.5.7-1.fc43.i686 
libxml2-debuginfo-2.12.10-1.fc43.i686 
ncurses-libs-debuginfo-6.5-5.20250125.fc42.i686 
xz-libs-debuginfo-5.8.1-1.fc43.i686
(gdb) bt
#0  checkDylibCommand () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:760
#1  0xf159206e in MachOObjectFile () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:1435
#2  0xf15af0f1 in create () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:1258
#3  createMachOObjectFile () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:5330
#4  0xf15b91f5 in createObjectFile () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/ObjectFile.cpp:193
#5  0xf15bf3b7 in createSymbolicFile () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/SymbolicFile.cpp:71
#6  0xf14ee817 in createBinary () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/Binary.cpp:78
#7  0xf14eea37 in createBinary () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/Binary.cpp:116
#8  0x565c017d in parseInputMachO () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/tools/llvm-objdump/MachODump.cpp:2538
#9  0x56563d01 in dumpInput () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/tools/llvm-objdump/llvm-objdump.cpp:3392
#10 
for_each<__gnu_cxx::__normal_iterator<std::__cxx11::basic_string<char, 
std::char_traits<char>, std::allocator<char> >*, 
std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, 
std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, 
std::char_traits<char>, std::allocator<char> > > > >, void 
(*)(llvm::StringRef)> () at 
/usr/lib/gcc/i686-redhat-linux/15/../../../../include/c++/15/bits/stl_algo.h:3798
#11 for_each<std::vector<std::__cxx11::basic_string<char, 
std::char_traits<char>, std::allocator<char> >, 
std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, 
std::allocator<char> > > >&, void (*)(llvm::StringRef)> ()
     at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/include/llvm/ADT/STLExtras.h:1733
#12 llvm_objdump_main () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/tools/llvm-objdump/llvm-objdump.cpp:3748
#13 0x56621227 in main () at 
/builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/redhat-linux-build/tools/llvm-objdump/llvm-objdump-driver.cpp:17
(gdb)

I hope this helps.

Konrad

 >
 > Thanks,
 > Florian
 >

Re: [PATCH v4] malloc: Improve malloc initialization

[Tulio Magno Quites Machado Filho]

Konrad Kleine <konrad.kleine@posteo.de> writes:
> #0  checkDylibCommand () at 
> /builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:760
> #1  0xf159206e in MachOObjectFile () at 
> /builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:1435

It's important to keep in mind that checkDylibCommand() shouldn't have
been called.  Somewhere in MachOObjectFile (), we take/do not take a
branch wrongly.
I apologize for not being more specific. The debug information in this
file is limited.

Running the same command on valgrind's memcheck returns the expected result.
Replacing the latest glibc malloc with tcmalloc, also succeeds.

I'm not blaming the new behavior for this defect, but this commit is
clearly one of the requirements in order to trigger this error.

-- 
Tulio Magno

Re: [PATCH v4] malloc: Improve malloc initialization

[Tulio Magno Quites Machado Filho]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:

> Hi Florian,
>
> I believe it is the __libc_initial not set yet since ptmalloc_init() does: 
>
> #if defined SHARED && IS_IN (libc)
>   /* In case this libc copy is in a non-default namespace, never use
>      brk.  Likewise if dlopened from statically linked program.  The
>      generic sbrk implementation also enforces this, but it is not
>      used on Hurd.  */
>   if (!__libc_initial)
>     __always_fail_morecore = true;
> #endif
>
> We can easily move the initialization.

I tried the following change and llvm-objdump is now back to the
expected behavior (no segmentation fault):

diff --git i/elf/libc_early_init.c w/elf/libc_early_init.c
index 24b99d82fe..f8ad29005b 100644
--- i/elf/libc_early_init.c
+++ w/elf/libc_early_init.c
@@ -33,9 +33,6 @@ _Bool __libc_initial;
 void
 __libc_early_init (_Bool initial)
 {
-  /* Initialize system malloc.  */
-  call_function_static_weak (__ptmalloc_init);
-
   /* Initialize ctype data.  */
   __ctype_init ();
 
@@ -46,6 +43,9 @@ __libc_early_init (_Bool initial)
   __libc_single_threaded_internal = __libc_initial = initial;
 #endif
 
+  /* Initialize system malloc.  */
+  call_function_static_weak (__ptmalloc_init);
+
   __pthread_early_init ();
 
   __getrandom_early_init (initial);


This is also bringing back brk() usage.

> However malloc will always switch to mmap due to:
>
>   if (mp_.hp_pagesize > 0 && mp_.hp_pagesize <= heap_max_size ())
>     {
>       /* Force mmap for main arena instead of sbrk, so MAP_HUGETLB is always
>          tried.  Also tune the mmap threshold, so allocation smaller than the
>          large page will also try to use large pages by falling back
>          to sysmalloc_mmap_fallback on sysmalloc.  */
>       if (!TUNABLE_IS_INITIALIZED (mmap_threshold))
>         do_set_mmap_threshold (mp_.hp_pagesize);
>       __always_fail_morecore = true;
>     }
>
> So this must be a CPU without huge pages...

Keep in mind that I do see brk() being used on the same machine if I
revert 25d37948c9f.

I believe what is disabling morecore is actually this part:

  if (!__libc_initial)
    __always_fail_morecore = true;

__libc_initial is set after __ptmalloc_init() is called.

But I wonder if this is all that is needed or if there are more data
dependencies because the usage of brk() or mmap should not impact this code.

-- 
Tulio Magno

Re: [PATCH v4] malloc: Improve malloc initialization

[Florian Weimer]

* Wilco Dijkstra:

> Hi Florian,
>
> I believe it is the __libc_initial not set yet since ptmalloc_init() does: 
>
> #if defined SHARED && IS_IN (libc)
>   /* In case this libc copy is in a non-default namespace, never use
>      brk.  Likewise if dlopened from statically linked program.  The
>      generic sbrk implementation also enforces this, but it is not
>      used on Hurd.  */
>   if (!__libc_initial)
>     __always_fail_morecore = true;
> #endif

Yes, that's what I meant.

> We can easily move the initialization.
>
> However malloc will always switch to mmap due to:
>
>   if (mp_.hp_pagesize > 0 && mp_.hp_pagesize <= heap_max_size ())
>     {
>       /* Force mmap for main arena instead of sbrk, so MAP_HUGETLB is always
>          tried.  Also tune the mmap threshold, so allocation smaller than the
>          large page will also try to use large pages by falling back
>          to sysmalloc_mmap_fallback on sysmalloc.  */
>       if (!TUNABLE_IS_INITIALIZED (mmap_threshold))
>         do_set_mmap_threshold (mp_.hp_pagesize);
>       __always_fail_morecore = true;
>     }
>
> So this must be a CPU without huge pages...
>
> Is there actually an advantage in keeping the option to use sbrk() since
> there are now quite a few reasons to not use it? Many years ago it was
> slightly faster than mmap, but I'm not sure that is still true.

Clearly, the non-sbrk code path is not an option today because with the
bug above, we start using it, and things break quite horribly.

We could use the non-main-arena paths unconditionally (at a performance
cost due to heap masking/arena pointer load) and remove both the sbrk
and non-sbrk code for the main arena.

The downside is reduction of indirect branch predictor performance on
some systems because the 64 MiB mapping for the initial heap creates a
gap between the initially loaded shared objects and subsequently
dlopen'ed objects that is large enough to hamper some branch predictor
implementation.

Thanks,
Florian

Re: [PATCH v4] malloc: Improve malloc initialization

[Florian Weimer]

* Tulio Magno Quites Machado Filho:

> Konrad Kleine <konrad.kleine@posteo.de> writes:
>> #0  checkDylibCommand () at 
>> /builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:760
>> #1  0xf159206e in MachOObjectFile () at 
>> /builddir/build/BUILD/llvm-20.1.4-build/llvm-project-20.1.4.src/llvm/lib/Object/MachOObjectFile.cpp:1435
>
> It's important to keep in mind that checkDylibCommand() shouldn't have
> been called.  Somewhere in MachOObjectFile (), we take/do not take a
> branch wrongly.  I apologize for not being more specific. The debug
> information in this file is limited.

Is it possible that this happened due to an unchecked allocation error?
That would be my best guess—we run out of virtual address space for this
workload because of unsuitable behavior of the non-sbrk allocator.

Thanks,
Florian

Re: [PATCH v4] malloc: Improve malloc initialization

[Wilco Dijkstra]

Hi Florian,

> Is it possible that this happened due to an unchecked allocation error?
> That would be my best guess—we run out of virtual address space for this
> workload because of unsuitable behavior of the non-sbrk allocator.

How could it run out? It will do a 1MB mmap rather than many sbrk that
increment by 1 page, so it uses at most 1MB extra VM.

Note it could run out of the maximum number of mmaps if the kernel is
configured with a small value (IIRC the default is 65536 which is fine).

It would be interesting to force __always_fail_morecore in an older GLIBC
and repeat the failing case.

Cheers,
Wilco

Re: [PATCH v4] malloc: Improve malloc initialization

[Florian Weimer]

* Wilco Dijkstra:

> Hi Florian,
>
>> Is it possible that this happened due to an unchecked allocation error?
>> That would be my best guess—we run out of virtual address space for this
>> workload because of unsuitable behavior of the non-sbrk allocator.
>
> How could it run out? It will do a 1MB mmap rather than many sbrk that
> increment by 1 page, so it uses at most 1MB extra VM.

Fragmentation?

> It would be interesting to force __always_fail_morecore in an older GLIBC
> and repeat the failing case.

It's related to addresses returned by malloc:

  [MachO] Improve bounds check
  <https://github.com/llvm/llvm-project/pull/141083>

So it was an LLVM bug, and it would be present with sbrk disabled on
older malloc versions.

This bug appears to be different, though:

  malloc regression on i686
  <https://sourceware.org/bugzilla/show_bug.cgi?id=32996>

It still reproduces with the __libc_initial problem fixed.

Thanks,
Florian