* [patch v4] aligned_alloc: conform to C17
@ 2023-03-22 21:38 DJ Delorie
2023-04-26 20:07 ` DJ Delorie
2023-05-02 12:43 ` Carlos O'Donell
0 siblings, 2 replies; 6+ messages in thread
From: DJ Delorie @ 2023-03-22 21:38 UTC (permalink / raw)
To: libc-alpha
Changes from V3:
* Simplified aligned_alloc symbol definition.
References:
https://patchwork.sourceware.org/project/glibc/patch/33ec9e0c1e587813b90e8aa771c2c8e6e379dd48.camel@posteo.net/
https://sourceware.org/bugzilla/show_bug.cgi?id=20137
https://sourceware.org/pipermail/libc-alpha/2023-February/145858.html
The memory.texi portion matches Martin's proposed patch.
man page portion, quoted to avoid CI/CD issues (I can send an official
patch separately after the glibc patch is applied):
> diff --git a/man3/posix_memalign.3 b/man3/posix_memalign.3
> index f5d6618b7..a73ff0421 100644
> --- a/man3/posix_memalign.3
> +++ b/man3/posix_memalign.3
> @@ -91,9 +91,8 @@ The function
> is the same as
> .BR memalign (),
> except for the added restriction that
> -.I size
> -should be a multiple of
> -.IR alignment .
> +.I alignment
> +must be a power of two.
> .PP
> The obsolete function
> .BR valloc ()
From 9cc7d558d9c06edb95102151212b319f21acf603 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 dfb51d344c..691714fb52 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -43,10 +43,12 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
tst-tcfree1 tst-tcfree2 tst-tcfree3 \
tst-safe-linking \
tst-mallocalign1 \
+ 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 76c50e3f58..ece5b8a224 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3509,6 +3509,27 @@ __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 C11. */
+void *
+weak_function
+aligned_alloc (size_t alignment, size_t bytes)
+{
+ if (!__malloc_initialized)
+ ptmalloc_init ();
+
+ /* Similar to memalign, but ISO C17 requires an error for invalid
+ alignments. Valid alignments 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)
@@ -3567,9 +3588,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..d6739376d4
--- /dev/null
+++ b/malloc/tst-aligned-alloc.c
@@ -0,0 +1,56 @@
+/* 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;
+
+ errno = 0;
+
+ p1 = aligned_alloc (64, 64);
+
+ if (p1 == NULL)
+ FAIL_EXIT1 ("aligned_alloc(64, 64) failed");
+
+ p2 = aligned_alloc (65, 64);
+
+ if (p2 != NULL)
+ FAIL_EXIT1 ("aligned_alloc(65, 64) did not fail");
+
+ p3 = aligned_alloc (0, 64);
+
+ if (p3 != NULL)
+ FAIL_EXIT1 ("aligned_alloc(0, 64) did not fail");
+
+ free (p1);
+ 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:
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch v4] aligned_alloc: conform to C17
2023-03-22 21:38 [patch v4] aligned_alloc: conform to C17 DJ Delorie
@ 2023-04-26 20:07 ` DJ Delorie
2023-05-02 12:43 ` Carlos O'Donell
1 sibling, 0 replies; 6+ messages in thread
From: DJ Delorie @ 2023-04-26 20:07 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha
Ping?
https://patchwork.sourceware.org/project/glibc/patch/xnh6uczcu3.fsf@greed.delorie.com/
https://sourceware.org/pipermail/libc-alpha/2023-March/146579.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch v4] aligned_alloc: conform to C17
2023-03-22 21:38 [patch v4] aligned_alloc: conform to C17 DJ Delorie
2023-04-26 20:07 ` DJ Delorie
@ 2023-05-02 12:43 ` Carlos O'Donell
2023-05-02 19:01 ` [patch v5] " DJ Delorie
1 sibling, 1 reply; 6+ messages in thread
From: Carlos O'Donell @ 2023-05-02 12:43 UTC (permalink / raw)
To: DJ Delorie, libc-alpha
On 3/22/23 17:38, DJ Delorie via Libc-alpha wrote:
>
> Changes from V3:
>
> * Simplified aligned_alloc symbol definition.
>
Looking forward to a v5.
> References:
> https://patchwork.sourceware.org/project/glibc/patch/33ec9e0c1e587813b90e8aa771c2c8e6e379dd48.camel@posteo.net/
> https://sourceware.org/bugzilla/show_bug.cgi?id=20137
> https://sourceware.org/pipermail/libc-alpha/2023-February/145858.html
>
> The memory.texi portion matches Martin's proposed patch.
>
> man page portion, quoted to avoid CI/CD issues (I can send an official
> patch separately after the glibc patch is applied):
>
>> diff --git a/man3/posix_memalign.3 b/man3/posix_memalign.3
>> index f5d6618b7..a73ff0421 100644
>> --- a/man3/posix_memalign.3
>> +++ b/man3/posix_memalign.3
>> @@ -91,9 +91,8 @@ The function
>> is the same as
>> .BR memalign (),
>> except for the added restriction that
>> -.I size
>> -should be a multiple of
>> -.IR alignment .
>> +.I alignment
>> +must be a power of two.
>> .PP
>> The obsolete function
>> .BR valloc ()
>
> From 9cc7d558d9c06edb95102151212b319f21acf603 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 dfb51d344c..691714fb52 100644
> --- a/malloc/Makefile
> +++ b/malloc/Makefile
> @@ -43,10 +43,12 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
> tst-tcfree1 tst-tcfree2 tst-tcfree3 \
> tst-safe-linking \
> tst-mallocalign1 \
> + tst-aligned-alloc \
OK. Adds a new test.
>
> tests-static := \
> tst-interpose-static-nothread \
> - tst-interpose-static-thread
> + tst-interpose-static-thread \
> + tst-aligned-alloc-static
OK. Adds a new test (built 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)
OK. Correctly provide a debug alias for aligned_alloc.
>
> static void *
> __debug_pvalloc (size_t bytes)
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 76c50e3f58..ece5b8a224 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3509,6 +3509,27 @@ __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 C11. */
This is not true, it's for ISO C17 (starting with n2310).
> +void *
> +weak_function
> +aligned_alloc (size_t alignment, size_t bytes)
> +{
> + if (!__malloc_initialized)
> + ptmalloc_init ();
> +
> + /* Similar to memalign, but ISO C17 requires an error for invalid
> + alignments. Valid alignments are non-negative powers of two. */
This isn't quite correct.
Suggest:
/* 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. */
The error is only in the case of alignments that are not supported by the implementation.
> + if (!powerof2 (alignment) || alignment == 0)
OK. Assumes MALLOC_ALIGNMENT is a power of 2, but that's true generally.
> + {
> + __set_errno (EINVAL);
> + return 0;
> + }
> +
> + void *address = RETURN_ADDRESS (0);
> + return _mid_memalign (alignment, bytes, address);
OK. We'll likely see back a higher alignment due to MALLOC_ALIGNMENT, but that's still aligned.
> +}
>
> static void *
> _mid_memalign (size_t alignment, size_t bytes, void *address)
> @@ -3567,9 +3588,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)
OK.
>
> 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..d6739376d4
> --- /dev/null
> +++ b/malloc/tst-aligned-alloc.c
> @@ -0,0 +1,56 @@
> +/* Copyright (C) 2023 Free Software Foundation, Inc.
Needs a first line explaining what the test does.
> + 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;
> +
> + errno = 0;
> +
Suggest:
/* 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");
Please add a degenerate test for alignment == 1 e.g. 2^(0)
> +
> + p2 = aligned_alloc (65, 64);
> +
> + if (p2 != NULL)
> + FAIL_EXIT1 ("aligned_alloc(65, 64) did not fail");
> +
> + p3 = aligned_alloc (0, 64);
> +
> + if (p3 != NULL)
> + FAIL_EXIT1 ("aligned_alloc(0, 64) did not fail");
Please add a SIZE_MAX / 2 + 1 alignment case that has to fail.
> +
> + free (p1);
> + 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.
OK.
>
> The @code{aligned_alloc} function returns a null pointer on error and sets
> @code{errno} to one of the following values:
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch v5] aligned_alloc: conform to C17
2023-05-02 12:43 ` Carlos O'Donell
@ 2023-05-02 19:01 ` DJ Delorie
2023-05-08 14:20 ` Carlos O'Donell
0 siblings, 1 reply; 6+ messages in thread
From: DJ Delorie @ 2023-05-02 19:01 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: libc-alpha
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.
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:
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch v5] aligned_alloc: conform to C17
2023-05-02 19:01 ` [patch v5] " DJ Delorie
@ 2023-05-08 14:20 ` Carlos O'Donell
2023-05-08 20:52 ` DJ Delorie
0 siblings, 1 reply; 6+ messages in thread
From: Carlos O'Donell @ 2023-05-08 14:20 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha
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:
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch v5] aligned_alloc: conform to C17
2023-05-08 14:20 ` Carlos O'Donell
@ 2023-05-08 20:52 ` DJ Delorie
0 siblings, 0 replies; 6+ messages in thread
From: DJ Delorie @ 2023-05-08 20:52 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: libc-alpha
"Carlos O'Donell" <carlos@redhat.com> writes:
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Thanks!
Pushed.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-08 20:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-22 21:38 [patch v4] aligned_alloc: conform to C17 DJ Delorie
2023-04-26 20:07 ` DJ Delorie
2023-05-02 12:43 ` Carlos O'Donell
2023-05-02 19:01 ` [patch v5] " DJ Delorie
2023-05-08 14:20 ` Carlos O'Donell
2023-05-08 20:52 ` DJ Delorie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).