libstdc++-v3: Have aligned_alloc() on Newlib

Message ID 20180808085238.28201-1-sebastian.huber@embedded-brains.de
State New
Headers show
Series
  • libstdc++-v3: Have aligned_alloc() on Newlib
Related show

Commit Message

Sebastian Huber Aug. 8, 2018, 8:52 a.m.
While building for Newlib, some configure checks must be hard coded.
The aligned_alloc() is supported since 2015 in Newlib.

libstdc++-v3

	PR target/85904
	* configure.ac): Define HAVE_ALIGNED_ALLOC if building for
	Newlib.
	* configure: Regnerate.
---
 libstdc++-v3/configure    | 2 ++
 libstdc++-v3/configure.ac | 1 +
 2 files changed, 3 insertions(+)

Comments

Jonathan Wakely Aug. 8, 2018, 11:18 a.m. | #1
On 08/08/18 10:52 +0200, Sebastian Huber wrote:
>While building for Newlib, some configure checks must be hard coded.
>The aligned_alloc() is supported since 2015 in Newlib.
>
>libstdc++-v3
>
>	PR target/85904
>	* configure.ac): Define HAVE_ALIGNED_ALLOC if building for

There's a stray closing parenthesis here.

>	Newlib.
>	* configure: Regnerate.

Typo "Regnerate".

But the patch itself is fine - OK for trunk.

I'm ambivalent about this being backported to gcc-7 and gcc-8 branches
(gcc-6 is unaffected as it doesn't use aligned_alloc).

It's strictly speaking an ABI change, because HAVE_ALIGNED_ALLOC
affects the memory layout for allocations from operator new(size_t,
align_val_t) (in new_opa.cc) which needs to agree with the
corresponding operator delete (in del_opa.cc). Using static linking it
might be possible to create a binary that has operator new using
aligned_alloc, but operator delete expecting to do ((void**)ptr)[-1],
which would be bad.

Those operators are C++17, so "experimental", but maybe we shouldn't
make the change on release branches.
Sebastian Huber Aug. 8, 2018, 1:38 p.m. | #2
On 08/08/18 13:18, Jonathan Wakely wrote:
> On 08/08/18 10:52 +0200, Sebastian Huber wrote:
>> While building for Newlib, some configure checks must be hard coded.
>> The aligned_alloc() is supported since 2015 in Newlib.
>>
>> libstdc++-v3
>>
>>     PR target/85904
>>     * configure.ac): Define HAVE_ALIGNED_ALLOC if building for
>
> There's a stray closing parenthesis here.
>
>>     Newlib.
>>     * configure: Regnerate.
>
> Typo "Regnerate".
>
> But the patch itself is fine - OK for trunk.

Oh, sorry, for all the typos.

>
> I'm ambivalent about this being backported to gcc-7 and gcc-8 branches
> (gcc-6 is unaffected as it doesn't use aligned_alloc).
>
> It's strictly speaking an ABI change, because HAVE_ALIGNED_ALLOC
> affects the memory layout for allocations from operator new(size_t,
> align_val_t) (in new_opa.cc) which needs to agree with the
> corresponding operator delete (in del_opa.cc). Using static linking it
> might be possible to create a binary that has operator new using
> aligned_alloc, but operator delete expecting to do ((void**)ptr)[-1],
> which would be bad.
>
> Those operators are C++17, so "experimental", but maybe we shouldn't
> make the change on release branches.
>

I was able to build a GCC 7.3.0 compiler, but now the build fails on the 
GCC 7 branch.
Jonathan Wakely Aug. 8, 2018, 2:01 p.m. | #3
On 08/08/18 15:57 +0200, Ulrich Weigand wrote:
>Jonathan Wakely wrote:
>> On 08/08/18 10:52 +0200, Sebastian Huber wrote:
>> >While building for Newlib, some configure checks must be hard coded.
>> >The aligned_alloc() is supported since 2015 in Newlib.
>> >
>> >libstdc++-v3
>> >
>> >	PR target/85904
>> >	* configure.ac): Define HAVE_ALIGNED_ALLOC if building for
>>
>> There's a stray closing parenthesis here.
>>
>> >	Newlib.
>> >	* configure: Regnerate.
>>
>> Typo "Regnerate".
>>
>> But the patch itself is fine - OK for trunk.
>>
>> I'm ambivalent about this being backported to gcc-7 and gcc-8 branches
>> (gcc-6 is unaffected as it doesn't use aligned_alloc).
>>
>> It's strictly speaking an ABI change, because HAVE_ALIGNED_ALLOC
>> affects the memory layout for allocations from operator new(size_t,
>> align_val_t) (in new_opa.cc) which needs to agree with the
>> corresponding operator delete (in del_opa.cc). Using static linking it
>> might be possible to create a binary that has operator new using
>> aligned_alloc, but operator delete expecting to do ((void**)ptr)[-1],
>> which would be bad.
>>
>> Those operators are C++17, so "experimental", but maybe we shouldn't
>> make the change on release branches.
>
>The way it is now I'm getting build failures on new SPU target
>(which is newlib based):
>
>/home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-head/src/libstdc++-v3/libsupc++/new_opa.cc:58:1: error: 'void* aligned_alloc(std::size_t, std::size_t)' was declared 'extern' and later 'static' [-fpermissive]
> aligned_alloc (std::size_t al, std::size_t sz)
> ^~~~~~~~~~~~~
>In file included from /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-build/spu/libstdc++-v3/include/cstdlib:75:62,
>                 from /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-build/spu/libstdc++-v3/include/stdlib.h:36,
>                 from /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-head/src/libstdc++-v3/libsupc++/new_opa.cc:27:
>/home/uweigand/dailybuild/spu-tc-2018-08-07/spu-toolchain/spu/include/stdlib.h:328:8: note: previous declaration of 'void* aligned_alloc(size_t, size_t)'
> void * aligned_alloc(size_t, size_t) __malloc_like __alloc_align(1)
>        ^~~~~~~~~~~~~
>
>This seems to be because configure is hard-coded to assume aligned_alloc
>is not available, but then the new_opc.cc file tries to define its own
>version, conflicting with the newlib prototype that is actually there.
>
>So one way or the other this needs to be fixed ...

But that's on trunk, right? Sebastian is about to fix that.

I expect the same problem exists on the branches too though.

I'm surprised nobody has reported this until now. It's been like that
since r244933 in Jan 2017.

Maybe we should put the fallback implementation into a namespace to
avoid the same problem on other targets where HAVE_ALIGNED_ALLOC isn't
set correctly.
Sebastian Huber Aug. 8, 2018, 2:04 p.m. | #4
On 08/08/18 16:01, Jonathan Wakely wrote:
> On 08/08/18 15:57 +0200, Ulrich Weigand wrote:
>> Jonathan Wakely wrote:
>>> On 08/08/18 10:52 +0200, Sebastian Huber wrote:
>>> >While building for Newlib, some configure checks must be hard coded.
>>> >The aligned_alloc() is supported since 2015 in Newlib.
>>> >
>>> >libstdc++-v3
>>> >
>>> >    PR target/85904
>>> >    * configure.ac): Define HAVE_ALIGNED_ALLOC if building for
>>>
>>> There's a stray closing parenthesis here.
>>>
>>> >    Newlib.
>>> >    * configure: Regnerate.
>>>
>>> Typo "Regnerate".
>>>
>>> But the patch itself is fine - OK for trunk.
>>>
>>> I'm ambivalent about this being backported to gcc-7 and gcc-8 branches
>>> (gcc-6 is unaffected as it doesn't use aligned_alloc).
>>>
>>> It's strictly speaking an ABI change, because HAVE_ALIGNED_ALLOC
>>> affects the memory layout for allocations from operator new(size_t,
>>> align_val_t) (in new_opa.cc) which needs to agree with the
>>> corresponding operator delete (in del_opa.cc). Using static linking it
>>> might be possible to create a binary that has operator new using
>>> aligned_alloc, but operator delete expecting to do ((void**)ptr)[-1],
>>> which would be bad.
>>>
>>> Those operators are C++17, so "experimental", but maybe we shouldn't
>>> make the change on release branches.
>>
>> The way it is now I'm getting build failures on new SPU target
>> (which is newlib based):
>>
>> /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-head/src/libstdc++-v3/libsupc++/new_opa.cc:58:1: 
>> error: 'void* aligned_alloc(std::size_t, std::size_t)' was declared 
>> 'extern' and later 'static' [-fpermissive]
>> aligned_alloc (std::size_t al, std::size_t sz)
>> ^~~~~~~~~~~~~
>> In file included from 
>> /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-build/spu/libstdc++-v3/include/cstdlib:75:62,
>>                 from 
>> /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-build/spu/libstdc++-v3/include/stdlib.h:36,
>>                 from 
>> /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-head/src/libstdc++-v3/libsupc++/new_opa.cc:27:
>> /home/uweigand/dailybuild/spu-tc-2018-08-07/spu-toolchain/spu/include/stdlib.h:328:8: 
>> note: previous declaration of 'void* aligned_alloc(size_t, size_t)'
>> void * aligned_alloc(size_t, size_t) __malloc_like __alloc_align(1)
>>        ^~~~~~~~~~~~~
>>
>> This seems to be because configure is hard-coded to assume aligned_alloc
>> is not available, but then the new_opc.cc file tries to define its own
>> version, conflicting with the newlib prototype that is actually there.
>>
>> So one way or the other this needs to be fixed ...
>
> But that's on trunk, right? Sebastian is about to fix that.
>
> I expect the same problem exists on the branches too though.
>
> I'm surprised nobody has reported this until now. It's been like that
> since r244933 in Jan 2017. 

I notices this problem today with a build of the GCC 7 branch. I am able 
to build a GCC 7.3.0.
Jonathan Wakely Aug. 8, 2018, 2:10 p.m. | #5
On 08/08/18 16:04 +0200, Sebastian Huber wrote:
>On 08/08/18 16:01, Jonathan Wakely wrote:
>>On 08/08/18 15:57 +0200, Ulrich Weigand wrote:
>>>Jonathan Wakely wrote:
>>>>On 08/08/18 10:52 +0200, Sebastian Huber wrote:
>>>>>While building for Newlib, some configure checks must be hard coded.
>>>>>The aligned_alloc() is supported since 2015 in Newlib.
>>>>>
>>>>>libstdc++-v3
>>>>>
>>>>>    PR target/85904
>>>>>    * configure.ac): Define HAVE_ALIGNED_ALLOC if building for
>>>>
>>>>There's a stray closing parenthesis here.
>>>>
>>>>>    Newlib.
>>>>>    * configure: Regnerate.
>>>>
>>>>Typo "Regnerate".
>>>>
>>>>But the patch itself is fine - OK for trunk.
>>>>
>>>>I'm ambivalent about this being backported to gcc-7 and gcc-8 branches
>>>>(gcc-6 is unaffected as it doesn't use aligned_alloc).
>>>>
>>>>It's strictly speaking an ABI change, because HAVE_ALIGNED_ALLOC
>>>>affects the memory layout for allocations from operator new(size_t,
>>>>align_val_t) (in new_opa.cc) which needs to agree with the
>>>>corresponding operator delete (in del_opa.cc). Using static linking it
>>>>might be possible to create a binary that has operator new using
>>>>aligned_alloc, but operator delete expecting to do ((void**)ptr)[-1],
>>>>which would be bad.
>>>>
>>>>Those operators are C++17, so "experimental", but maybe we shouldn't
>>>>make the change on release branches.
>>>
>>>The way it is now I'm getting build failures on new SPU target
>>>(which is newlib based):
>>>
>>>/home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-head/src/libstdc++-v3/libsupc++/new_opa.cc:58:1: 
>>>error: 'void* aligned_alloc(std::size_t, std::size_t)' was 
>>>declared 'extern' and later 'static' [-fpermissive]
>>>aligned_alloc (std::size_t al, std::size_t sz)
>>>^~~~~~~~~~~~~
>>>In file included from /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-build/spu/libstdc++-v3/include/cstdlib:75:62,
>>>                from /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-build/spu/libstdc++-v3/include/stdlib.h:36,
>>>                from /home/uweigand/dailybuild/spu-tc-2018-08-07/gcc-head/src/libstdc++-v3/libsupc++/new_opa.cc:27:
>>>/home/uweigand/dailybuild/spu-tc-2018-08-07/spu-toolchain/spu/include/stdlib.h:328:8: 
>>>note: previous declaration of 'void* aligned_alloc(size_t, 
>>>size_t)'
>>>void * aligned_alloc(size_t, size_t) __malloc_like __alloc_align(1)
>>>       ^~~~~~~~~~~~~
>>>
>>>This seems to be because configure is hard-coded to assume aligned_alloc
>>>is not available, but then the new_opc.cc file tries to define its own
>>>version, conflicting with the newlib prototype that is actually there.
>>>
>>>So one way or the other this needs to be fixed ...
>>
>>But that's on trunk, right? Sebastian is about to fix that.
>>
>>I expect the same problem exists on the branches too though.
>>
>>I'm surprised nobody has reported this until now. It's been like that
>>since r244933 in Jan 2017.
>
>I notices this problem today with a build of the GCC 7 branch. I am 
>able to build a GCC 7.3.0.

Aha, so newlib was using memalign previously:

@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
 #else
 extern "C" void *memalign(std::size_t boundary, std::size_t size);
 #endif
-#define aligned_alloc memalign
-#else
+static inline void*
+aligned_alloc (std::size_t al, std::size_t sz)
+{
+#ifdef __sun
+  // Solaris 10 memalign requires that alignment is greater than or equal to
+  // the size of a word.
+  if (al < sizeof(int))
+    al = sizeof(int);
+#endif
+  return memalign (al, sz);
+}
+#else // !HAVE__ALIGNED_MALLOC && !HAVE_POSIX_MEMALIGN && !HAVE_MEMALIGN
 #include <stdint.h>
 // The C library doesn't provide any aligned allocation functions, define one.
 // This is a modified version of code from gcc/config/i386/gmm_malloc.h


OK, I've regressed the branches then - I'll fix that.
Jonathan Wakely Aug. 8, 2018, 2:33 p.m. | #6
On 08/08/18 16:22 +0200, Ulrich Weigand wrote:
>Jonathan Wakely wrote:
>
>> Aha, so newlib was using memalign previously:
>>
>> @@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
>>  #else
>>  extern "C" void *memalign(std::size_t boundary, std::size_t size);
>>  #endif
>> -#define aligned_alloc memalign
>
>Yes, exactly ... this commit introduced the regression.
>
>> OK, I've regressed the branches then - I'll fix that.

This should fix it. I'll finish testing and commit it.

Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
gcc-7-branch and gcc-8-branch, because changing newlib from using
memalign to aligned_alloc is safe.

Sorry for the mess.
Jonathan Wakely Aug. 8, 2018, 2:46 p.m. | #7
On 08/08/18 15:33 +0100, Jonathan Wakely wrote:
>On 08/08/18 16:22 +0200, Ulrich Weigand wrote:
>>Jonathan Wakely wrote:
>>
>>>Aha, so newlib was using memalign previously:
>>>
>>>@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
>>> #else
>>> extern "C" void *memalign(std::size_t boundary, std::size_t size);
>>> #endif
>>>-#define aligned_alloc memalign
>>
>>Yes, exactly ... this commit introduced the regression.
>>
>>>OK, I've regressed the branches then - I'll fix that.
>
>This should fix it. I'll finish testing and commit it.
>
>Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
>gcc-7-branch and gcc-8-branch, because changing newlib from using
>memalign to aligned_alloc is safe.

With the patch this time ...
commit cb2a7aae2668b690cfaed5093e0107e0ee64bb0e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Aug 8 15:28:48 2018 +0100

    Prevent internal aligned_alloc clashing with libc version
    
    If configure fails to detect aligned_alloc we will try to define our
    own in new_opa.cc but that could clash with the libcversion in
    <stdlib.h>. Use a namespace to keep them distinct.
    
            * libsupc++/new_opa.cc (aligned_alloc): Declare inside namespace to
            avoid clashing with an ::aligned_alloc function that was not detected
            by configure.

diff --git a/libstdc++-v3/libsupc++/new_opa.cc b/libstdc++-v3/libsupc++/new_opa.cc
index 5be0cc2ca65..68eac5b8ceb 100644
--- a/libstdc++-v3/libsupc++/new_opa.cc
+++ b/libstdc++-v3/libsupc++/new_opa.cc
@@ -25,15 +25,30 @@
 
 #include <bits/c++config.h>
 #include <stdlib.h>
+#include <stdint.h>
 #include <bits/exception_defines.h>
 #include "new"
 
+#if !_GLIBCXX_HAVE_ALIGNED_ALLOC && !_GLIBCXX_HAVE__ALIGNED_MALLOC \
+  && !_GLIBCXX_HAVE_POSIX_MEMALIGN && _GLIBCXX_HAVE_MEMALIGN
+# if _GLIBCXX_HOSTED && __has_include(<malloc.h>)
+// Some C libraries declare memalign in <malloc.h>
+#  include <malloc.h>
+# else
+extern "C" void *memalign(std::size_t boundary, std::size_t size);
+# endif
+#endif
+
 using std::new_handler;
 using std::bad_alloc;
 
-#if !_GLIBCXX_HAVE_ALIGNED_ALLOC
-#if _GLIBCXX_HAVE__ALIGNED_MALLOC
-#define aligned_alloc(al,sz) _aligned_malloc(sz,al)
+namespace __gnu_cxx {
+#if _GLIBCXX_HAVE_ALIGNED_ALLOC
+using ::aligned_alloc;
+#elif _GLIBCXX_HAVE__ALIGNED_MALLOC
+static inline void*
+aligned_alloc (std::size_t al, std::size_t sz)
+{ return _aligned_malloc(sz, al); }
 #elif _GLIBCXX_HAVE_POSIX_MEMALIGN
 static inline void*
 aligned_alloc (std::size_t al, std::size_t sz)
@@ -49,11 +64,6 @@ aligned_alloc (std::size_t al, std::size_t sz)
   return nullptr;
 }
 #elif _GLIBCXX_HAVE_MEMALIGN
-#if _GLIBCXX_HOSTED
-#include <malloc.h>
-#else
-extern "C" void *memalign(std::size_t boundary, std::size_t size);
-#endif
 static inline void*
 aligned_alloc (std::size_t al, std::size_t sz)
 {
@@ -66,7 +76,6 @@ aligned_alloc (std::size_t al, std::size_t sz)
   return memalign (al, sz);
 }
 #else // !HAVE__ALIGNED_MALLOC && !HAVE_POSIX_MEMALIGN && !HAVE_MEMALIGN
-#include <stdint.h>
 // The C library doesn't provide any aligned allocation functions, define one.
 // This is a modified version of code from gcc/config/i386/gmm_malloc.h
 static inline void*
@@ -87,7 +96,7 @@ aligned_alloc (std::size_t al, std::size_t sz)
   return aligned_ptr;
 }
 #endif
-#endif
+} // namespace __gnu_cxx
 
 _GLIBCXX_WEAK_DEFINITION void *
 operator new (std::size_t sz, std::align_val_t al)
@@ -116,6 +125,7 @@ operator new (std::size_t sz, std::align_val_t al)
     sz += align - rem;
 #endif
 
+  using __gnu_cxx::aligned_alloc;
   while (__builtin_expect ((p = aligned_alloc (align, sz)) == 0, false))
     {
       new_handler handler = std::get_new_handler ();
Jonathan Wakely Aug. 8, 2018, 3:53 p.m. | #8
On 08/08/18 15:46 +0100, Jonathan Wakely wrote:
>On 08/08/18 15:33 +0100, Jonathan Wakely wrote:
>>On 08/08/18 16:22 +0200, Ulrich Weigand wrote:
>>>Jonathan Wakely wrote:
>>>
>>>>Aha, so newlib was using memalign previously:
>>>>
>>>>@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
>>>>#else
>>>>extern "C" void *memalign(std::size_t boundary, std::size_t size);
>>>>#endif
>>>>-#define aligned_alloc memalign
>>>
>>>Yes, exactly ... this commit introduced the regression.
>>>
>>>>OK, I've regressed the branches then - I'll fix that.
>>
>>This should fix it. I'll finish testing and commit it.
>>
>>Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
>>gcc-7-branch and gcc-8-branch, because changing newlib from using
>>memalign to aligned_alloc is safe.
>
>With the patch this time ...

Committed to trunk, gcc-8-branch and gcc-7-branch.

Rainer, please let me know if this still works on Solaris 10, thanks.
commit 4f61feff8ad3615c0b686deac3852d98734ef964
Author: redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Aug 8 15:16:43 2018 +0000

    Prevent internal aligned_alloc clashing with libc version
    
    If configure fails to detect aligned_alloc we will try to define our
    own in new_opa.cc but that could clash with the libcversion in
    <stdlib.h>. Use a namespace to keep them distinct.
    
            * libsupc++/new_opa.cc (aligned_alloc): Declare inside namespace to
            avoid clashing with an ::aligned_alloc function that was not detected
            by configure.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@263409 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/libsupc++/new_opa.cc b/libstdc++-v3/libsupc++/new_opa.cc
index 5be0cc2ca65..68eac5b8ceb 100644
--- a/libstdc++-v3/libsupc++/new_opa.cc
+++ b/libstdc++-v3/libsupc++/new_opa.cc
@@ -25,15 +25,30 @@
 
 #include <bits/c++config.h>
 #include <stdlib.h>
+#include <stdint.h>
 #include <bits/exception_defines.h>
 #include "new"
 
+#if !_GLIBCXX_HAVE_ALIGNED_ALLOC && !_GLIBCXX_HAVE__ALIGNED_MALLOC \
+  && !_GLIBCXX_HAVE_POSIX_MEMALIGN && _GLIBCXX_HAVE_MEMALIGN
+# if _GLIBCXX_HOSTED && __has_include(<malloc.h>)
+// Some C libraries declare memalign in <malloc.h>
+#  include <malloc.h>
+# else
+extern "C" void *memalign(std::size_t boundary, std::size_t size);
+# endif
+#endif
+
 using std::new_handler;
 using std::bad_alloc;
 
-#if !_GLIBCXX_HAVE_ALIGNED_ALLOC
-#if _GLIBCXX_HAVE__ALIGNED_MALLOC
-#define aligned_alloc(al,sz) _aligned_malloc(sz,al)
+namespace __gnu_cxx {
+#if _GLIBCXX_HAVE_ALIGNED_ALLOC
+using ::aligned_alloc;
+#elif _GLIBCXX_HAVE__ALIGNED_MALLOC
+static inline void*
+aligned_alloc (std::size_t al, std::size_t sz)
+{ return _aligned_malloc(sz, al); }
 #elif _GLIBCXX_HAVE_POSIX_MEMALIGN
 static inline void*
 aligned_alloc (std::size_t al, std::size_t sz)
@@ -49,11 +64,6 @@ aligned_alloc (std::size_t al, std::size_t sz)
   return nullptr;
 }
 #elif _GLIBCXX_HAVE_MEMALIGN
-#if _GLIBCXX_HOSTED
-#include <malloc.h>
-#else
-extern "C" void *memalign(std::size_t boundary, std::size_t size);
-#endif
 static inline void*
 aligned_alloc (std::size_t al, std::size_t sz)
 {
@@ -66,7 +76,6 @@ aligned_alloc (std::size_t al, std::size_t sz)
   return memalign (al, sz);
 }
 #else // !HAVE__ALIGNED_MALLOC && !HAVE_POSIX_MEMALIGN && !HAVE_MEMALIGN
-#include <stdint.h>
 // The C library doesn't provide any aligned allocation functions, define one.
 // This is a modified version of code from gcc/config/i386/gmm_malloc.h
 static inline void*
@@ -87,7 +96,7 @@ aligned_alloc (std::size_t al, std::size_t sz)
   return aligned_ptr;
 }
 #endif
-#endif
+} // namespace __gnu_cxx
 
 _GLIBCXX_WEAK_DEFINITION void *
 operator new (std::size_t sz, std::align_val_t al)
@@ -116,6 +125,7 @@ operator new (std::size_t sz, std::align_val_t al)
     sz += align - rem;
 #endif
 
+  using __gnu_cxx::aligned_alloc;
   while (__builtin_expect ((p = aligned_alloc (align, sz)) == 0, false))
     {
       new_handler handler = std::get_new_handler ();
Sebastian Huber Aug. 9, 2018, 4:56 a.m. | #9
On 08/08/18 16:33, Jonathan Wakely wrote:
> On 08/08/18 16:22 +0200, Ulrich Weigand wrote:
>> Jonathan Wakely wrote:
>>
>>> Aha, so newlib was using memalign previously:
>>>
>>> @@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
>>>  #else
>>>  extern "C" void *memalign(std::size_t boundary, std::size_t size);
>>>  #endif
>>> -#define aligned_alloc memalign
>>
>> Yes, exactly ... this commit introduced the regression.
>>
>>> OK, I've regressed the branches then - I'll fix that.
>
> This should fix it. I'll finish testing and commit it.
>
> Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
> gcc-7-branch and gcc-8-branch, because changing newlib from using
> memalign to aligned_alloc is safe. 

Should I check in my patch in addition to your patch?
Jonathan Wakely Aug. 9, 2018, 9:08 a.m. | #10
On 09/08/18 06:56 +0200, Sebastian Huber wrote:
>On 08/08/18 16:33, Jonathan Wakely wrote:
>>On 08/08/18 16:22 +0200, Ulrich Weigand wrote:
>>>Jonathan Wakely wrote:
>>>
>>>>Aha, so newlib was using memalign previously:
>>>>
>>>>@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
>>>> #else
>>>> extern "C" void *memalign(std::size_t boundary, std::size_t size);
>>>> #endif
>>>>-#define aligned_alloc memalign
>>>
>>>Yes, exactly ... this commit introduced the regression.
>>>
>>>>OK, I've regressed the branches then - I'll fix that.
>>
>>This should fix it. I'll finish testing and commit it.
>>
>>Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
>>gcc-7-branch and gcc-8-branch, because changing newlib from using
>>memalign to aligned_alloc is safe.
>
>Should I check in my patch in addition to your patch?

Yes please, on trunk and 7 and 8. It's better to use the standard
aligned_alloc if available.
Rainer Orth Aug. 9, 2018, 10:53 a.m. | #11
Hi Jonathan,

> Committed to trunk, gcc-8-branch and gcc-7-branch.
>
> Rainer, please let me know if this still works on Solaris 10, thanks.

I ran i386-pc-solaris2.1[01] and sparc-sun-solaris2.1[01] bootstraps
last night: no regressions.

Thanks.
        Rainer
Szabolcs Nagy Aug. 13, 2018, 11:55 a.m. | #12
On 09/08/18 10:08, Jonathan Wakely wrote:
> On 09/08/18 06:56 +0200, Sebastian Huber wrote:
>> On 08/08/18 16:33, Jonathan Wakely wrote:
>>> On 08/08/18 16:22 +0200, Ulrich Weigand wrote:
>>>> Jonathan Wakely wrote:
>>>>
>>>>> Aha, so newlib was using memalign previously:
>>>>>
>>>>> @@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
>>>>>  #else
>>>>>  extern "C" void *memalign(std::size_t boundary, std::size_t size);
>>>>>  #endif
>>>>> -#define aligned_alloc memalign
>>>>
>>>> Yes, exactly ... this commit introduced the regression.
>>>>
>>>>> OK, I've regressed the branches then - I'll fix that.
>>>
>>> This should fix it. I'll finish testing and commit it.
>>>
>>> Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
>>> gcc-7-branch and gcc-8-branch, because changing newlib from using
>>> memalign to aligned_alloc is safe.
>>
>> Should I check in my patch in addition to your patch?
> 
> Yes please, on trunk and 7 and 8. It's better to use the standard
> aligned_alloc if available.
> 

but the newlib aligned_alloc is broken on baremetal targets,
it is implemented using posix_memalign which is not provided
by the newlib malloc implementation (except on cygwin)

in libstdc++ test log i see

spawn /B/gcc/xg++ -shared-libgcc -B/B/gcc -nostdinc++ -L/B/aarch64-none-elf/libstdc++-v3/src -L/B/aarch64-none-elf/libstdc++-v3/src/.libs 
-L/B/aarch64-none-elf/libstdc++-v3/libsupc++/.libs -B/P/aarch64-none-elf/bin/ -B/P/aarch64-none-elf/lib/ -isystem /P/aarch64-none-elf/include 
-isystem /P/aarch64-none-elf/sys-include -B/B/aarch64-none-elf/./libstdc++-v3/src/.libs -fmessage-length=0 -fno-show-column -ffunction-sections 
-fdata-sections -g -ffunction-sections -fdata-sections -O2 -DLOCALEDIR="." -nostdinc++ 
-I/B/aarch64-none-elf/libstdc++-v3/include/aarch64-none-elf -I/B/aarch64-none-elf/libstdc++-v3/include -I/S/gcc/libstdc++-v3/libsupc++ 
-I/S/gcc/libstdc++-v3/include/backward -I/S/gcc/libstdc++-v3/testsuite/util 
/S/gcc/libstdc++-v3/testsuite/18_support/aligned_alloc/aligned_alloc.cc -std=gnu++17 -fno-diagnostics-show-caret -fdiagnostics-color=never 
./libtestc++.a -L/B/aarch64-none-elf/libstdc++-v3/src/filesystem/.libs -specs=aem-ve.specs -lm -mcmodel=small -o ./aligned_alloc.exe
/P/aarch64-none-elf/bin/ld: /P/aarch64-none-elf/lib/libg.a(lib_a-aligned_alloc.o): in function `aligned_alloc':
/S/newlib-cygwin/newlib/libc/stdlib/aligned_alloc.c:35: undefined reference to `posix_memalign'
/P/aarch64-none-elf/bin/ld: /S/newlib-cygwin/newlib/libc/stdlib/aligned_alloc.c:35:(.text.aligned_alloc+0x14): relocation truncated to fit: 
R_AARCH64_CALL26 against undefined symbol `posix_memalign'
collect2: error: ld returned 1 exit status


FAIL: 18_support/aligned_alloc/aligned_alloc.cc (test for excess errors)
FAIL: 18_support/new_aligned.cc (test for excess errors)
FAIL: 20_util/allocator/overaligned.cc (test for excess errors)
FAIL: 20_util/memory_resource/2.cc (test for excess errors)
FAIL: 20_util/monotonic_buffer_resource/allocate.cc (test for excess errors)
FAIL: 20_util/monotonic_buffer_resource/deallocate.cc (test for excess errors)
FAIL: 20_util/monotonic_buffer_resource/release.cc (test for excess errors)
FAIL: 20_util/monotonic_buffer_resource/upstream_resource.cc (test for excess errors)
FAIL: 20_util/polymorphic_allocator/resource.cc (test for excess errors)
FAIL: 20_util/polymorphic_allocator/select.cc (test for excess errors)
FAIL: ext/bitmap_allocator/overaligned.cc (test for excess errors)
FAIL: ext/mt_allocator/overaligned.cc (test for excess errors)
FAIL: ext/new_allocator/overaligned.cc (test for excess errors)
FAIL: ext/pool_allocator/overaligned.cc (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new1.C   (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new2.C   (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new6.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new6.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new8.C   (test for excess errors)
UNRESOLVED: 18_support/aligned_alloc/aligned_alloc.cc compilation failed to produce executable
UNRESOLVED: 18_support/new_aligned.cc compilation failed to produce executable
UNRESOLVED: 20_util/allocator/overaligned.cc compilation failed to produce executable
UNRESOLVED: 20_util/memory_resource/2.cc compilation failed to produce executable
UNRESOLVED: 20_util/monotonic_buffer_resource/allocate.cc compilation failed to produce executable
UNRESOLVED: 20_util/monotonic_buffer_resource/deallocate.cc compilation failed to produce executable
UNRESOLVED: 20_util/monotonic_buffer_resource/release.cc compilation failed to produce executable
UNRESOLVED: 20_util/monotonic_buffer_resource/upstream_resource.cc compilation failed to produce executable
UNRESOLVED: 20_util/polymorphic_allocator/resource.cc compilation failed to produce executable
UNRESOLVED: 20_util/polymorphic_allocator/select.cc compilation failed to produce executable
UNRESOLVED: ext/bitmap_allocator/overaligned.cc compilation failed to produce executable
UNRESOLVED: ext/mt_allocator/overaligned.cc compilation failed to produce executable
UNRESOLVED: ext/new_allocator/overaligned.cc compilation failed to produce executable
UNRESOLVED: ext/pool_allocator/overaligned.cc compilation failed to produce executable
UNRESOLVED: g++.dg/cpp1z/aligned-new1.C   compilation failed to produce executable
UNRESOLVED: g++.dg/cpp1z/aligned-new2.C   compilation failed to produce executable
UNRESOLVED: g++.dg/cpp1z/aligned-new5.C  -std=gnu++11 compilation failed to produce executable
UNRESOLVED: g++.dg/cpp1z/aligned-new5.C  -std=gnu++14 compilation failed to produce executable
UNRESOLVED: g++.dg/cpp1z/aligned-new5.C  -std=gnu++98 compilation failed to produce executable
UNRESOLVED: g++.dg/cpp1z/aligned-new6.C  -std=gnu++11 compilation failed to produce executable
UNRESOLVED: g++.dg/cpp1z/aligned-new6.C  -std=gnu++14 compilation failed to produce executable
UNRESOLVED: g++.dg/cpp1z/aligned-new8.C   compilation failed to produce executable
Jonathan Wakely Aug. 13, 2018, 12:04 p.m. | #13
On 13/08/18 12:55 +0100, Szabolcs Nagy wrote:
>On 09/08/18 10:08, Jonathan Wakely wrote:
>>On 09/08/18 06:56 +0200, Sebastian Huber wrote:
>>>On 08/08/18 16:33, Jonathan Wakely wrote:
>>>>On 08/08/18 16:22 +0200, Ulrich Weigand wrote:
>>>>>Jonathan Wakely wrote:
>>>>>
>>>>>>Aha, so newlib was using memalign previously:
>>>>>>
>>>>>>@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
>>>>>> #else
>>>>>> extern "C" void *memalign(std::size_t boundary, std::size_t size);
>>>>>> #endif
>>>>>>-#define aligned_alloc memalign
>>>>>
>>>>>Yes, exactly ... this commit introduced the regression.
>>>>>
>>>>>>OK, I've regressed the branches then - I'll fix that.
>>>>
>>>>This should fix it. I'll finish testing and commit it.
>>>>
>>>>Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
>>>>gcc-7-branch and gcc-8-branch, because changing newlib from using
>>>>memalign to aligned_alloc is safe.
>>>
>>>Should I check in my patch in addition to your patch?
>>
>>Yes please, on trunk and 7 and 8. It's better to use the standard
>>aligned_alloc if available.
>>
>
>but the newlib aligned_alloc is broken on baremetal targets,
>it is implemented using posix_memalign which is not provided
>by the newlib malloc implementation (except on cygwin)

Ouch, OK, let's revert it. Using memalign is fine.

The original problem that I think Sebastian was trying to solve should
be fixed by r263409 anyway (and was backported to all branches).
Jonathan Wakely Aug. 13, 2018, 6:57 p.m. | #14
On 13/08/18 13:04 +0100, Jonathan Wakely wrote:
>On 13/08/18 12:55 +0100, Szabolcs Nagy wrote:
>>On 09/08/18 10:08, Jonathan Wakely wrote:
>>>On 09/08/18 06:56 +0200, Sebastian Huber wrote:
>>>>On 08/08/18 16:33, Jonathan Wakely wrote:
>>>>>On 08/08/18 16:22 +0200, Ulrich Weigand wrote:
>>>>>>Jonathan Wakely wrote:
>>>>>>
>>>>>>>Aha, so newlib was using memalign previously:
>>>>>>>
>>>>>>>@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
>>>>>>> #else
>>>>>>> extern "C" void *memalign(std::size_t boundary, std::size_t size);
>>>>>>> #endif
>>>>>>>-#define aligned_alloc memalign
>>>>>>
>>>>>>Yes, exactly ... this commit introduced the regression.
>>>>>>
>>>>>>>OK, I've regressed the branches then - I'll fix that.
>>>>>
>>>>>This should fix it. I'll finish testing and commit it.
>>>>>
>>>>>Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
>>>>>gcc-7-branch and gcc-8-branch, because changing newlib from using
>>>>>memalign to aligned_alloc is safe.
>>>>
>>>>Should I check in my patch in addition to your patch?
>>>
>>>Yes please, on trunk and 7 and 8. It's better to use the standard
>>>aligned_alloc if available.
>>>
>>
>>but the newlib aligned_alloc is broken on baremetal targets,
>>it is implemented using posix_memalign which is not provided
>>by the newlib malloc implementation (except on cygwin)
>
>Ouch, OK, let's revert it. Using memalign is fine.
>
>The original problem that I think Sebastian was trying to solve should
>be fixed by r263409 anyway (and was backported to all branches).

I've committed this to trunk and will do so on the branches too.
commit 6fe6d2c2256a73bca9ca3754761bd724b654db13
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 13 14:05:46 2018 +0100

    Revert "libstdc++-v3: Have aligned_alloc() on Newlib"
    
    This reverts commit r263461 / 2e920cd849b3cf0a72df4f172e27676a3e70b73f
    because aligned_alloc is not defined for baremetal newlib targets, see
    https://gcc.gnu.org/ml/libstdc++/2018-08/msg00065.html
    
    Revert
    2018-08-10  Sebastian Huber  <sebastian.huber@embedded-brains.de>
    
            PR target/85904
            * configure.ac: Define HAVE_ALIGNED_ALLOC if building for
            Newlib.
            * configure: Regenerate.

diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index e15228dde5e..332af3706d3 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -330,7 +330,6 @@ else
     AC_DEFINE(HAVE_TANF)
     AC_DEFINE(HAVE_TANHF)
 
-    AC_DEFINE(HAVE_ALIGNED_ALLOC)
     AC_DEFINE(HAVE_ICONV)
     AC_DEFINE(HAVE_MEMALIGN)
   else
Szabolcs Nagy Aug. 14, 2018, 11:32 a.m. | #15
On 13/08/18 19:57, Jonathan Wakely wrote:
> On 13/08/18 13:04 +0100, Jonathan Wakely wrote:
>> On 13/08/18 12:55 +0100, Szabolcs Nagy wrote:
>>> On 09/08/18 10:08, Jonathan Wakely wrote:
>>>> Yes please, on trunk and 7 and 8. It's better to use the standard
>>>> aligned_alloc if available.
>>>>
>>>
>>> but the newlib aligned_alloc is broken on baremetal targets,
>>> it is implemented using posix_memalign which is not provided
>>> by the newlib malloc implementation (except on cygwin)
>>
>> Ouch, OK, let's revert it. Using memalign is fine.
>>
>> The original problem that I think Sebastian was trying to solve should
>> be fixed by r263409 anyway (and was backported to all branches).
> 
> I've committed this to trunk and will do so on the branches too.
> 

thanks, this fixed the issue for me on trunk.

Patch

diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index d33081d544c..53803b7e599 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -28967,6 +28967,8 @@  else
     $as_echo "#define HAVE_TANHF 1" >>confdefs.h
 
 
+    $as_echo "#define HAVE_ALIGNED_ALLOC 1" >>confdefs.h
+
     $as_echo "#define HAVE_ICONV 1" >>confdefs.h
 
     $as_echo "#define HAVE_MEMALIGN 1" >>confdefs.h
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 332af3706d3..e15228dde5e 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -330,6 +330,7 @@  else
     AC_DEFINE(HAVE_TANF)
     AC_DEFINE(HAVE_TANHF)
 
+    AC_DEFINE(HAVE_ALIGNED_ALLOC)
     AC_DEFINE(HAVE_ICONV)
     AC_DEFINE(HAVE_MEMALIGN)
   else