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

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

On 16/09/16 09:04 +0200, Rainer Orth wrote:
>Hi Jason,
>
>> OK, one more:
>
>this works just fine on both sparc-sun-solaris2.12 and
>i386-pc-solaris2.12.
>
>Once Jonathan's patch to heed aligned_alloc's requirement on size being
>a multiple of alignment is in, all is fine on Solaris.

I've got a slightly different fix now.

We only need to make the size a multiple of alignment for
aligned_alloc, however for posix_memalign we need to ensure the
alignment is a multiple of sizeof(void*).

I'm testing this now (but only on x86_64 GNU/Linux where it wasn't
failing anyway).

Would using __builtin_expect (sz == 0, false) make sense?  Surely it's
rare to try to allocate zero bytes.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1623 bytes --]

commit 216b9547230295e615bab86aaede65554f63e57d
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Sep 16 09:54:51 2016 +0100

    Adjust arguments to aligned_alloc or posix_memalign
    
    	* libsupc++/new_opa.cc [_GLIBCXX_HAVE_POSIX_MEMALIGN] (aligned_alloc):
    	Increase alignment if less than sizeof(void*).
    	[_GLIBCXX_HAVE_ALIGNED_ALLOC] (operator new(size_t, align_val_t)):
    	Increase size if not a multiple of alignment.

diff --git a/libstdc++-v3/libsupc++/new_opa.cc b/libstdc++-v3/libsupc++/new_opa.cc
index 6ff5421..9c859c1 100644
--- a/libstdc++-v3/libsupc++/new_opa.cc
+++ b/libstdc++-v3/libsupc++/new_opa.cc
@@ -39,6 +39,9 @@ static inline void*
 aligned_alloc (std::size_t al, std::size_t sz)
 {
   void *ptr;
+  // The value of alignment shall be a power of two multiple of sizeof(void *).
+  if (al < sizeof(void*))
+    al = sizeof(void*);
   int ret = posix_memalign (&ptr, al, sz);
   if (ret == 0)
     return ptr;
@@ -58,13 +61,19 @@ _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;
 
-  while (__builtin_expect ((p = aligned_alloc ((std::size_t)al, sz)) == 0,
-			   false))
+#if _GLIBCXX_HAVE_ALIGNED_ALLOC
+  /* C11: the value of size shall be an integral multiple of alignment.  */
+  if (std::size_t rem = sz % align)
+    sz += align - rem;
+#endif
+
+  while (__builtin_expect ((p = aligned_alloc (align, sz)) == 0, false))
     {
       new_handler handler = std::get_new_handler ();
       if (! handler)