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

[Jonathan Wakely]

On 26/01/17 01:05 +0100, Jakub Jelinek wrote:
>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.

Good point, thanks.

>Also, for the arguments you use std::size_t, but not here, shouldn't that
>be std::uintptr_t then?

We use std::size_t there because we only included <bits/c++config.h>
and not <stddef.h>, so we're not guaranteed to have ::size_t declared.
If I include <stdint.h> for uintptr_t then it won't be in namespace
std and can be used unqualified. Elsewhere we guard use of stdint.h
with _GLIBCXX_USE_C99_STDINT_TR1 but I don't think we need that
nowadays,

Dave Anglin confirmed this fixes the problem, so I'll commit this
version (just adding <stdint.h> and using uintptr_t instead of
size_t).

I forgot to mention that the g++.dg/cpp1z/aligned-new3.C test needed
tweaking because it replaced the aligned-delete operator but not the
aligned-new operator. That meant it was failing when this fallback
implementation of aligned_alloc is used, because aligned-new did the
prefixed allocation, but the replaced operator delete doesn't free the
original pointer stored in the prefix. By replacing both operators we
avoid the prefixing entirely.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 3592 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20170126/e0da93bf/attachment.bin>