* [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage
@ 2021-10-21 9:28 John Scott
2021-10-21 15:41 ` Alejandro Colomar (man-pages)
2021-10-25 15:45 ` Martin Sebor
0 siblings, 2 replies; 6+ messages in thread
From: John Scott @ 2021-10-21 9:28 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1.1: Type: text/plain, Size: 35 bytes --]
I'm not subscribed; please CC me.
[-- Attachment #1.2: 0001-manual-remove-an-obsolete-requirement-on-aligned_all.patch --]
[-- Type: text/x-patch, Size: 1579 bytes --]
From 315c821421a2279d77e846bf927b7c4107e32d43 Mon Sep 17 00:00:00 2001
From: John Scott <jscott@posteo.net>
Date: Thu, 21 Oct 2021 05:20:55 -0400
Subject: [PATCH] manual: remove an obsolete requirement on aligned_alloc()
usage
The C11 standard made it undefined behavior if the size of
the allocation was not a multiple of the page size. As discussed
at BZ #20137, changes to the standard were proposed and
subsequently adopted in Defect Report 460.
In particular, the following sentence
"The value of alignment shall be a valid alignment supported by
the implementation and the value of size shall be an integral
multiple of alignment."
was changed to
"If the value of alignment is not a valid alignment supported by
the implementation the function shall fail by returning a null pointer."
The DR is at
http://www.open-std.org/jtc1/sc22/wg14/www/docs/summary.htm#dr_460
---
manual/memory.texi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/manual/memory.texi b/manual/memory.texi
index 0b2b9c87..5c16f7ae 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:
--
2.33.0
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage
2021-10-21 9:28 [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage John Scott
@ 2021-10-21 15:41 ` Alejandro Colomar (man-pages)
2021-10-25 15:45 ` Martin Sebor
1 sibling, 0 replies; 6+ messages in thread
From: Alejandro Colomar (man-pages) @ 2021-10-21 15:41 UTC (permalink / raw)
To: John Scott, libc-alpha
Hi John,
On 10/21/21 11:28 AM, John Scott via Libc-alpha wrote:
> 0001-manual-remove-an-obsolete-requirement-on-aligned_all.patch
>
> From 315c821421a2279d77e846bf927b7c4107e32d43 Mon Sep 17 00:00:00 2001
> From: John Scott<jscott@posteo.net>
> Date: Thu, 21 Oct 2021 05:20:55 -0400
> Subject: [PATCH] manual: remove an obsolete requirement on aligned_alloc()
> usage
>
> The C11 standard made it undefined behavior if the size of
> the allocation was not a multiple of the page size. As discussed
> at BZ #20137, changes to the standard were proposed and
> subsequently adopted in Defect Report 460.
Was that fixed in C17? Or in (yet unreleased) C2X?
>
> In particular, the following sentence
> "The value of alignment shall be a valid alignment supported by
> the implementation and the value of size shall be an integral
> multiple of alignment."
> was changed to
> "If the value of alignment is not a valid alignment supported by
> the implementation the function shall fail by returning a null pointer."
Did any implementation make use of that Undefined Behavior prior to its
fix? If so, at least in the man-pages, we should keep a historical note
to warn about it.
Thanks,
Alex
>
> The DR is at
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/summary.htm#dr_460
> ---
> manual/memory.texi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/manual/memory.texi b/manual/memory.texi
> index 0b2b9c87..5c16f7ae 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:
> -- 2.33.0
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage
2021-10-21 9:28 [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage John Scott
2021-10-21 15:41 ` Alejandro Colomar (man-pages)
@ 2021-10-25 15:45 ` Martin Sebor
2021-10-25 16:03 ` Martin Sebor
2021-10-26 1:50 ` Paul Eggert
1 sibling, 2 replies; 6+ messages in thread
From: Martin Sebor @ 2021-10-25 15:45 UTC (permalink / raw)
To: John Scott, libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]
On 10/21/21 3:28 AM, John Scott via Libc-alpha wrote:
> I'm not subscribed; please CC me.
>
Carlos asked me offline to comment on this change. I'm not
a Glibc expert so I'll do my best but I defer to others here
who know the allocator much better.
diff --git a/manual/memory.texi b/manual/memory.texi
index 0b2b9c87..5c16f7ae 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.
Based on my code inspection of _mid_memalign in malloc.c I think
a change that would more accurately reflect the implementation
is one that described that when alignment is not a power of two
it's bumped up to next larger power of two, and adjusted
the EINVAL condition appropriately. Attached is a diff along
these lines. Please feel free to use it as you see fit.
Martin
[-- Attachment #2: glibc-doc-20137.diff --]
[-- Type: text/x-patch, Size: 1293 bytes --]
diff --git a/manual/memory.texi b/manual/memory.texi
index 0b2b9c8795..9e407cd44a 100644
--- a/manual/memory.texi
+++ b/manual/memory.texi
@@ -994,8 +994,8 @@ power of two than that, use @code{aligned_alloc} or @code{posix_memalign}.
@safety{@prelim{}@mtsafe{}@asunsafe{@asulock{}}@acunsafe{@aculock{} @acsfd{} @acsmem{}}}
@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}.
+address is a multiple of @var{adjusted-alignment}. The value of @var{adjusted-alignment}
+is the smallest power of two that's greater than or equal to @var{alignment}.
The @code{aligned_alloc} function returns a null pointer on error and sets
@code{errno} to one of the following values:
@@ -1005,7 +1005,7 @@ The @code{aligned_alloc} function returns a null pointer on error and sets
There was insufficient memory available to satisfy the request.
@item EINVAL
-@var{alignment} is not a power of two.
+The value of @var{alignment} is greater than @code{SIZE_MAX / 2 + 1}.
This function was introduced in @w{ISO C11} and hence may have better
portability to modern non-POSIX systems than @code{posix_memalign}.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage
2021-10-25 15:45 ` Martin Sebor
@ 2021-10-25 16:03 ` Martin Sebor
2021-10-26 1:50 ` Paul Eggert
1 sibling, 0 replies; 6+ messages in thread
From: Martin Sebor @ 2021-10-25 16:03 UTC (permalink / raw)
To: John Scott, libc-alpha
On 10/25/21 9:45 AM, Martin Sebor wrote:
> On 10/21/21 3:28 AM, John Scott via Libc-alpha wrote:
>> I'm not subscribed; please CC me.
>>
>
> Carlos asked me offline to comment on this change. I'm not
> a Glibc expert so I'll do my best but I defer to others here
> who know the allocator much better.
>
> diff --git a/manual/memory.texi b/manual/memory.texi
> index 0b2b9c87..5c16f7ae 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.
>
> Based on my code inspection of _mid_memalign in malloc.c I think
> a change that would more accurately reflect the implementation
> is one that described that when alignment is not a power of two
> it's bumped up to next larger power of two, and adjusted
> the EINVAL condition appropriately. Attached is a diff along
> these lines. Please feel free to use it as you see fit.
Also, to answer the follow-up question in Patchwork (I can't find
it in my mailbox):
Was that fixed in C17? Or in (yet unreleased) C2X?
I believe the DR460 change went into C17. Here's an early C2x
draft from 2018 publicly available on the WG14 site that should
still be very close to C17:
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2310.pdf
>
> Martin
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage
2021-10-25 15:45 ` Martin Sebor
2021-10-25 16:03 ` Martin Sebor
@ 2021-10-26 1:50 ` Paul Eggert
2021-10-26 2:34 ` Martin Sebor
1 sibling, 1 reply; 6+ messages in thread
From: Paul Eggert @ 2021-10-26 1:50 UTC (permalink / raw)
To: Martin Sebor; +Cc: John Scott, libc-alpha
[-- Attachment #1: Type: text/plain, Size: 962 bytes --]
On 10/25/21 08:45, Martin Sebor via Libc-alpha wrote:
> Based on my code inspection of _mid_memalign in malloc.c I think
> a change that would more accurately reflect the implementation
> is one that described that when alignment is not a power of two
> it's bumped up to next larger power of two, and adjusted
> the EINVAL condition appropriately.
Let's not document this implementation detail, as it might tie us down
unnecessarily in the future.
> I believe the DR460 change went into C17. Here's an early C2x
> draft from 2018 publicly available on the WG14 site that should
> still be very close to C17:
>
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2310.pdf
This says that aligned_alloc "shall fail" if the alignment is not a
valid alignment supported by the implementation. This means the current
glibc alloc_aligned behavior does not conform to C17 and should be fixed
so that it does. Something like the attached (untested) patch, say.
[-- Attachment #2: align.patch --]
[-- Type: text/x-patch, Size: 1155 bytes --]
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 2ba1fee144..386fb5a4cb 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3416,6 +3416,12 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
mstate ar_ptr;
void *p;
+ if (!powerof2 (alignment))
+ {
+ __set_errno (EINVAL);
+ return 0;
+ }
+
/* If we need less alignment than we give anyway, just relay to malloc. */
if (alignment <= MALLOC_ALIGNMENT)
return __libc_malloc (bytes);
@@ -3424,24 +3430,6 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
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 0;
- }
-
-
- /* Make sure alignment is power of 2. */
- if (!powerof2 (alignment))
- {
- size_t a = MALLOC_ALIGNMENT * 2;
- while (a < alignment)
- a <<= 1;
- alignment = a;
- }
-
if (SINGLE_THREAD_P)
{
p = _int_memalign (&main_arena, alignment, bytes);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage
2021-10-26 1:50 ` Paul Eggert
@ 2021-10-26 2:34 ` Martin Sebor
0 siblings, 0 replies; 6+ messages in thread
From: Martin Sebor @ 2021-10-26 2:34 UTC (permalink / raw)
To: Paul Eggert; +Cc: John Scott, libc-alpha
On 10/25/21 7:50 PM, Paul Eggert wrote:
> On 10/25/21 08:45, Martin Sebor via Libc-alpha wrote:
>> Based on my code inspection of _mid_memalign in malloc.c I think
>> a change that would more accurately reflect the implementation
>> is one that described that when alignment is not a power of two
>> it's bumped up to next larger power of two, and adjusted
>> the EINVAL condition appropriately.
>
> Let's not document this implementation detail, as it might tie us down
> unnecessarily in the future.
>
>> I believe the DR460 change went into C17. Here's an early C2x
>> draft from 2018 publicly available on the WG14 site that should
>> still be very close to C17:
>>
>> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2310.pdf
>
> This says that aligned_alloc "shall fail" if the alignment is not a
> valid alignment supported by the implementation. This means the current
> glibc alloc_aligned behavior does not conform to C17 and should be fixed
> so that it does. Something like the attached (untested) patch, say.
I agree. I took the term "valid alignment" here to mean one
specifically accepted by aligned_alloc() but 6.2.8 is explicit
that
"Every valid alignment value shall be a nonnegative integral
power of two."
so although implementations get to decide on the set of valid
extended alignments they're not allowed to be accept ones that
are in conflict with the blanket requirement above.
Martin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-26 2:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 9:28 [PATCH] manual: remove an obsolete requirement on aligned_alloc() usage John Scott
2021-10-21 15:41 ` Alejandro Colomar (man-pages)
2021-10-25 15:45 ` Martin Sebor
2021-10-25 16:03 ` Martin Sebor
2021-10-26 1:50 ` Paul Eggert
2021-10-26 2:34 ` Martin Sebor
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).