manual: remove an obsolete requirement on aligned_alloc() usage

  I'm not subscribed; please CC me.
  

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
  

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
  

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
  

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.
  

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