[PATCH] malloc: replace instances of __builtin_expect with __glibc_unlikely in malloc.c
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Tue Jun 24 20:15:35 GMT 2025
On 24/06/25 17:11, William Hunt wrote:
> On 2025-06-24 20:45, Adhemerval Zanella Netto wrote:
>>
>>
>> On 24/06/25 14:10, William Hunt wrote:
>>> Replaced all instances of __builtin_expect within malloc.c
>>> to __glibc_unlikely. This improves the portability of glibc
>>> by avoiding calls to GNU C built-in functions. Since all
>>> the expected results from calls to __builtin_expect were 0,
>>> __glibc_likely was never used as a replacement. Multiple
>>> calls to __builtin_expect within a single if statement have
>>> been replaced with one call to __glibc_unlikely, which wraps
>>> every condition.
>>>
>>> Passes regress, OK for commit?
>>
>> LGTM, thanks. There are still some __builtin_expect usage on malloc/malloc-debug.c.
>>
>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>
> I specified the patch as specifically for malloc.c,
> should I update malloc-debug.c as well in this patch
> or make a seperate patch to cover the malloc/ branch?
It is up to you, this patch looks fine as is.
>>> ---
>>> malloc/malloc.c | 41 ++++++++++++++++++++---------------------
>>> 1 file changed, 20 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/malloc/malloc.c b/malloc/malloc.c
>>> index d28cd66faa..e73085af42 100644
>>> --- a/malloc/malloc.c
>>> +++ b/malloc/malloc.c
>>> @@ -1623,7 +1623,7 @@ unlink_chunk (mstate av, mchunkptr p)
>>> mchunkptr fd = p->fd;
>>> mchunkptr bk = p->bk;
>>>
>>> - if (__builtin_expect (fd->bk != p || bk->fd != p, 0))
>>> + if (__glibc_unlikely (fd->bk != p || bk->fd != p))
>>> malloc_printerr ("corrupted double-linked list");
>>>
>>> fd->bk = bk;
>>> @@ -3630,8 +3630,8 @@ __libc_realloc (void *oldmem, size_t bytes)
>>> never wraps around at the end of the address space. Therefore
>>> we can exclude some size values which might appear here by
>>> accident or by "design" from some intruder. */
>>> - if ((__builtin_expect ((uintptr_t) oldp > (uintptr_t) -oldsize, 0)
>>> - || __builtin_expect (misaligned_chunk (oldp), 0)))
>>> + if (__glibc_unlikely ((uintptr_t) oldp > (uintptr_t) -oldsize
>>> + || misaligned_chunk (oldp)))
>>> malloc_printerr ("realloc(): invalid pointer");
>>>
>>> nb = checked_request2size (bytes);
>>> @@ -3901,7 +3901,7 @@ __libc_calloc2 (size_t sz)
>>> /* Two optional cases in which clearing not necessary */
>>> if (chunk_is_mmapped (p))
>>> {
>>> - if (__builtin_expect (perturb_byte, 0))
>>> + if (__glibc_unlikely (perturb_byte))
>>> return memset (mem, 0, sz);
>>>
>>> return mem;
>>> @@ -4066,7 +4066,7 @@ _int_malloc (mstate av, size_t bytes)
>>> if (__glibc_likely (victim != NULL))
>>> {
>>> size_t victim_idx = fastbin_index (chunksize (victim));
>>> - if (__builtin_expect (victim_idx != idx, 0))
>>> + if (__glibc_unlikely (victim_idx != idx))
>>> malloc_printerr ("malloc(): memory corruption (fast)");
>>> check_remalloced_chunk (av, victim, nb);
>>> #if USE_TCACHE
>>> @@ -4655,10 +4655,9 @@ _int_free_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T size, int have_lock)
>>> #endif
>>> ) {
>>>
>>> - if (__builtin_expect (chunksize_nomask (chunk_at_offset (p, size))
>>> - <= CHUNK_HDR_SZ, 0)
>>> - || __builtin_expect (chunksize (chunk_at_offset (p, size))
>>> - >= av->system_mem, 0))
>>> + if (__glibc_unlikely (
>>> + chunksize_nomask (chunk_at_offset(p, size)) <= CHUNK_HDR_SZ
>>> + || chunksize (chunk_at_offset(p, size)) >= av->system_mem))
>>> {
>>> bool fail = true;
>>> /* We might not have a lock at this point and concurrent modifications
>>> @@ -4689,7 +4688,7 @@ _int_free_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T size, int have_lock)
>>> {
>>> /* Check that the top of the bin is not the record we are going to
>>> add (i.e., double free). */
>>> - if (__builtin_expect (old == p, 0))
>>> + if (__glibc_unlikely (old == p))
>>> malloc_printerr ("double free or corruption (fasttop)");
>>> p->fd = PROTECT_PTR (&p->fd, old);
>>> *fb = p;
>>> @@ -4699,7 +4698,7 @@ _int_free_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T size, int have_lock)
>>> {
>>> /* Check that the top of the bin is not the record we are going to
>>> add (i.e., double free). */
>>> - if (__builtin_expect (old == p, 0))
>>> + if (__glibc_unlikely (old == p))
>>> malloc_printerr ("double free or corruption (fasttop)");
>>> old2 = old;
>>> p->fd = PROTECT_PTR (&p->fd, old);
>>> @@ -4712,7 +4711,7 @@ _int_free_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T size, int have_lock)
>>> only if we have the lock, otherwise it might have already been
>>> allocated again. */
>>> if (have_lock && old != NULL
>>> - && __builtin_expect (fastbin_index (chunksize (old)) != idx, 0))
>>> + && __glibc_unlikely (fastbin_index (chunksize (old)) != idx))
>>> malloc_printerr ("invalid fastbin entry (free)");
>>> }
>>>
>>> @@ -4779,17 +4778,17 @@ _int_free_merge_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T size)
>>> if (__glibc_unlikely (p == av->top))
>>> malloc_printerr ("double free or corruption (top)");
>>> /* Or whether the next chunk is beyond the boundaries of the arena. */
>>> - if (__builtin_expect (contiguous (av)
>>> + if (__glibc_unlikely (contiguous (av)
>>> && (char *) nextchunk
>>> - >= ((char *) av->top + chunksize(av->top)), 0))
>>> + >= ((char *) av->top + chunksize(av->top))))
>>> malloc_printerr ("double free or corruption (out)");
>>> /* Or whether the block is actually not marked used. */
>>> if (__glibc_unlikely (!prev_inuse(nextchunk)))
>>> malloc_printerr ("double free or corruption (!prev)");
>>>
>>> INTERNAL_SIZE_T nextsize = chunksize(nextchunk);
>>> - if (__builtin_expect (chunksize_nomask (nextchunk) <= CHUNK_HDR_SZ, 0)
>>> - || __builtin_expect (nextsize >= av->system_mem, 0))
>>> + if (__glibc_unlikely (chunksize_nomask (nextchunk) <= CHUNK_HDR_SZ
>>> + || nextsize >= av->system_mem))
>>> malloc_printerr ("free(): invalid next size (normal)");
>>>
>>> free_perturb (chunk2mem(p), size - CHUNK_HDR_SZ);
>>> @@ -5046,9 +5045,9 @@ _int_realloc (mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
>>> unsigned long remainder_size; /* its size */
>>>
>>> /* oldmem size */
>>> - if (__builtin_expect (chunksize_nomask (oldp) <= CHUNK_HDR_SZ, 0)
>>> - || __builtin_expect (oldsize >= av->system_mem, 0)
>>> - || __builtin_expect (oldsize != chunksize (oldp), 0))
>>> + if (__glibc_unlikely (chunksize_nomask (oldp) <= CHUNK_HDR_SZ
>>> + || oldsize >= av->system_mem
>>> + || oldsize != chunksize (oldp)))
>>> malloc_printerr ("realloc(): invalid old size");
>>>
>>> check_inuse_chunk (av, oldp);
>>> @@ -5058,8 +5057,8 @@ _int_realloc (mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
>>>
>>> next = chunk_at_offset (oldp, oldsize);
>>> INTERNAL_SIZE_T nextsize = chunksize (next);
>>> - if (__builtin_expect (chunksize_nomask (next) <= CHUNK_HDR_SZ, 0)
>>> - || __builtin_expect (nextsize >= av->system_mem, 0))
>>> + if (__glibc_unlikely (chunksize_nomask (next) <= CHUNK_HDR_SZ
>>> + || nextsize >= av->system_mem))
>>> malloc_printerr ("realloc(): invalid next size");
>>>
>>> if ((unsigned long) (oldsize) >= (unsigned long) (nb))
>>
More information about the Libc-alpha
mailing list