Re: RFA (libstdc++): PATCH to implement C++17 over-aligned new

[Jonathan Wakely <jwakely at redhat dot com>]

On 12/09/16 16:13 +0200, Rainer Orth wrote:
> Jason Merrill <jason@redhat.com> writes:
>
> > On Thu, Sep 8, 2016 at 7:06 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > On 08/09/16 09:10 +0200, Marc Glisse wrote:
> > > > Do we want a generic fallback implementation (similar to
> > > > gcc/config/i386/gmm_malloc.h)? A windows version with _aligned_malloc /
> > > > _aligned_free would also be possible.
> > >
> > > Making it work for MinGW would be nice.
> >
> > OK, this is what I'm checking in; could someone test it on MinGW?
>
> The new tests are failing in various ways on Solaris:
>
> * 32-bit:
>
> +FAIL: g++.dg/cpp1z/aligned-new1.C   (test for excess errors)
> +WARNING: g++.dg/cpp1z/aligned-new1.C   compilation failed to produce executable
> +FAIL: g++.dg/cpp1z/aligned-new2.C   (test for excess errors)
> +WARNING: g++.dg/cpp1z/aligned-new2.C   compilation failed to produce executable
> +FAIL: g++.dg/cpp1z/aligned-new3.C   (test for excess errors)
> +WARNING: g++.dg/cpp1z/aligned-new3.C   compilation failed to produce executable
> +FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++11 (test for excess errors)
> +WARNING: g++.dg/cpp1z/aligned-new5.C  -std=gnu++11 compilation failed to produc
> e executable
> +FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++14 (test for excess errors)
> +WARNING: g++.dg/cpp1z/aligned-new5.C  -std=gnu++14 compilation failed to produc
> e executable
> +FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++98 (test for excess errors)
> +WARNING: g++.dg/cpp1z/aligned-new5.C  -std=gnu++98 compilation failed to produc
> e executable
>
>  All instances of
>
> Excess errors:
> Undefined                       first referenced
> symbol                             in file
> operator new(unsigned int, std::align_val_t) /var/tmp//cc_0Nrkd.o
> ld: fatal: symbol referencing errors
>
>  libsupc++/new_opa.o contains
>
>  _ZnwjSt11align_val_t (operator new(unsigned int, std::align_val_t))
>
>  while for 64-bit there is
>
>  _ZnwmSt11align_val_t (operator new(unsigned long, std::align_val_t))
>
>  The former isn't matched by config/abi/pre/gnu.ver
>
>    # C++17 aligned new/delete
>    _ZnwmSt11align_val_t;
>    _ZnwmSt11align_val_tRKSt9nothrow_t;
>    _ZnamSt11align_val_t;
>    _ZnamSt11align_val_tRKSt9nothrow_t;
>
>  I strongly suspects this needs to be
>
>  _Znw[jmy]* just as for regular new/delete.

Yes, that's right. Patch approved if you want to change that (I won't
be able to until tomorrow).

> * For 64-bit, I get
>
> +FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++11 execution test
> +FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++14 execution test
> +FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++98 execution test
>
>  which fails like this:
>
> terminate called after throwing an instance of 'std::bad_alloc'
>  what():  std::bad_alloc
>
>  gdb shows
>
> #7  0xffff80ff1d104bdc in __cxxabiv1::__cxa_throw (obj=<optimized out>,
>    tinfo=0xffff80ff1d2d0c98 <typeinfo for std::bad_alloc>,
>    dest=0xffff80ff1d1028f0 <std::bad_alloc::~bad_alloc()>)
>    at /vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++/eh_throw.cc:96
> #8  0xffff80ff1d10604c in operator new (sz=4, al=(unknown: 64))
>    at /vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++/new_opa.cc:71
> #9  0x00000000004010df in main ()
>    at /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/cpp1z/aligned-new5.C:11
>
>  and aligned_alloc(3C) documents
>
>       The  value of alignment must be a valid alignment supported by the sys-
>       tem, that is, any power of two (1, 2, 4, 8, ...), and the value of size
>       must be an integral multiple of alignment.
>
>  which isn't the case here.

Ah, it seems GNU's aligned_alloc doesn't check that requirement. So we
need to increase the requested size, maybe something like this patch.



diff --git a/libstdc++-v3/libsupc++/new_opa.cc b/libstdc++-v3/libsupc++/new_opa.cc
index 6ff5421..f2c0fdb 100644
--- a/libstdc++-v3/libsupc++/new_opa.cc
+++ b/libstdc++-v3/libsupc++/new_opa.cc
@@ -58,12 +58,15 @@ _GLIBCXX_WEAK_DEFINITION void *
 operator new (std::size_t sz, std::align_val_t al)
 {
   void *p;
+  std::size_t align = (std::size_t)al;
 
   /* malloc (0) is unpredictable; avoid it.  */
   if (sz == 0)
-    sz = 1;
+    sz = align;
+  if (std::size_t rem = sz % align)
+    sz += align - rem;
 
-  while (__builtin_expect ((p = aligned_alloc ((std::size_t)al, sz)) == 0,
+  while (__builtin_expect ((p = aligned_alloc (align, sz)) == 0,
 			   false))
     {
       new_handler handler = std::get_new_handler ();