[v5] aligned_alloc: conform to C17

Message ID xncz3iftgp.fsf@greed.delorie.com
State Committed
Commit d1417176a35d27ffb8da0ffb1e33154163b6eeb2
Headers
Series [v5] aligned_alloc: conform to C17 |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

DJ Delorie May 2, 2023, 7:01 p.m. UTC
  Changes as requested...

From a2c1315544c72dedfcbd94b2efa7afb3eb1f0a02 Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Tue, 21 Mar 2023 00:46:43 -0400
Subject: aligned_alloc: conform to C17

This patch adds the strict checking for power-of-two alignments
in aligned_alloc(), and updates the manual accordingly.
  

Comments

Carlos O'Donell May 8, 2023, 2:20 p.m. UTC | #1
On 5/2/23 15:01, DJ Delorie wrote:
> 
> Changes as requested...
> 

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> From a2c1315544c72dedfcbd94b2efa7afb3eb1f0a02 Mon Sep 17 00:00:00 2001
> From: DJ Delorie <dj@redhat.com>
> Date: Tue, 21 Mar 2023 00:46:43 -0400
> Subject: aligned_alloc: conform to C17
> 
> This patch adds the strict checking for power-of-two alignments
> in aligned_alloc(), and updates the manual accordingly.
> 
> diff --git a/malloc/Makefile b/malloc/Makefile
> index f4cae2538c..57a9677512 100644
> --- a/malloc/Makefile
> +++ b/malloc/Makefile
> @@ -44,11 +44,13 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
>  	 tst-safe-linking \
>  	 tst-mallocalign1 \
>  	 tst-memalign-2 \
> -	 tst-memalign-3
> +	 tst-memalign-3 \
> +	 tst-aligned-alloc

OK.

>  
>  tests-static := \
>  	 tst-interpose-static-nothread \
> -	 tst-interpose-static-thread
> +	 tst-interpose-static-thread \
> +	 tst-aligned-alloc-static

OK.

>  
>  # Test for the malloc_set_state symbol removed in glibc 2.25.
>  ifeq ($(have-GLIBC_2.23)$(build-shared),yesyes)
> diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
> index 3867d15698..da9d2340d3 100644
> --- a/malloc/malloc-debug.c
> +++ b/malloc/malloc-debug.c
> @@ -299,7 +299,14 @@ __debug_memalign (size_t alignment, size_t bytes)
>    return _debug_mid_memalign (alignment, bytes, RETURN_ADDRESS (0));
>  }
>  strong_alias (__debug_memalign, memalign)
> -strong_alias (__debug_memalign, aligned_alloc)
> +static void *
> +__debug_aligned_alloc (size_t alignment, size_t bytes)
> +{
> +  if (!powerof2 (alignment) || alignment == 0)
> +    return NULL;
> +  return _debug_mid_memalign (alignment, bytes, RETURN_ADDRESS (0));
> +}
> +strong_alias (__debug_aligned_alloc, aligned_alloc)

OK.

>  
>  static void *
>  __debug_pvalloc (size_t bytes)
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index e33ed665db..5d8b61d66c 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3528,6 +3528,29 @@ __libc_memalign (size_t alignment, size_t bytes)
>    void *address = RETURN_ADDRESS (0);
>    return _mid_memalign (alignment, bytes, address);
>  }
> +libc_hidden_def (__libc_memalign)
> +
> +/* For ISO C17.  */

OK. Corrected.

> +void *
> +weak_function
> +aligned_alloc (size_t alignment, size_t bytes)
> +{
> +  if (!__malloc_initialized)
> +    ptmalloc_init ();
> +
> +/* 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.  */

OK.

> +  if (!powerof2 (alignment) || alignment == 0)
> +    {
> +      __set_errno (EINVAL);
> +      return 0;
> +    }
> +
> +  void *address = RETURN_ADDRESS (0);
> +  return _mid_memalign (alignment, bytes, address);
> +}
>  
>  static void *
>  _mid_memalign (size_t alignment, size_t bytes, void *address)
> @@ -3618,9 +3641,6 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
>            ar_ptr == arena_for_chunk (mem2chunk (p)));
>    return tag_new_usable (p);
>  }
> -/* For ISO C11.  */
> -weak_alias (__libc_memalign, aligned_alloc)
> -libc_hidden_def (__libc_memalign)
>  
>  void *
>  __libc_valloc (size_t bytes)
> diff --git a/malloc/tst-aligned-alloc-static.c b/malloc/tst-aligned-alloc-static.c
> new file mode 100644
> index 0000000000..d504473094
> --- /dev/null
> +++ b/malloc/tst-aligned-alloc-static.c
> @@ -0,0 +1 @@
> +#include "tst-aligned-alloc.c"
> diff --git a/malloc/tst-aligned-alloc.c b/malloc/tst-aligned-alloc.c
> new file mode 100644
> index 0000000000..8bd6527147
> --- /dev/null
> +++ b/malloc/tst-aligned-alloc.c
> @@ -0,0 +1,80 @@
> +/* Test for C17 alignment requirements.

OK. Corrected.

> +   Copyright (C) 2023 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <malloc.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <libc-diag.h>
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +{
> +  void *p1;
> +  void *p2;
> +  void *p3;
> +  void *p4;
> +  void *p5;
> +
> +  errno = 0;
> +
> +  /* The implementation supports alignments that are non-negative powers of 2.
> +     We test 5 distinct conditions here:
> +     - A non-negative power of 2 alignment e.g. 64.
> +     - A degenerate zero power of 2 alignment e.g. 1.
> +     - A non-power-of-2 alignment e.g. 65.
> +     - A zero alignment.
> +     - A corner case SIZE_MAX / 2 + 1 alignment.
> +  */
> +
> +  p1 = aligned_alloc (64, 64);
> +
> +  if (p1 == NULL)
> +    FAIL_EXIT1 ("aligned_alloc(64, 64) failed");
> +
> +  p2 = aligned_alloc (1, 64);
> +
> +  if (p2 == NULL)
> +    FAIL_EXIT1 ("aligned_alloc(1, 64) failed");
> +
> +  p3 = aligned_alloc (65, 64);
> +
> +  if (p3 != NULL)
> +    FAIL_EXIT1 ("aligned_alloc(65, 64) did not fail");
> +
> +  p4 = aligned_alloc (0, 64);
> +
> +  if (p4 != NULL)
> +    FAIL_EXIT1 ("aligned_alloc(0, 64) did not fail");
> +
> +  /* This is an alignment like 0x80000000...UL */
> +  p5 = aligned_alloc (SIZE_MAX / 2 + 1, 64);
> +
> +  if (p5 != NULL)
> +    FAIL_EXIT1 ("aligned_alloc(SIZE_MAX/2+1, 64) did not fail");
> +
> +  free (p1);
> +  free (p2);
> +  return 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/manual/memory.texi b/manual/memory.texi
> index 9d3398a326..8952ff2bfa 100644
> --- a/manual/memory.texi
> +++ b/manual/memory.texi
> @@ -995,7 +995,7 @@ power of two than that, use @code{aligned_alloc} or @code{posix_memalign}.
>  @c Alias to memalign.
>  The @code{aligned_alloc} function allocates a block of @var{size} bytes whose
>  address is a multiple of @var{alignment}.  The @var{alignment} must be a
> -power of two and @var{size} must be a multiple of @var{alignment}.
> +power of two.
>  
>  The @code{aligned_alloc} function returns a null pointer on error and sets
>  @code{errno} to one of the following values:
>
  
DJ Delorie May 8, 2023, 8:52 p.m. UTC | #2
"Carlos O'Donell" <carlos@redhat.com> writes:
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Thanks!

Pushed.
  

Patch

diff --git a/malloc/Makefile b/malloc/Makefile
index f4cae2538c..57a9677512 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -44,11 +44,13 @@  tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
 	 tst-safe-linking \
 	 tst-mallocalign1 \
 	 tst-memalign-2 \
-	 tst-memalign-3
+	 tst-memalign-3 \
+	 tst-aligned-alloc
 
 tests-static := \
 	 tst-interpose-static-nothread \
-	 tst-interpose-static-thread
+	 tst-interpose-static-thread \
+	 tst-aligned-alloc-static
 
 # Test for the malloc_set_state symbol removed in glibc 2.25.
 ifeq ($(have-GLIBC_2.23)$(build-shared),yesyes)
diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
index 3867d15698..da9d2340d3 100644
--- a/malloc/malloc-debug.c
+++ b/malloc/malloc-debug.c
@@ -299,7 +299,14 @@  __debug_memalign (size_t alignment, size_t bytes)
   return _debug_mid_memalign (alignment, bytes, RETURN_ADDRESS (0));
 }
 strong_alias (__debug_memalign, memalign)
-strong_alias (__debug_memalign, aligned_alloc)
+static void *
+__debug_aligned_alloc (size_t alignment, size_t bytes)
+{
+  if (!powerof2 (alignment) || alignment == 0)
+    return NULL;
+  return _debug_mid_memalign (alignment, bytes, RETURN_ADDRESS (0));
+}
+strong_alias (__debug_aligned_alloc, aligned_alloc)
 
 static void *
 __debug_pvalloc (size_t bytes)
diff --git a/malloc/malloc.c b/malloc/malloc.c
index e33ed665db..5d8b61d66c 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3528,6 +3528,29 @@  __libc_memalign (size_t alignment, size_t bytes)
   void *address = RETURN_ADDRESS (0);
   return _mid_memalign (alignment, bytes, address);
 }
+libc_hidden_def (__libc_memalign)
+
+/* For ISO C17.  */
+void *
+weak_function
+aligned_alloc (size_t alignment, size_t bytes)
+{
+  if (!__malloc_initialized)
+    ptmalloc_init ();
+
+/* 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)
+    {
+      __set_errno (EINVAL);
+      return 0;
+    }
+
+  void *address = RETURN_ADDRESS (0);
+  return _mid_memalign (alignment, bytes, address);
+}
 
 static void *
 _mid_memalign (size_t alignment, size_t bytes, void *address)
@@ -3618,9 +3641,6 @@  _mid_memalign (size_t alignment, size_t bytes, void *address)
           ar_ptr == arena_for_chunk (mem2chunk (p)));
   return tag_new_usable (p);
 }
-/* For ISO C11.  */
-weak_alias (__libc_memalign, aligned_alloc)
-libc_hidden_def (__libc_memalign)
 
 void *
 __libc_valloc (size_t bytes)
diff --git a/malloc/tst-aligned-alloc-static.c b/malloc/tst-aligned-alloc-static.c
new file mode 100644
index 0000000000..d504473094
--- /dev/null
+++ b/malloc/tst-aligned-alloc-static.c
@@ -0,0 +1 @@ 
+#include "tst-aligned-alloc.c"
diff --git a/malloc/tst-aligned-alloc.c b/malloc/tst-aligned-alloc.c
new file mode 100644
index 0000000000..8bd6527147
--- /dev/null
+++ b/malloc/tst-aligned-alloc.c
@@ -0,0 +1,80 @@ 
+/* Test for C17 alignment requirements.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <malloc.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <libc-diag.h>
+#include <support/check.h>
+
+static int
+do_test (void)
+{
+  void *p1;
+  void *p2;
+  void *p3;
+  void *p4;
+  void *p5;
+
+  errno = 0;
+
+  /* The implementation supports alignments that are non-negative powers of 2.
+     We test 5 distinct conditions here:
+     - A non-negative power of 2 alignment e.g. 64.
+     - A degenerate zero power of 2 alignment e.g. 1.
+     - A non-power-of-2 alignment e.g. 65.
+     - A zero alignment.
+     - A corner case SIZE_MAX / 2 + 1 alignment.
+  */
+
+  p1 = aligned_alloc (64, 64);
+
+  if (p1 == NULL)
+    FAIL_EXIT1 ("aligned_alloc(64, 64) failed");
+
+  p2 = aligned_alloc (1, 64);
+
+  if (p2 == NULL)
+    FAIL_EXIT1 ("aligned_alloc(1, 64) failed");
+
+  p3 = aligned_alloc (65, 64);
+
+  if (p3 != NULL)
+    FAIL_EXIT1 ("aligned_alloc(65, 64) did not fail");
+
+  p4 = aligned_alloc (0, 64);
+
+  if (p4 != NULL)
+    FAIL_EXIT1 ("aligned_alloc(0, 64) did not fail");
+
+  /* This is an alignment like 0x80000000...UL */
+  p5 = aligned_alloc (SIZE_MAX / 2 + 1, 64);
+
+  if (p5 != NULL)
+    FAIL_EXIT1 ("aligned_alloc(SIZE_MAX/2+1, 64) did not fail");
+
+  free (p1);
+  free (p2);
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/manual/memory.texi b/manual/memory.texi
index 9d3398a326..8952ff2bfa 100644
--- a/manual/memory.texi
+++ b/manual/memory.texi
@@ -995,7 +995,7 @@  power of two than that, use @code{aligned_alloc} or @code{posix_memalign}.
 @c Alias to memalign.
 The @code{aligned_alloc} function allocates a block of @var{size} bytes whose
 address is a multiple of @var{alignment}.  The @var{alignment} must be a
-power of two and @var{size} must be a multiple of @var{alignment}.
+power of two.
 
 The @code{aligned_alloc} function returns a null pointer on error and sets
 @code{errno} to one of the following values: