[PATCH v2] malloc: Cleanup libc_realloc

[Wilco Dijkstra]

Hi DJ,

>    if (bytes <= usable)
>      {
>        size_t difference = usable - bytes;
> -      if ((unsigned long) difference < 2 * sizeof (INTERNAL_SIZE_T)
> -       || (chunk_is_mmapped (oldp) && difference <= GLRO (dl_pagesize)))
> +      if ((unsigned long) difference < 2 * sizeof (INTERNAL_SIZE_T))
>        return oldmem;
>      }

> Why is this an "unnecessary special case"?

Because the same check is done in mremap_chunk already. Basically it does
not make sense to have several special cases for mmap when there is already
a big if statement that handles everything mmap related.

Note the 2 * sizeof (INTERNAL_SIZE_T) is not quite right due to the minimum
chunk size being MINSIZE, but I'll leave that for another time...

> -      /* Note the extra SIZE_SZ overhead. */
> -      if (oldsize - SIZE_SZ >= nb)
> -        return oldmem;                         /* do nothing */
> -

> I think this only applies to non-mmap'd chunks, so ok.

This executes when mremap doesn't exist or fails to work. I think we may
need this for now, but I'll change it in v2 to avoid relying on internal details of
mmap layout.

Cheers,
Wilco


v2: Rebased, change check for shrinking after mremap

Minor cleanup of libc_realloc: remove unnecessary special cases for mmap, move
ar_ptr initialization, first check for oldmem == NULL.

---

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 9d646abae9a74106c81e3b78dd3ebd4cdcdd4808..e9c1a9224234d1bcae3f92042cf0cc0e350b4d1e 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3504,17 +3504,17 @@ __libc_realloc (void *oldmem, size_t bytes)
 
   void *newp;             /* chunk to return */
 
+  /* realloc of null is supposed to be same as malloc */
+  if (oldmem == NULL)
+    return __libc_malloc (bytes);
+
 #if REALLOC_ZERO_BYTES_FREES
-  if (bytes == 0 && oldmem != NULL)
+  if (bytes == 0)
     {
       __libc_free (oldmem); return NULL;
     }
 #endif
 
-  /* realloc of null is supposed to be same as malloc */
-  if (oldmem == NULL)
-    return __libc_malloc (bytes);
-
   /* Perform a quick check to ensure that the pointer's tag matches the
      memory's tag.  */
   if (__glibc_unlikely (mtag_enabled))
@@ -3532,19 +3532,13 @@ __libc_realloc (void *oldmem, size_t bytes)
   if (bytes <= usable)
     {
       size_t difference = usable - bytes;
-      if ((unsigned long) difference < 2 * sizeof (INTERNAL_SIZE_T)
-	  || (chunk_is_mmapped (oldp) && difference <= GLRO (dl_pagesize)))
+      if ((unsigned long) difference < 2 * sizeof (INTERNAL_SIZE_T))
 	return oldmem;
     }
 
   /* its size */
   const INTERNAL_SIZE_T oldsize = chunksize (oldp);
 
-  if (chunk_is_mmapped (oldp))
-    ar_ptr = NULL;
-  else
-    ar_ptr = arena_for_chunk (oldp);
-
   /* Little security check which won't hurt performance: the allocator
      never wraps around at the end of the address space.  Therefore
      we can exclude some size values which might appear here by
@@ -3577,9 +3571,9 @@ __libc_realloc (void *oldmem, size_t bytes)
 	  return tag_new_usable (newmem);
 	}
 #endif
-      /* Note the extra SIZE_SZ overhead. */
-      if (oldsize - SIZE_SZ >= nb)
-        return oldmem;                         /* do nothing */
+      /* Return if shrinking and mremap was unsuccessful.  */
+      if (bytes <= usable)
+	return oldmem;
 
       /* Must alloc, copy, free. */
       newmem = __libc_malloc (bytes);
@@ -3591,6 +3585,8 @@ __libc_realloc (void *oldmem, size_t bytes)
       return newmem;
     }
 
+  ar_ptr = arena_for_chunk (oldp);
+
   if (SINGLE_THREAD_P)
     {
       newp = _int_realloc (ar_ptr, oldp, oldsize, nb);