This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.
commit a3b99f1671a3a1989a6d2d90162d33365a409c02
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jan 26 10:52:52 2017 +0000

    PR libstdc++/79190 add fallback aligned_alloc implementation
    
    libstdc++-v3:
    
    	PR libstdc++/79190
    	* libsupc++/del_opa.cc (operator delete(void*, std::align_val_t))
    	[!_GLIBCXX_HAVE_ALIGNED_ALLOC && !_GLIBCXX_HAVE_POSIX_MEMALIGN
    	&& !_GLIBCXX_HAVE_MEMALIGN && !_GLIBCXX_HAVE__ALIGNED_MALLOC]:
    	Retrieve original pointer value allocated by malloc.
    	* libsupc++/new_opa.cc [!_GLIBCXX_HAVE_ALIGNED_ALLOC
    	&& !_GLIBCXX_HAVE_POSIX_MEMALIGN && !_GLIBCXX_HAVE_MEMALIGN
    	&& !_GLIBCXX_HAVE__ALIGNED_MALLOC] (aligned_alloc(size_t, size_t)):
    	Define, adjusting pointer value allocated by malloc and storing for
    	retrieval by operator delete.
    
    gcc/testsuite:
    
    	PR libstdc++/79190
    	* g++.dg/cpp1z/aligned-new3.C: Replace operator new so behaviour
    	matches replaced operator delete.

diff --git a/gcc/testsuite/g++.dg/cpp1z/aligned-new3.C b/gcc/testsuite/g++.dg/cpp1z/aligned-new3.C
index 73e3343..e50e62c 100644
--- a/gcc/testsuite/g++.dg/cpp1z/aligned-new3.C
+++ b/gcc/testsuite/g++.dg/cpp1z/aligned-new3.C
@@ -7,6 +7,11 @@ struct alignas(64) A {
   int i;
 };
 
+void* operator new (std::size_t n, std::align_val_t)
+{
+  return operator new (n);
+}
+
 bool deleted = false;
 void operator delete (void *p, std::size_t, std::align_val_t)
 {
diff --git a/libstdc++-v3/libsupc++/del_opa.cc b/libstdc++-v3/libsupc++/del_opa.cc
index c9afb46..f7bf7a4 100644
--- a/libstdc++-v3/libsupc++/del_opa.cc
+++ b/libstdc++-v3/libsupc++/del_opa.cc
@@ -46,9 +46,13 @@ _GLIBCXX_END_NAMESPACE_VERSION
 _GLIBCXX_WEAK_DEFINITION void
 operator delete(void* ptr, std::align_val_t) _GLIBCXX_USE_NOEXCEPT
 {
-#if !_GLIBCXX_HAVE_ALIGNED_ALLOC && _GLIBCXX_HAVE__ALIGNED_MALLOC
+#if _GLIBCXX_HAVE_ALIGNED_ALLOC || _GLIBCXX_HAVE_POSIX_MEMALIGN \
+    || _GLIBCXX_HAVE_MEMALIGN
+  std::free (ptr);
+#elif _GLIBCXX_HAVE__ALIGNED_MALLOC
   _aligned_free (ptr);
 #else
-  std::free(ptr);
+  if (ptr)
+    std::free (((void **) ptr)[-1]); // See aligned_alloc in new_opa.cc
 #endif
 }
diff --git a/libstdc++-v3/libsupc++/new_opa.cc b/libstdc++-v3/libsupc++/new_opa.cc
index 0bc2b5f..e2a0d2f 100644
--- a/libstdc++-v3/libsupc++/new_opa.cc
+++ b/libstdc++-v3/libsupc++/new_opa.cc
@@ -55,9 +55,32 @@ 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);
+#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*
+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 *) (((uintptr_t) malloc_ptr + al) & -al);
+
+  // Store the original malloc value where it can be found by operator delete.
+  ((void **) aligned_ptr)[-1] = malloc_ptr;
+
+  return aligned_ptr;
+}
 #endif
 #endif
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]