Re: [PATCH] libstdc++-v3: Have aligned_alloc() on Newlib

[Jonathan Wakely <jwakely at redhat dot com>]

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.