[PATCH] malloc.c, arena.c: Use ALIGN_UP, powerof2, and pagesize consistently.

["Carlos O'Donell" <carlos at redhat dot com>]

I was auditing malloc.c and arena.c for reetrancy issues and
decided to do a cleanup as I read the code.

The malloc implementation has instance of the ALIGN_UP pattern
that would be made clearer by using the so-named libc-internal.h
macro.

Even more interesting is that the micro-optimization of
precomputing `pagesize - 1` in an attempt to make subsequent
ALIGN_UP operations fast results in larger code than just simply
using the ALIGN_UP macro with the pagesize variable (gcc 4.8).

e.g.
---
<new_heap>:
...
+       48 01 fe                add    %rdi,%rsi
+       48 81 fe ff 7f 00 00    cmp    $0x7fff,%rsi
        55                      push   %rbp
-                               lea    (%rdi,%rsi,1),%rbp
        48 8b 40 18             mov    0x?(%rax),%rax
        53                      push   %rbx
-       48 83 e8 01             sub    $0x1,%rax
-       48 81 fd ff 7f 00 00    cmp    $0x7fff,%rbp
...
        48 8b 3d 00 00 00 00    mov    0x?(%rip),%rdi        #? <new_heap+0x?>

-       48 01 c5                add    %rax,%rbp
-       48 f7 d0                not    %rax
+                               lea    -0x?(%rsi,%rax,1),%rbp
+       48 f7 d8                neg    %rax
        48 21 c5                and    %rax,%rbp
        48 85 ff                test   %rdi,%rdi
                                je     ? <new_heap+0x?>
...
        5d                      pop    %rbp
        41 5c                   pop    %r12
        c3                      retq
-       0f 1f 44 00 00          nopl   0x?(%rax,%rax,1)
...
-       66 66 66 2e 0f 1f 84    data32 data32 nopw %cs:0x?(%rax,%rax,1)
-       00 00 00 00 00 
+       0f 1f 40 00             nopl   0x?(%rax)
---

In addition we cleanup the use of page_mask, pagemask, pagesz, and
pagesize and settle on a readable and common variable "pagesize."
In only one instance do we leave page_mask since it makes sense for
that particular case (hooks.c:mem2chunk_check).

Lastly we use powerof2 where appropriate.

Tested on x86_64. Verified no regressions.

OK to checkin?

2014-12-30  Carlos O'Donell  <carlos@redhat.com>

	* arena.c (new_heap): Use pagesize. Call ALIGN_UP.
	(grow_heap): Likewise.
	* malloc.c: Include libc-internal.h.
	(do_check_malloc): Call powerof2.
	(sysmalloc): Use pagesize. Call ALIGN_UP.
	(systrim): Use pagesize.
	(mremap_chunk): Use pagesize. Call ALIGN_UP.
	(__libc_valloc): Use pagesize.
	(__libc_pvalloc): Use pagesize. Call ALIGN_UP.	

diff --git a/malloc/arena.c b/malloc/arena.c
index 60ae9a4..f0879f2 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -510,7 +510,7 @@ static heap_info *
 internal_function
 new_heap (size_t size, size_t top_pad)
 {
-  size_t page_mask = GLRO (dl_pagesize) - 1;
+  size_t pagesize = GLRO (dl_pagesize);
   char *p1, *p2;
   unsigned long ul;
   heap_info *h;
@@ -523,7 +523,7 @@ new_heap (size_t size, size_t top_pad)
     return 0;
   else
     size = HEAP_MAX_SIZE;
-  size = (size + page_mask) & ~page_mask;
+  size = ALIGN_UP (size, pagesize);
 
   /* A memory region aligned to a multiple of HEAP_MAX_SIZE is needed.
      No swap space needs to be reserved for the following large
@@ -588,10 +588,10 @@ new_heap (size_t size, size_t top_pad)
 static int
 grow_heap (heap_info *h, long diff)
 {
-  size_t page_mask = GLRO (dl_pagesize) - 1;
+  size_t pagesize = GLRO (dl_pagesize);
   long new_size;
 
-  diff = (diff + page_mask) & ~page_mask;
+  diff = ALIGN_UP (diff, pagesize);
   new_size = (long) h->size + diff;
   if ((unsigned long) new_size > (unsigned long) HEAP_MAX_SIZE)
     return -1;
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 875fe2e..bc8f5da 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -241,6 +241,9 @@
 /* For MIN, MAX, powerof2.  */
 #include <sys/param.h>
 
+/* For ALIGN_UP.  */
+#include <libc-internal.h>
+
 
 /*
   Debugging:
@@ -2111,7 +2114,7 @@ do_check_malloc_state (mstate av)
     return;
 
   /* pagesize is a power of 2 */
-  assert ((GLRO (dl_pagesize) & (GLRO (dl_pagesize) - 1)) == 0);
+  assert (powerof2(GLRO (dl_pagesize)));
 
   /* A contiguous main_arena is consistent with sbrk_base.  */
   if (av == &main_arena && contiguous (av))
@@ -2266,7 +2269,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
   unsigned long remainder_size;   /* its size */
 
 
-  size_t pagemask = GLRO (dl_pagesize) - 1;
+  size_t pagesize = GLRO (dl_pagesize);
   bool tried_mmap = false;
 
 
@@ -2292,9 +2295,9 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
          need for further alignments unless we have have high alignment.
        */
       if (MALLOC_ALIGNMENT == 2 * SIZE_SZ)
-        size = (nb + SIZE_SZ + pagemask) & ~pagemask;
+        size = ALIGN_UP (nb + SIZE_SZ, pagesize);
       else
-        size = (nb + SIZE_SZ + MALLOC_ALIGN_MASK + pagemask) & ~pagemask;
+        size = ALIGN_UP (nb + SIZE_SZ + MALLOC_ALIGN_MASK, pagesize);
       tried_mmap = true;
 
       /* Don't try if size wraps around 0 */
@@ -2367,7 +2370,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
   assert ((old_top == initial_top (av) && old_size == 0) ||
           ((unsigned long) (old_size) >= MINSIZE &&
            prev_inuse (old_top) &&
-           ((unsigned long) old_end & pagemask) == 0));
+           ((unsigned long) old_end & (pagesize - 1)) == 0));
 
   /* Precondition: not enough current space to satisfy nb request */
   assert ((unsigned long) (old_size) < (unsigned long) (nb + MINSIZE));
@@ -2447,7 +2450,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
          previous calls. Otherwise, we correct to page-align below.
        */
 
-      size = (size + pagemask) & ~pagemask;
+      size = ALIGN_UP (size, pagesize);
 
       /*
          Don't try to call MORECORE if argument is so big as to appear
@@ -2481,7 +2484,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
 
           /* Cannot merge with old top, so add its size back in */
           if (contiguous (av))
-            size = (size + old_size + pagemask) & ~pagemask;
+            size = ALIGN_UP (size + old_size, pagesize);
 
           /* If we are relying on mmap as backup, then use larger units */
           if ((unsigned long) (size) < (unsigned long) (MMAP_AS_MORECORE_SIZE))
@@ -2587,7 +2590,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
 
                   /* Extend the end address to hit a page boundary */
                   end_misalign = (INTERNAL_SIZE_T) (brk + size + correction);
-                  correction += ((end_misalign + pagemask) & ~pagemask) - end_misalign;
+                  correction += (ALIGN_UP (end_misalign, pagesize)) - end_misalign;
 
                   assert (correction >= 0);
                   snd_brk = (char *) (MORECORE (correction));
@@ -2738,10 +2741,10 @@ systrim (size_t pad, mstate av)
   long released;         /* Amount actually released */
   char *current_brk;     /* address returned by pre-check sbrk call */
   char *new_brk;         /* address returned by post-check sbrk call */
-  size_t pagesz;
+  size_t pagesize;
   long top_area;
 
-  pagesz = GLRO (dl_pagesize);
+  pagesize = GLRO (dl_pagesize);
   top_size = chunksize (av->top);
 
   top_area = top_size - MINSIZE - 1;
@@ -2749,7 +2752,7 @@ systrim (size_t pad, mstate av)
     return 0;
 
   /* Release in pagesize units, keeping at least one page */
-  extra = (top_area - pad) & ~(pagesz - 1);
+  extra = (top_area - pad) & ~(pagesize - 1);
 
   if (extra == 0)
     return 0;
@@ -2834,7 +2837,7 @@ static mchunkptr
 internal_function
 mremap_chunk (mchunkptr p, size_t new_size)
 {
-  size_t page_mask = GLRO (dl_pagesize) - 1;
+  size_t pagesize = GLRO (dl_pagesize);
   INTERNAL_SIZE_T offset = p->prev_size;
   INTERNAL_SIZE_T size = chunksize (p);
   char *cp;
@@ -2843,7 +2846,7 @@ mremap_chunk (mchunkptr p, size_t new_size)
   assert (((size + offset) & (GLRO (dl_pagesize) - 1)) == 0);
 
   /* Note the extra SIZE_SZ overhead as in mmap_chunk(). */
-  new_size = (new_size + offset + SIZE_SZ + page_mask) & ~page_mask;
+  new_size = ALIGN_UP (new_size + offset + SIZE_SZ, pagesize);
 
   /* No need to remap if the number of pages does not change.  */
   if (size + offset == new_size)
@@ -3122,8 +3125,8 @@ __libc_valloc (size_t bytes)
     ptmalloc_init ();
 
   void *address = RETURN_ADDRESS (0);
-  size_t pagesz = GLRO (dl_pagesize);
-  return _mid_memalign (pagesz, bytes, address);
+  size_t pagesize = GLRO (dl_pagesize);
+  return _mid_memalign (pagesize, bytes, address);
 }
 
 void *
@@ -3133,18 +3136,17 @@ __libc_pvalloc (size_t bytes)
     ptmalloc_init ();
 
   void *address = RETURN_ADDRESS (0);
-  size_t pagesz = GLRO (dl_pagesize);
-  size_t page_mask = GLRO (dl_pagesize) - 1;
-  size_t rounded_bytes = (bytes + page_mask) & ~(page_mask);
+  size_t pagesize = GLRO (dl_pagesize);
+  size_t rounded_bytes = ALIGN_UP (bytes, pagesize);
 
   /* Check for overflow.  */
-  if (bytes > SIZE_MAX - 2 * pagesz - MINSIZE)
+  if (bytes > SIZE_MAX - 2 * pagesize - MINSIZE)
     {
       __set_errno (ENOMEM);
       return 0;
     }
 
-  return _mid_memalign (pagesz, rounded_bytes, address);
+  return _mid_memalign (pagesize, rounded_bytes, address);
 }
 
 void *
---

Cheers,
Carlos.