[PATCH 2/2] malloc: introduce ifuncs for malloc functions

[Yury Khrustalev]

Thanks for the comments, I'll wait a bit for more feedback and then send
v2 with the fixes.

On Fri, Apr 17, 2026 at 04:37:43PM -0400, DJ Delorie wrote:
> 
> Yury Khrustalev <yury.khrustalev@arm.com> writes:
> > Introduce ifuncs and resolvers for functions pertinent to the
> > malloc interface.
> >
> > In order to do this, we first rename all core implementations by
> > adding the '_core' suffix. These functions are supposed to be strictly
> 
> Not wanting to bikeshed, but don't we have a history of using _generic
> for this purpose?  Or is it your intention that all specific variants
> will always call the "core" variants (vs replacing them), and thus a
> different suffix is warranted?

I'm happy to change the suffix here, however it's not just a "generic"
implementation. It represents "core" functionality of Glibc's malloc.
A generic (i.e. default-for-all-targets-that-don't-have-their-own-way)
implementation just happens to be the same as the "core" one, and some
targets may choose to use the "core" functions as part of their own
implementation.

So, yes, like you say, I think a different suffix should be used here.

> ... 
> 
> > diff i--git a/malloc/malloc.c b/malloc/malloc.c
> > index 57b58382b1..215feeebb0 100644
> > --- a/malloc/malloc.c
> > +++ b/malloc/malloc.c
> > @@ -216,8 +216,6 @@
> >  #include <assert.h>
> >  #include <intprops.h>
> >  
> > -#include <shlib-compat.h>
> 
> SHLIB_COMPAT is still used though, in the !HAVE_IFUNC case.

True, I'll fix this in v2.

> ...
>
> > -void     __libc_free(void*);
> > -libc_hidden_proto (__libc_free)
> > +void __libc_free_core (void *);
> > +libc_hidden_proto (__libc_free_core)
> 
> Arbitrary whitespace changes, but new way is better.  Ok.

Yeah, I thought I'd fix code style while I'm at it.

> ...
>
> > -void*  __libc_valloc(size_t);
> > -
> > +void *__libc_valloc_core (size_t);
> > +libc_hidden_proto (__libc_valloc_core)
> 
> Ok.
> 
> Did we check that the pusblished (non-hidden) symbol list is the same
> before and after?

If I understand this bit correctly, there are tests that check public
symbols, and I've hit a few failures while working on this patch and
fixed them.

Also, the public symbols related to valloc() are done via aliases that
didn't change. Strictly speaking, we only need libc_hidden_{def,proto}
here to avoid PLT-based call when local call is enough.

> ...
>
> > @@ -3142,7 +3143,7 @@ tcache_double_free_verify (tcache_entry *e)
> >       or user data that happens to match the key.  Since we are not sure,
> >       clear the key and retry freeing it.  */
> >    e->key = 0;
> > -  __libc_free (e);
> > +  __libc_free_core (e);
> >  }
> 
> This is only called from within __libc_free_core itself anyway, and only
> for the same block, so any ifunc wrapper would have already had its
> chance at this chunk.  Thus, not calling the ifunc wrapper again seems
> correct to me.

Is this "OK" or "Not OK"? :)

This is the fragile part of any abstractions around malloc (ifunc-based or
otherwise). It is important that whenever an internal function needs to
used __libc_foo() it uses __libc_foo_core(). If it uses a non-_core symbol,
the returned result may not be suitable for subsequent use of it internally.

Non-_core functions return and accept user-pointers which are different from
internal pointers that are used by the _core function. Unfortunately, at
this stage there is no semantic way to differentiate those, and this part
requires further refactoring in malloc.

Off-topic, but I think that all internal _core functions should operate on
something like

  struct internal_ptr_t {
      void *ptr;
  }

instead of 'void *'. But I digress. The non-_core functions should not be
used in malloc.c. Perhaps, we need some grep-based test here.

> ...
>
> >    /* realloc of null is supposed to be same as malloc */
> >    if (oldmem == NULL)
> > -    return __libc_malloc (bytes);
> > +    return __libc_malloc_core (bytes);
> 
> Ok.  We continue to assume that any wrapper will have its chance at the
> interface between internal stuff and the user's program.
> 
> I wonder, though... we current do tagging operations inside malloc
> internals, like splitting chunks.  How will the ifunc interface handle
> these?

That's left for the next patch series, stay tuned! Core parts of malloc
should not be aware of any tagging or whatnot. I'm working on it now.

Having ifuncs is essential step to facilitate this work.

> ...
>
> > diff --git a/sysdeps/aarch64/malloc-ifuncs.c b/sysdeps/aarch64/malloc-ifuncs.c
> 
> ...
> 
> > +#if IS_IN (libc)
> 
> Is there ever a case where we're not in libc?

I think not, but I might be missing something here. FWIW, the 'malloc' and
'__malloc' (now gone) aliases were only declared under this macro, so I
think the same should be done for ifunc resolvers that now back these
aliases.

> 
> > +#include <malloc/malloc-internal.h>
> > +#include <malloc-ifuncs.h>
> > +
> > +/* AArch64-specific resolvers for malloc ifuncs.  */
> > +
> > +IFUNC_PROTO (__libc_malloc);
> > +IFUNC_RESOLVER (__libc_malloc, arg0, arg1)
> 
> It would be nice if there were some hint as to what these arguments are,
> or are used for.  Ifuncs are complicated enough without obfuscating this
> information.

That's why I kept them as part of macro arguments. Perhaps, we should make
it even more explicit. I'll think about it. Maybe at least argument types
should be here too. As for the names, I think they have to be pretty generic.

> ...
> 
> > diff --git a/sysdeps/generic/malloc-ifuncs.c b/sysdeps/generic/malloc-ifuncs.c
>
> ...
>
> > +
> > +# if HAVE_IFUNC
> > +
> > +/* These resolvers are used by default unless overridden by a target.
> > +   The target-specific resolvers must respect this logic if the default
> > +   resolvers replicating it where appropriate.
> 
> Grammar?  "if..where" sounds like something is missing.

Should be: of the default resolvers, replicating it where appropriate.
Will fix.

> 
> > +   Any aliases for mallo API functions must be defined here as well
> 
> typo "mallo"

Will fix.

Cheers,
Yury