[PATCH 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258]

[Jonathan Wakely <jwakely@redhat.com>]

I'v refactored the code a bit, adding a new __get_temporary_buffer
helper, because I want to use that in <stacktrace> in [PATCH 2/2].

I considered making __return_temporary_buffer also work for the
non-sized case, using non-sized delete if __len == 0, and then making
std::return_temporary_buffer call __return_temporary_buffer(p, 0). But
that didn't really seem much simpler/clearer, so I didn't do it.

-- >8 --

This adds extended alignment support to std::get_temporary_buffer etc.
so that when std::stable_sort uses a temporary buffer it works for
overaligned types.

Also simplify the _Temporary_buffer type by using RAII for the
allocation, via a new data member. This simplifies the _Temporary_buffer
constructor and destructor by makingthem only responsible for
constructing and destroying the elements, not managing the memory.

libstdc++-v3/ChangeLog:

	PR libstdc++/105258
	* include/bits/stl_tempbuf.h (__detail::__get_temporary_buffer):
	New function to do allocation for get_temporary_buffer, with
	extended alignment support.
	(__detail::__return_temporary_buffer): Support extended
	alignment.
	(get_temporary_buffer): Use __get_temporary_buffer.
	(return_temporary_buffer): Support extended alignment. Add
	deprecated attribute.
	(_Temporary_buffer): Move allocation and deallocation into a
	subobject and remove try-catch block in constructor.
	(__uninitialized_construct_buf): Use argument deduction for
	value type.
	* testsuite/20_util/temporary_buffer.cc: Add dg-warning for new
	deprecated warning.
	* testsuite/25_algorithms/stable_sort/overaligned.cc: New test.
---
 libstdc++-v3/include/bits/stl_tempbuf.h       | 128 ++++++++++++------
 .../testsuite/20_util/temporary_buffer.cc     |   2 +-
 .../25_algorithms/stable_sort/overaligned.cc  |  29 ++++
 3 files changed, 113 insertions(+), 46 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/25_algorithms/stable_sort/overaligned.cc

diff --git a/libstdc++-v3/include/bits/stl_tempbuf.h b/libstdc++-v3/include/bits/stl_tempbuf.h
index 77b121460f9..018ee18de15 100644
--- a/libstdc++-v3/include/bits/stl_tempbuf.h
+++ b/libstdc++-v3/include/bits/stl_tempbuf.h
@@ -66,19 +66,55 @@ namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
+#if __has_builtin(__builtin_operator_new) >= 201802L
+# define _GLIBCXX_OPERATOR_NEW __builtin_operator_new
+# define _GLIBCXX_OPERATOR_DELETE __builtin_operator_delete
+#else
+# define _GLIBCXX_OPERATOR_NEW ::operator new
+# define _GLIBCXX_OPERATOR_DELETE ::operator delete
+#endif
+
   namespace __detail
   {
+    // Equivalent to std::get_temporary_buffer but won't return a smaller size.
+    // It either returns a buffer of __len elements, or a null pointer.
+    template<typename _Tp>
+      inline _Tp*
+      __get_temporary_buffer(ptrdiff_t __len) _GLIBCXX_NOTHROW
+      {
+#if __cpp_aligned_new
+	if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
+	  return (_Tp*) _GLIBCXX_OPERATOR_NEW(__len * sizeof(_Tp),
+					      align_val_t(alignof(_Tp)),
+					      nothrow_t());
+#endif
+	return (_Tp*) _GLIBCXX_OPERATOR_NEW(__len * sizeof(_Tp), nothrow_t());
+      }
+
+    // Equivalent to std::return_temporary_buffer but with a size argument.
+    // The size is the number of elements, not the number of bytes.
     template<typename _Tp>
       inline void
       __return_temporary_buffer(_Tp* __p,
 				size_t __len __attribute__((__unused__)))
       {
 #if __cpp_sized_deallocation
-	::operator delete(__p, __len * sizeof(_Tp));
+# define _GLIBCXX_SIZED_DEALLOC(T, p, n) (p), (n) * sizeof(T)
 #else
-	::operator delete(__p);
+# define _GLIBCXX_SIZED_DEALLOC(T, p, n) (p)
 #endif
+
+#if __cpp_aligned_new
+	if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
+	  {
+	    _GLIBCXX_OPERATOR_DELETE(_GLIBCXX_SIZED_DEALLOC(_Tp, __p, __len),
+				     align_val_t(alignof(_Tp)));
+	    return;
+	  }
+#endif
+	_GLIBCXX_OPERATOR_DELETE(_GLIBCXX_SIZED_DEALLOC(_Tp, __p, __len));
       }
+#undef _GLIBCXX_SIZED_DEALLOC
   }
 
   /**
@@ -90,7 +126,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    *
    *  This function tries to obtain storage for @c __len adjacent Tp
    *  objects.  The objects themselves are not constructed, of course.
-   *  A pair<> is returned containing <em>the buffer s address and
+   *  A pair<> is returned containing <em>the buffer's address and
    *  capacity (in the units of sizeof(_Tp)), or a pair of 0 values if
    *  no storage can be obtained.</em>  Note that the capacity obtained
    *  may be less than that requested if the memory is unavailable;
@@ -110,13 +146,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       while (__len > 0)
 	{
-	  _Tp* __tmp = static_cast<_Tp*>(::operator new(__len * sizeof(_Tp),
-							std::nothrow));
-	  if (__tmp != 0)
-	    return std::pair<_Tp*, ptrdiff_t>(__tmp, __len);
+	  if (_Tp* __tmp = __detail::__get_temporary_buffer<_Tp>(__len))
+	    return pair<_Tp*, ptrdiff_t>(__tmp, __len);
 	  __len = __len == 1 ? 0 : ((__len + 1) / 2);
 	}
-      return std::pair<_Tp*, ptrdiff_t>(static_cast<_Tp*>(0), 0);
+      return pair<_Tp*, ptrdiff_t>();
     }
 
   /**
@@ -127,9 +161,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    *  Frees the memory pointed to by __p.
    */
   template<typename _Tp>
+    _GLIBCXX17_DEPRECATED
     inline void
     return_temporary_buffer(_Tp* __p)
-    { ::operator delete(__p); }
+    {
+#if __cpp_aligned_new
+      if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
+	_GLIBCXX_OPERATOR_DELETE(__p, align_val_t(alignof(_Tp)));
+      else
+#endif
+      _GLIBCXX_OPERATOR_DELETE(__p);
+    }
+
+#undef _GLIBCXX_OPERATOR_DELETE
+#undef _GLIBCXX_OPERATOR_NEW
 
   /**
    *  This class is used in two places: stl_algo.h and ext/memory,
@@ -150,14 +195,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     protected:
       size_type  _M_original_len;
-      size_type  _M_len;
-      pointer    _M_buffer;
+      struct _Impl
+      {
+	explicit
+	_Impl(ptrdiff_t __original_len)
+	{
+	  pair<pointer, size_type> __p(
+	    std::get_temporary_buffer<value_type>(__original_len));
+	  _M_len = __p.second;
+	  _M_buffer = __p.first;
+	}
+
+	~_Impl()
+	{ std::__detail::__return_temporary_buffer(_M_buffer, _M_len); }
+
+	size_type  _M_len;
+	pointer    _M_buffer;
+      } _M_impl;
 
     public:
       /// As per Table mumble.
       size_type
       size() const
-      { return _M_len; }
+      { return _M_impl._M_len; }
 
       /// Returns the size requested by the constructor; may be >size().
       size_type
@@ -167,12 +227,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       /// As per Table mumble.
       iterator
       begin()
-      { return _M_buffer; }
+      { return _M_impl._M_buffer; }
 
       /// As per Table mumble.
       iterator
       end()
-      { return _M_buffer + _M_len; }
+      { return _M_impl._M_buffer + _M_impl._M_len; }
 
       /**
        * Constructs a temporary buffer of a size somewhere between
@@ -181,10 +241,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Temporary_buffer(_ForwardIterator __seed, size_type __original_len);
 
       ~_Temporary_buffer()
-      {
-	std::_Destroy(_M_buffer, _M_buffer + _M_len);
-	std::__detail::__return_temporary_buffer(_M_buffer, _M_len);
-      }
+      { std::_Destroy(_M_impl._M_buffer, _M_impl._M_buffer + _M_impl._M_len); }
 
     private:
       // Disable copy constructor and assignment operator.
@@ -203,7 +260,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         __ucr(_Pointer __first, _Pointer __last,
 	      _ForwardIterator __seed)
         {
-	  if (__first == __last)
+	  if (__builtin_expect(__first == __last, 0))
 	    return;
 
 	  _Pointer __cur = __first;
@@ -243,17 +300,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // the same value when the algorithm finishes, unless one of the
   // constructions throws.
   //
-  // Requirements: _Pointer::value_type(_Tp&&) is valid.
-  template<typename _Pointer, typename _ForwardIterator>
+  // Requirements:
+  // _Tp is move constructible and constructible from std::move(*__seed).
+  template<typename _Tp, typename _ForwardIterator>
     inline void
-    __uninitialized_construct_buf(_Pointer __first, _Pointer __last,
+    __uninitialized_construct_buf(_Tp* __first, _Tp* __last,
 				  _ForwardIterator __seed)
     {
-      typedef typename std::iterator_traits<_Pointer>::value_type
-	_ValueType;
-
       std::__uninitialized_construct_buf_dispatch<
-        __has_trivial_constructor(_ValueType)>::
+	__has_trivial_constructor(_Tp)>::
 	  __ucr(__first, __last, __seed);
     }
 
@@ -262,26 +317,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _ForwardIterator, typename _Tp>
     _Temporary_buffer<_ForwardIterator, _Tp>::
     _Temporary_buffer(_ForwardIterator __seed, size_type __original_len)
-    : _M_original_len(__original_len), _M_len(0), _M_buffer(0)
+    : _M_original_len(__original_len), _M_impl(__original_len)
     {
-      std::pair<pointer, size_type> __p(
-		std::get_temporary_buffer<value_type>(_M_original_len));
-
-      if (__p.first)
-	{
-	  __try
-	    {
-	      std::__uninitialized_construct_buf(__p.first, __p.first + __p.second,
-						 __seed);
-	      _M_buffer = __p.first;
-	      _M_len = __p.second;
-	    }
-	  __catch(...)
-	    {
-	      std::__detail::__return_temporary_buffer(__p.first, __p.second);
-	      __throw_exception_again;
-	    }
-	}
+      std::__uninitialized_construct_buf(begin(), end(), __seed);
     }
 #pragma GCC diagnostic pop
 
diff --git a/libstdc++-v3/testsuite/20_util/temporary_buffer.cc b/libstdc++-v3/testsuite/20_util/temporary_buffer.cc
index 155d19034e5..065739be29d 100644
--- a/libstdc++-v3/testsuite/20_util/temporary_buffer.cc
+++ b/libstdc++-v3/testsuite/20_util/temporary_buffer.cc
@@ -44,7 +44,7 @@ int main(void)
       VERIFY( results.first == 0 );
   }
 
-  std::return_temporary_buffer(results.first);
+  std::return_temporary_buffer(results.first); // { dg-warning "deprecated" "" { target c++17 } }
 
   return 0;
 }
diff --git a/libstdc++-v3/testsuite/25_algorithms/stable_sort/overaligned.cc b/libstdc++-v3/testsuite/25_algorithms/stable_sort/overaligned.cc
new file mode 100644
index 00000000000..3c200624617
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/stable_sort/overaligned.cc
@@ -0,0 +1,29 @@
+// { dg-do run { target c++17 } }
+// PR libstdc++/105258 std::get_temporary_buffer() does not respect alignment
+
+#include <algorithm>
+#include <cstdint>
+#include <testsuite_hooks.h>
+
+struct alignas(__STDCPP_DEFAULT_NEW_ALIGNMENT__ * 2) Overaligned
+{
+  ~Overaligned()
+  {
+    auto alignment = reinterpret_cast<std::uintptr_t>(this);
+    VERIFY( (alignment % alignof(Overaligned)) == 0 );
+  }
+
+  bool operator<(const Overaligned&) const { return false; }
+};
+
+void
+test_pr105258()
+{
+  Overaligned o[2];
+  std::stable_sort(o, o+2);
+}
+
+int main()
+{
+  test_pr105258();
+}
-- 
2.45.1