[PATCH v4] malloc: Improve memalign alignment

[Wilco Dijkstra]

v4: Improve comments, address review feedback

Use generic stdc_bit_width to safely adapt to input types. Move rounding up of
alignments that are not powers of 2 to __libc_memalign.  Simplify alignment handling
of aligned_alloc and __posix_memalign. Add a testcase for non-power of 2 memalign
and fix malloc-debug.

Passes regress, OK for commit?

---

diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
index 269003b4eff28cf74be54e62a433a06fd6a280b5..c69890a24cc76e772ef5ecb82c44a4c7cb256673 100644
--- a/malloc/malloc-debug.c
+++ b/malloc/malloc-debug.c
@@ -269,6 +269,15 @@ _debug_mid_memalign (size_t alignment, size_t bytes, const void *address)
 static void *
 __debug_memalign (size_t alignment, size_t bytes)
 {
+  /* Round the alignment up to a power of 2.  Reject alignments that overflow
+     when rounded up.  Zero alignment is accepted and handled later.  */
+  if (alignment > SIZE_MAX / 2 + 1)
+    {
+      errno = EINVAL;
+      return NULL;
+    }
+
+  alignment = stdc_bit_ceil (alignment);
   return _debug_mid_memalign (alignment, bytes, RETURN_ADDRESS (0));
 }
 strong_alias (__debug_memalign, memalign)
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 710887f4da20ad03a10a8281e707af7d82c3b9d7..bd263bc9b6350c8885d85afdbad8a3917431fc40 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -2993,10 +2993,8 @@ tcache_key_initialize (void)
 static __always_inline size_t
 large_csize2tidx(size_t nb)
 {
-  size_t idx = TCACHE_SMALL_BINS
-	       + __builtin_clz (MAX_TCACHE_SMALL_SIZE)
-	       - __builtin_clz (nb);
-  return idx;
+  size_t idx = stdc_bit_width (nb) - stdc_bit_width (MAX_TCACHE_SMALL_SIZE);
+  return idx + TCACHE_SMALL_BINS;
 }
 
 /* Caller must ensure that we know tc_idx is valid and there's room
@@ -3512,6 +3510,19 @@ libc_hidden_def (__libc_realloc)
 void *
 __libc_memalign (size_t alignment, size_t bytes)
 {
+  /* Round the alignment up to a power of 2.  Reject alignments that overflow
+     when rounded up.  Zero alignment is handled by _mid_memalign.  */
+  if (__glibc_unlikely (!powerof2 (alignment)))
+    {
+      if (alignment > SIZE_MAX / 2 + 1)
+	{
+	  __set_errno (EINVAL);
+	  return NULL;
+	}
+
+      alignment = stdc_bit_ceil (alignment);
+    }
+
   return _mid_memalign (alignment, bytes);
 }
 libc_hidden_def (__libc_memalign)
@@ -3521,11 +3532,9 @@ void *
 weak_function
 aligned_alloc (size_t alignment, size_t bytes)
 {
-/* 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
-   are non-negative powers of two.  */
-  if (!powerof2 (alignment) || alignment == 0)
+/* Starting with ISO C17 the standard requires an error for alignments
+   that are not supported.  Only integral powers of 2 are valid.  */
+  if (!stdc_has_single_bit (alignment))
     {
       __set_errno (EINVAL);
       return NULL;
@@ -3571,28 +3580,6 @@ _mid_memalign (size_t alignment, size_t bytes)
   if (alignment <= MALLOC_ALIGNMENT)
     return __libc_malloc (bytes);
 
-  /* Otherwise, ensure that it is at least a minimum chunk size */
-  if (alignment < MINSIZE)
-    alignment = MINSIZE;
-
-  /* If the alignment is greater than SIZE_MAX / 2 + 1 it cannot be a
-     power of 2 and will cause overflow in the check below.  */
-  if (alignment > SIZE_MAX / 2 + 1)
-    {
-      __set_errno (EINVAL);
-      return NULL;
-    }
-
-
-  /* Make sure alignment is power of 2.  */
-  if (!powerof2 (alignment))
-    {
-      size_t a = MALLOC_ALIGNMENT * 2;
-      while (a < alignment)
-        a <<= 1;
-      alignment = a;
-    }
-
 #if USE_TCACHE
   void *victim = tcache_get_align (checked_request2size (bytes), alignment);
   if (victim != NULL)
@@ -5307,25 +5294,18 @@ malloc_printerr_tail (const char *str)
 int
 __posix_memalign (void **memptr, size_t alignment, size_t size)
 {
-  void *mem;
-
   /* Test whether the SIZE argument is valid.  It must be a power of
-     two multiple of sizeof (void *).  */
-  if (alignment % sizeof (void *) != 0
-      || !powerof2 (alignment / sizeof (void *))
-      || alignment == 0)
+     two multiple of sizeof (void *) (which must be either 4 or 8).  */
+  if (alignment < sizeof (void *) || !powerof2 (alignment))
     return EINVAL;
 
+  void *mem = _mid_memalign (alignment, size);
 
-  mem = _mid_memalign (alignment, size);
-
-  if (mem != NULL)
-    {
-      *memptr = mem;
-      return 0;
-    }
+  if (mem == NULL)
+    return ENOMEM;
 
-  return ENOMEM;
+  *memptr = mem;
+  return 0;
 }
 weak_alias (__posix_memalign, posix_memalign)
 #endif
diff --git a/malloc/tst-memalign.c b/malloc/tst-memalign.c
index 771dd9c62eb708a8f3bd5c508794abbf9aa2a54d..82b8e7b8725c78e9db84844b0d33431118844ef9 100644
--- a/malloc/tst-memalign.c
+++ b/malloc/tst-memalign.c
@@ -119,6 +119,19 @@ do_test (void)
 
   free (p);
 
+  /* Check the alignment is rounded up to a power of 2.  */
+  p = memalign (257, 10);
+
+  if (p == NULL)
+    merror ("memalign (257, 10) failed.");
+
+  ptrval = (unsigned long) p;
+
+  if ((ptrval & 0x1ff) != 0)
+    merror ("pointer is not aligned to 0x200");
+
+  free (p);
+
   return errors != 0;
 }