[libc++] Factor out common logic for calling aligned allocation

[ldionne]

Summary

There were a couple of places where we needed to call the underlying
platform's aligned allocation/deallocation function. Instead of having
the same logic all over the place, extract the logic into a pair of
helper functions libcpp_aligned_alloc and libcpp_aligned_free.

The code in libcxxabi/src/fallback_malloc.cpp looks like it could be
simplified after this change -- I purposefully did not simplify it
further to keep this change as straightforward as possible, since it
is touching very important parts of the library.

Also, the changes in libcxx/src/new.cpp and libcxxabi/src/stdlib_new_delete.cpp
are basically the same -- I just kept both source files in sync.

The underlying reason for this refactoring is to make it easier to support
platforms that provide aligned allocation through C11's aligned_alloc
function instead of posix_memalign. After this change, we'll only have
to add support for that in a single place.

[ldionne]

Guard with !defined(_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION)

[dmajor]

Our mingw builds are failing with error: no member named '_aligned_malloc' in the global namespace. Should the new header have an #include <malloc.h> somewhere?

[ldionne]

In D91379#2423372, @dmajor wrote:

Our mingw builds are failing with error: no member named '_aligned_malloc' in the global namespace. Should the new header have an #include <malloc.h> somewhere?

That's weird, as we were not including <malloc.h> anywhere before either. Can you confirm that including <malloc.h> fixes your issue? I'm wary of adding non standard includes in our headers, since it causes all kinds of problems.

[dmajor]

In D91379#2426975, @ldionne wrote:

In D91379#2423372, @dmajor wrote:

Our mingw builds are failing with error: no member named '_aligned_malloc' in the global namespace. Should the new header have an #include <malloc.h> somewhere?

That's weird, as we were not including <malloc.h> anywhere before either. Can you confirm that including <malloc.h> fixes your issue? I'm wary of adding non standard includes in our headers, since it causes all kinds of problems.

That didn't fix our issue. On closer inspection I think this is a weirdness in the mingw headers but I could use a double check: https://github.com/mirror/mingw-w64/search?q=_MM_MALLOC_H_INCLUDED

_aligned_malloc can be provided by either <malloc.h> or <stdlib.h>, as long as you haven't defined _MM_MALLOC_H_INCLUDED. Previously, new.cpp worked because it included <stdlib.h> first thing. Now, in our build, translation units that first include <intrin.h> will set _MM_MALLOC_H_INCLUDED which prevents anything later from picking up _aligned_malloc. Am I interpreting that correctly? This trickery in <intrin.h> seems very strange...

[mstorsjo]

In D91379#2427555, @dmajor wrote:

That didn't fix our issue. On closer inspection I think this is a weirdness in the mingw headers but I could use a double check: https://github.com/mirror/mingw-w64/search?q=_MM_MALLOC_H_INCLUDED

_aligned_malloc can be provided by either <malloc.h> or <stdlib.h>, as long as you haven't defined _MM_MALLOC_H_INCLUDED. Previously, new.cpp worked because it included <stdlib.h> first thing. Now, in our build, translation units that first include <intrin.h> will set _MM_MALLOC_H_INCLUDED which prevents anything later from picking up _aligned_malloc. Am I interpreting that correctly? This trickery in <intrin.h> seems very strange...

I haven't run into this issue - how do you end up getting intrin.h included before the other includes here?

As for the mingw headers, yeah that aspect with _mm_malloc vs _aligned_malloc does seem like a bit of a mess - digging through the history finds commits from 2009 and 2010 that touched that, but I can't entirely make out what they attempted to do... I'll see if I can manage to fix that (figure out exactly what it did attempt to do back in 2010, and straighten out a bit of it). Anything relating to intrin.h is a huge mess though, as it is pretty tightly coupled with the compiler and interacts with compiler provided headers.

[dmajor]

In D91379#2427960, @mstorsjo wrote:

I haven't run into this issue - how do you end up getting intrin.h included before the other includes here?

Any case with intrin.h coming first should do it:

$ cat test.cpp
#include <intrin.h>
#include <new>

$ ./clang/bin/x86_64-w64-mingw32-clang++ -c test.cpp 
In file included from test.cpp:2:
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:314:12: error: no member named '_aligned_malloc' in the global namespace
  return ::_aligned_malloc(__size, __alignment);
         ~~^
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:326:5: error: no type named '_aligned_free' in the global namespace
  ::_aligned_free(__ptr);
  ~~^
2 errors generated.

[mstorsjo]

In D91379#2428659, @dmajor wrote:

In D91379#2427960, @mstorsjo wrote:

I haven't run into this issue - how do you end up getting intrin.h included before the other includes here?

Any case with intrin.h coming first should do it:

$ cat test.cpp
#include <intrin.h>
#include <new>

$ ./clang/bin/x86_64-w64-mingw32-clang++ -c test.cpp 
In file included from test.cpp:2:
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:314:12: error: no member named '_aligned_malloc' in the global namespace
  return ::_aligned_malloc(__size, __alignment);
         ~~^
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:326:5: error: no type named '_aligned_free' in the global namespace
  ::_aligned_free(__ptr);
  ~~^
2 errors generated.

Ah, doh. Indeed, I can reproduce that. Apparently the code I build regularly doesn't happen to use that setup.

I'll look into fixing the mingw-w64 header bug.

[ldionne]

In D91379#2429302, @mstorsjo wrote:

In D91379#2428659, @dmajor wrote:

In D91379#2427960, @mstorsjo wrote:

I haven't run into this issue - how do you end up getting intrin.h included before the other includes here?

Any case with intrin.h coming first should do it:

$ cat test.cpp
#include <intrin.h>
#include <new>

$ ./clang/bin/x86_64-w64-mingw32-clang++ -c test.cpp 
In file included from test.cpp:2:
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:314:12: error: no member named '_aligned_malloc' in the global namespace
  return ::_aligned_malloc(__size, __alignment);
         ~~^
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:326:5: error: no type named '_aligned_free' in the global namespace
  ::_aligned_free(__ptr);
  ~~^
2 errors generated.

Ah, doh. Indeed, I can reproduce that. Apparently the code I build regularly doesn't happen to use that setup.

I'll look into fixing the mingw-w64 header bug.

Actually, is there any chance that mingw could implement aligned_alloc from C11? This would increase their level of conformance, and solve this issue at the same time by providing a canonical, standardized name to do aligned allocation.

[mstorsjo]

In D91379#2429304, @ldionne wrote:

Actually, is there any chance that mingw could implement aligned_alloc from C11? This would increase their level of conformance, and solve this issue at the same time by providing a canonical, standardized name to do aligned allocation.

Hmm, maybe... But it's hard for a public header to use it unless we drop support for older versions of the mingw headers. Which might of course be pretty doable (as it doesn't require any newer version of windows, just a newer toolchain).

[mstorsjo]

In D91379#2429302, @mstorsjo wrote:

In D91379#2428659, @dmajor wrote:

In D91379#2427960, @mstorsjo wrote:

I haven't run into this issue - how do you end up getting intrin.h included before the other includes here?

Any case with intrin.h coming first should do it:

$ cat test.cpp
#include <intrin.h>
#include <new>

$ ./clang/bin/x86_64-w64-mingw32-clang++ -c test.cpp 
In file included from test.cpp:2:
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:314:12: error: no member named '_aligned_malloc' in the global namespace
  return ::_aligned_malloc(__size, __alignment);
         ~~^
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:326:5: error: no type named '_aligned_free' in the global namespace
  ::_aligned_free(__ptr);
  ~~^
2 errors generated.

Ah, doh. Indeed, I can reproduce that. Apparently the code I build regularly doesn't happen to use that setup.

I'll look into fixing the mingw-w64 header bug.

See https://sourceforge.net/p/mingw-w64/mailman/message/37168701/ and https://sourceforge.net/p/mingw-w64/mailman/message/37168702/ for patches that should fix this. It turned out to be less obvious and a bit more convolved than it first seemed.

[mstorsjo]

In D91379#2433128, @mstorsjo wrote:

In D91379#2429302, @mstorsjo wrote:

In D91379#2428659, @dmajor wrote:

In D91379#2427960, @mstorsjo wrote:

I haven't run into this issue - how do you end up getting intrin.h included before the other includes here?

Any case with intrin.h coming first should do it:

$ cat test.cpp
#include <intrin.h>
#include <new>

$ ./clang/bin/x86_64-w64-mingw32-clang++ -c test.cpp 
In file included from test.cpp:2:
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:314:12: error: no member named '_aligned_malloc' in the global namespace
  return ::_aligned_malloc(__size, __alignment);
         ~~^
/home/vm/Desktop/mingwtest/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:326:5: error: no type named '_aligned_free' in the global namespace
  ::_aligned_free(__ptr);
  ~~^
2 errors generated.

Ah, doh. Indeed, I can reproduce that. Apparently the code I build regularly doesn't happen to use that setup.

I'll look into fixing the mingw-w64 header bug.

See https://sourceforge.net/p/mingw-w64/mailman/message/37168701/ and https://sourceforge.net/p/mingw-w64/mailman/message/37168702/ for patches that should fix this. It turned out to be less obvious and a bit more convolved than it first seemed.

A fix for this is pushed now in https://sourceforge.net/p/mingw-w64/mingw-w64/ci/660e09f3cb20f181b6d6435cb623d65a3922a063/.

[dmajor]

A fix for this is pushed now in https://sourceforge.net/p/mingw-w64/mingw-w64/ci/660e09f3cb20f181b6d6435cb623d65a3922a063/.

Confirmed that our mingw build flavors are successful at this revision. Thanks @mstorsjo !

[ldionne]

In D91379#2437420, @dmajor wrote:

A fix for this is pushed now in https://sourceforge.net/p/mingw-w64/mingw-w64/ci/660e09f3cb20f181b6d6435cb623d65a3922a063/.

Confirmed that our mingw build flavors are successful at this revision. Thanks @mstorsjo !

Thanks a lot folks! I think that's by far the best resolution.

libcxx/include/new

 void* operator new[](std::size_t size, void* ptr) noexcept;             // nodiscard in C++2a
 void  operator delete  (void* ptr, void*) noexcept;
 void  operator delete[](void* ptr, void*) noexcept;
 
 */
 
 #include <__config>
 #include <__availability>
+#include <cstddef>
+#include <cstdlib>
 #include <exception>
 #include <type_traits>
-#include <cstddef>
 #include <version>
-#ifdef _LIBCPP_NO_EXCEPTIONS
-#include <cstdlib>
-#endif
 
 #if defined(_LIBCPP_ABI_VCRUNTIME)
 #include <new.h>
 #endif
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
if (__is_overaligned_for_new(__align)) {
       const align_val_t __align_val = static_cast<align_val_t>(__align);
       return __libcpp_operator_delete(__ptr, __align_val);
     } else {
       return __libcpp_operator_delete(__ptr);
     }
 #endif
 }
 
+#if !defined(_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION)
+// Low-level helpers to call the aligned allocation and deallocation functions
+// on the target platform. This is used to implement libc++'s own memory
+// allocation routines -- if you need to allocate memory inside the library,
+// chances are that you want to use `__libcpp_allocate` instead.
+//
+// Returns the allocated memory, or `nullptr` on failure.
+inline _LIBCPP_INLINE_VISIBILITY
+void* __libcpp_aligned_alloc(std::size_t __alignment, std::size_t __size) {
+#if defined(_LIBCPP_MSVCRT_LIKE)
+  return ::_aligned_malloc(__size, __alignment);
+#else
+  void* __result = nullptr;
+  ::posix_memalign(&__result, __alignment, __size);
+  // If posix_memalign fails, __result is unmodified so we still return `nullptr`.
+  return __result;
+#endif
+}
+
+inline _LIBCPP_INLINE_VISIBILITY
+void __libcpp_aligned_free(void* __ptr) {
+#if defined(_LIBCPP_MSVCRT_LIKE)
+  ::_aligned_free(__ptr);
+#else
+  ::free(__ptr);
+#endif
+}
+#endif // !_LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+
+
 template <class _Tp>
 _LIBCPP_NODISCARD_AFTER_CXX17 inline
 _LIBCPP_CONSTEXPR _Tp* __launder(_Tp* __p) _NOEXCEPT
 {
     static_assert (!(is_function<_Tp>::value), "can't launder functions" );
     static_assert (!(is_same<void, typename remove_cv<_Tp>::type>::value), "can't launder cv-void" );
 #ifdef _LIBCPP_COMPILER_HAS_BUILTIN_LAUNDER
     return __builtin_launder(__p);

libcxx/src/new.cpp

 _LIBCPP_WEAK
 void *
 operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC
 {
     if (size == 0)
         size = 1;
     if (static_cast<size_t>(alignment) < sizeof(void*))
       alignment = std::align_val_t(sizeof(void*));
+
+    // Try allocating memory. If allocation fails and there is a new_handler,
+    // call it to try free up memory, and try again until it succeeds, or until
+    // the new_handler decides to terminate.
+    //
+    // If allocation fails and there is no new_handler, we throw bad_alloc
+    // (or return nullptr if exceptions are disabled).
     void* p;
-#if defined(_LIBCPP_MSVCRT_LIKE)
-    while ((p = _aligned_malloc(size, static_cast<size_t>(alignment))) == nullptr)
-#else
-    while (::posix_memalign(&p, static_cast<size_t>(alignment), size) != 0)
-#endif
+    while ((p = std::__libcpp_aligned_alloc(static_cast<std::size_t>(alignment), size)) == nullptr)
     {
-        // If posix_memalign fails and there is a new_handler,
-        // call it to try free up memory.
         std::new_handler nh = std::get_new_handler();
         if (nh)
             nh();
         else {
 #ifndef _LIBCPP_NO_EXCEPTIONS
             throw std::bad_alloc();
 #else
-            p = nullptr; // posix_memalign doesn't initialize 'p' on failure
             break;
 #endif
         }
     }
     return p;
 }
 
 _LIBCPP_WEAK

 #endif  // _LIBCPP_NO_EXCEPTIONS
     return p;
 }
 
 _LIBCPP_WEAK
 void
 operator delete(void* ptr, std::align_val_t) _NOEXCEPT
 {
-    if (ptr)
-#if defined(_LIBCPP_MSVCRT_LIKE)
-        ::_aligned_free(ptr);
-#else
-        ::free(ptr);
-#endif
+    if (ptr) {
+        std::__libcpp_aligned_free(ptr);
+    }
 }
 
 _LIBCPP_WEAK
 void
 operator delete(void* ptr, std::align_val_t alignment, const std::nothrow_t&) _NOEXCEPT
 {
     ::operator delete(ptr, alignment);
 }

libcxxabi/src/fallback_malloc.cpp

 #ifndef _LIBCXXABI_HAS_NO_THREADS
 #if defined(__ELF__) && defined(_LIBCXXABI_LINK_PTHREAD_LIB)
 #pragma comment(lib, "pthread")
 #endif
 #endif
 
 #include <stdlib.h> // for malloc, calloc, free
 #include <string.h> // for memset
+#include <new> // for std::__libcpp_aligned_{alloc,free}
 
 //  A small, simple heap manager based (loosely) on
 //  the startup heap manager from FreeBSD, optimized for space.
 //
 //  Manages a fixed-size memory pool, supports malloc and free only.
 //  No support for realloc.
 //
 //  Allocates chunks in multiples of four bytes, with a four byte header

 } // end unnamed namespace
 
 namespace __cxxabiv1 {
 
 struct __attribute__((aligned)) __aligned_type {};
 
 void* __aligned_malloc_with_fallback(size_t size) {
 #if defined(_WIN32)
-  if (void* dest = _aligned_malloc(size, alignof(__aligned_type)))
+  if (void* dest = std::__libcpp_aligned_alloc(alignof(__aligned_type), size))
     return dest;
 #elif defined(_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION)
   if (void* dest = ::malloc(size))
     return dest;
 #else
   if (size == 0)
     size = 1;
-  void* dest;
-  if (::posix_memalign(&dest, __alignof(__aligned_type), size) == 0)
+  if (void* dest = std::__libcpp_aligned_alloc(__alignof(__aligned_type), size))
     return dest;
 #endif
   return fallback_malloc(size);
 }
 
 void* __calloc_with_fallback(size_t count, size_t size) {
   void* ptr = ::calloc(count, size);
   if (NULL != ptr)
     return ptr;
   // if calloc fails, fall back to emergency stash
   ptr = fallback_malloc(size * count);
   if (NULL != ptr)
     ::memset(ptr, 0, size * count);
   return ptr;
 }
 
 void __aligned_free_with_fallback(void* ptr) {
   if (is_fallback_ptr(ptr))
     fallback_free(ptr);
   else {
-#if defined(_WIN32)
-    ::_aligned_free(ptr);
-#else
-    ::free(ptr);
-#endif
+    std::__libcpp_aligned_free(ptr);
   }
 }
 
 void __free_with_fallback(void* ptr) {
   if (is_fallback_ptr(ptr))
     fallback_free(ptr);
   else
     ::free(ptr);
 }
 
 } // namespace __cxxabiv1

libcxxabi/src/stdlib_new_delete.cpp

 _LIBCXXABI_WEAK
 void *
 operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC
 {
     if (size == 0)
         size = 1;
     if (static_cast<size_t>(alignment) < sizeof(void*))
       alignment = std::align_val_t(sizeof(void*));
+
+    // Try allocating memory. If allocation fails and there is a new_handler,
+    // call it to try free up memory, and try again until it succeeds, or until
+    // the new_handler decides to terminate.
+    //
+    // If allocation fails and there is no new_handler, we throw bad_alloc
+    // (or return nullptr if exceptions are disabled).
     void* p;
-#if defined(_LIBCPP_WIN32API)
-    while ((p = _aligned_malloc(size, static_cast<size_t>(alignment))) == nullptr)
-#else
-    while (::posix_memalign(&p, static_cast<size_t>(alignment), size) != 0)
-#endif
+    while ((p = std::__libcpp_aligned_alloc(static_cast<std::size_t>(alignment), size)) == nullptr)
     {
-        // If posix_memalign fails and there is a new_handler,
-        // call it to try free up memory.
         std::new_handler nh = std::get_new_handler();
         if (nh)
             nh();
         else {
 #ifndef _LIBCXXABI_NO_EXCEPTIONS
             throw std::bad_alloc();
 #else
-            p = nullptr; // posix_memalign doesn't initialize 'p' on failure
             break;
 #endif
         }
     }
     return p;
 }
 
 _LIBCXXABI_WEAK

 #endif  // _LIBCXXABI_NO_EXCEPTIONS
     return p;
 }
 
 _LIBCXXABI_WEAK
 void
 operator delete(void* ptr, std::align_val_t) _NOEXCEPT
 {
-    if (ptr)
-#if defined(_LIBCPP_WIN32API)
-        ::_aligned_free(ptr);
-#else
-        ::free(ptr);
-#endif
+    if (ptr) {
+        std::__libcpp_aligned_free(ptr);
+    }
 }
 
 _LIBCXXABI_WEAK
 void
 operator delete(void* ptr, std::align_val_t alignment, const std::nothrow_t&) _NOEXCEPT
 {
     ::operator delete(ptr, alignment);
 }