Re: [patch] aligned_alloc: conform to C17

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 16/03/23 17:48, DJ Delorie via Libc-alpha wrote:
> 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 4767e0e764e1a7a5ef01e303f503036379dd42c5 Mon Sep 17 00:00:00 2001
> From: DJ Delorie <dj@redhat.com>
> Date: Thu, 16 Mar 2023 01:33:41 -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.

Hi D.J, this patch does not build correctly [1].  Maybe you send it too soon?

[1] https://patchwork.sourceware.org/project/glibc/patch/xnv8j02zji.fsf@greed.delorie.com/

> 
> diff --git a/malloc/Versions b/malloc/Versions
> index c763395c6d..28f41a94f3 100644
> --- a/malloc/Versions
> +++ b/malloc/Versions
> @@ -67,6 +67,9 @@ libc {
>    GLIBC_2.33 {
>      mallinfo2;
>    }
> +  GLIBC_2.38 {
> +    __libc_aligned_alloc;
> +  }
>    GLIBC_PRIVATE {
>      # Internal startup hook for libpthread.
>      __libc_malloc_pthread_startup;

How is is actually used? There is no redirection in the header if -std=c17 is used,
nor any abilist update.

> diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
> index 3867d15698..4d2ec04a1a 100644
> --- a/malloc/malloc-debug.c
> +++ b/malloc/malloc-debug.c
> @@ -268,7 +268,8 @@ __debug_realloc (void *oldmem, size_t bytes)
>  strong_alias (__debug_realloc, realloc)
>  
>  static void *
> -_debug_mid_memalign (size_t alignment, size_t bytes, const void *address)
> +_debug_mid_memalign (size_t alignment, size_t bytes, const void *address,
> +		     int check_alignment)
>  {
>    void *(*hook) (size_t, size_t, const void *) =
>      atomic_forced_read (__memalign_hook);
> @@ -281,9 +282,15 @@ _debug_mid_memalign (size_t alignment, size_t bytes, const void *address)
>    if ((!__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK)
>         || !memalign_mcheck_before (alignment, &bytes, &victim)))
>      {
> -      victim = (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)
> -		? memalign_check (alignment, bytes)
> -		: __libc_memalign (alignment, bytes));
> +      if (check_alignment && !powerof2 (alignment))
> +	{
> +	  __set_errno (EINVAL);
> +	  victim = NULL;
> +	}
> +      else
> +	victim = (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)
> +		  ? memalign_check (alignment, bytes)
> +		  : __libc_memalign (alignment, bytes));
>      }
>    if (__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK) && victim != NULL)
>      victim = memalign_mcheck_after (victim, alignment, orig_bytes);
> @@ -296,10 +303,15 @@ _debug_mid_memalign (size_t alignment, size_t bytes, const void *address)
>  static void *
>  __debug_memalign (size_t alignment, size_t bytes)
>  {
> -  return _debug_mid_memalign (alignment, bytes, RETURN_ADDRESS (0));
> +  return _debug_mid_memalign (alignment, bytes, RETURN_ADDRESS (0), 0);
>  }
>  strong_alias (__debug_memalign, memalign)
> -strong_alias (__debug_memalign, aligned_alloc)
> +static void *
> +__debug_aligned_alloc (size_t alignment, size_t bytes)
> +{
> +  return _debug_mid_memalign (alignment, bytes, RETURN_ADDRESS (0), 1);
> +}
> +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..09619ed168 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -656,6 +656,9 @@ libc_hidden_proto (__libc_realloc)
>  void*  __libc_memalign(size_t, size_t);
>  libc_hidden_proto (__libc_memalign)
>  
> +void * __libc_aligned_alloc (size_t alignment, size_t bytes);
> +libc_hidden_proto (__libc_aligned_alloc)
> +
>  /*
>    valloc(size_t n);
>    Equivalent to memalign(pagesize, n), where pagesize is the page
> @@ -3509,6 +3512,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 *
> +__libc_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))
> +    {
> +      __set_errno (EINVAL);
> +      return 0;
> +    }
> +
> +  void *address = RETURN_ADDRESS (0);
> +  return _mid_memalign (alignment, bytes, address);
> +}
> +libc_hidden_def (__libc_aligned_alloc)
>  
>  static void *
>  _mid_memalign (size_t alignment, size_t bytes, void *address)
> @@ -3567,9 +3591,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)
> @@ -5903,6 +5924,7 @@ weak_alias (__libc_mallinfo, mallinfo)
>  strong_alias (__libc_mallinfo2, __mallinfo2)
>  weak_alias (__libc_mallinfo2, mallinfo2)
>  strong_alias (__libc_mallopt, __mallopt) weak_alias (__libc_mallopt, mallopt)
> +weak_alias (__libc_aligned_alloc, aligned_alloc)
>  
>  weak_alias (__malloc_stats, malloc_stats)
>  weak_alias (__malloc_usable_size, malloc_usable_size)
> 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:
>