Re: [PATCH] PR libstdc++/79190 add fallback aligned_alloc implementation

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Jan 24, 2017 at 06:33:51PM +0000, Jonathan Wakely wrote:
> --- a/libstdc++-v3/libsupc++/new_opa.cc
> +++ b/libstdc++-v3/libsupc++/new_opa.cc
> @@ -55,9 +55,30 @@ extern "C" void *memalign(std::size_t boundary, std::size_t size);
>  #endif
>  #define aligned_alloc memalign
>  #else
> -// The C library doesn't provide any aligned allocation functions, declare
> -// aligned_alloc and get a link failure if aligned new is used.
> -extern "C" void *aligned_alloc(std::size_t, std::size_t);
> +// This is a modified version of code from gcc/config/i386/gmm_malloc.h
> +static inline void*
> +aligned_alloc (std::size_t al, std::size_t sz)
> +{
> +  // Alignment must be a power of two.
> +  if (al & (al - 1))
> +    return nullptr;
> +  else if (!sz)
> +    return nullptr;
> +
> +  // We need extra bytes to store the original value returned by malloc.
> +  if (al < sizeof(void*))
> +    al = sizeof(void*);
> +  void* const malloc_ptr = malloc(sz + al);
> +  if (!malloc_ptr)
> +    return nullptr;
> +  // Align to the requested value, leaving room for the original malloc value.
> +  void* const aligned_ptr = (void *) (((size_t) malloc_ptr + al) & -al);

Shouldn't this be cast to uintptr_t rather than size_t?  On some targets
that is not the same thing, I think e.g. on m32c:
grep 'SIZE_TYPE\|UINTPTR_TYPE\|POINTER_SIZE\|INT_TYPE_SIZE' config/m32c/*
config/m32c/m32c.h:#define POINTER_SIZE (TARGET_A16 ? 16 : 32)
config/m32c/m32c.h:#define INT_TYPE_SIZE 16
config/m32c/m32c.h:#undef UINTPTR_TYPE
config/m32c/m32c.h:#define UINTPTR_TYPE (TARGET_A16 ? "unsigned int" : "long unsigned int")
config/m32c/m32c.h:#undef  SIZE_TYPE
config/m32c/m32c.h:#define SIZE_TYPE "unsigned int"
which means e.g. for -mcpu=m32c pointers are 24-bit, integers/size_t are
16-bit and uintptr_t is 32-bit, so if you cast a pointer to size_t, you'll
lose the upper 8 bits.
Also, for the arguments you use std::size_t, but not here, shouldn't that
be std::uintptr_t then?

> +
> +  // Store the original malloc value where it can be found by operator delete.
> +  ((void **) aligned_ptr)[-1] = malloc_ptr;
> +
> +  return aligned_ptr;
> +}
>  #endif
>  #endif
>  

	Jakub