* [PATCH 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258]
@ 2024-06-01 10:25 Jonathan Wakely
2024-06-01 10:25 ` [PATCH 2/2] libstdc++: Reuse temporary buffer utils in <stacktrace> Jonathan Wakely
2024-06-01 11:19 ` [PATCH v2 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258] Jonathan Wakely
0 siblings, 2 replies; 8+ messages in thread
From: Jonathan Wakely @ 2024-06-01 10:25 UTC (permalink / raw)
To: libstdc++, gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] libstdc++: Reuse temporary buffer utils in <stacktrace>
2024-06-01 10:25 [PATCH 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258] Jonathan Wakely
@ 2024-06-01 10:25 ` Jonathan Wakely
2024-06-03 20:22 ` Jonathan Wakely
2024-06-01 11:19 ` [PATCH v2 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258] Jonathan Wakely
1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2024-06-01 10:25 UTC (permalink / raw)
To: libstdc++, gcc-patches
The non-throwing allocation logic in std::stacktrace duplicates the
logic in <bits/stl_tempbuf.h>, so we can just reuse those utilities.
libstdc++-v3/ChangeLog:
* include/std/stacktrace (basic_stacktrace::_Impl::_M_allocate):
Use __detail::__get_temporary_buffer.
(basic_stacktrace::_Impl::_M_deallocate): Use
__detail::__return_temporary_buffer.
---
libstdc++-v3/include/std/stacktrace | 29 ++++-------------------------
1 file changed, 4 insertions(+), 25 deletions(-)
diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index 962dbed7a41..e0a543920bc 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -45,6 +45,7 @@
#include <bits/stl_algo.h>
#include <bits/stl_iterator.h>
#include <bits/stl_uninitialized.h>
+#include <bits/stl_tempbuf.h> // __get_temporary_buffer
#include <ext/numeric_traits.h>
namespace std _GLIBCXX_VISIBILITY(default)
@@ -545,21 +546,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return std::min(__size_max, __alloc_max);
}
-#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
-
-#if __cpp_sized_deallocation
-# define _GLIBCXX_SIZED_DELETE(T, p, n) \
- _GLIBCXX_OPERATOR_DELETE((p), (n) * sizeof(T))
-#else
-# define _GLIBCXX_SIZED_DELETE(T, p, n) _GLIBCXX_OPERATOR_DELETE(p)
-#endif
-
// Precondition: _M_frames == nullptr && __n != 0
pointer
_M_allocate(allocator_type& __alloc, size_type __n) noexcept
@@ -570,11 +556,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
// For std::allocator we use nothrow-new directly so we
// don't need to handle bad_alloc exceptions.
- size_t __nb = __n * sizeof(value_type);
- void* const __p = _GLIBCXX_OPERATOR_NEW (__nb, nothrow_t{});
+ auto __p = __detail::__get_temporary_buffer<value_type>(__n);
if (__p == nullptr) [[unlikely]]
return nullptr;
- _M_frames = static_cast<pointer>(__p);
+ _M_frames = __p;
}
else
{
@@ -599,9 +584,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (_M_capacity)
{
if constexpr (is_same_v<allocator_type, allocator<value_type>>)
- _GLIBCXX_SIZED_DELETE(value_type,
- static_cast<void*>(_M_frames),
- _M_capacity);
+ __detail::__return_temporary_buffer(_M_frames, _M_capacity);
else
__alloc.deallocate(_M_frames, _M_capacity);
_M_frames = nullptr;
@@ -609,10 +592,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
}
-#undef _GLIBCXX_SIZED_DELETE
-#undef _GLIBCXX_OPERATOR_DELETE
-#undef _GLIBCXX_OPERATOR_NEW
-
// Precondition: __n <= _M_size
void
_M_resize(size_type __n, allocator_type& __alloc) noexcept
--
2.45.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258]
2024-06-01 10:25 [PATCH 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258] Jonathan Wakely
2024-06-01 10:25 ` [PATCH 2/2] libstdc++: Reuse temporary buffer utils in <stacktrace> Jonathan Wakely
@ 2024-06-01 11:19 ` Jonathan Wakely
2024-06-03 20:22 ` Jonathan Wakely
1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2024-06-01 11:19 UTC (permalink / raw)
To: libstdc++, gcc-patches
Although it's only used from places where we are allocating a sensible
size, __detail::__get_temporary_buffer should really still check that
len * sizeof(T) doesn't wrap around to zero and allocate a buffer that's
smaller than expected.
This v2 patch is the same as v1 except for adding this check:
inline _Tp*
__get_temporary_buffer(ptrdiff_t __len) _GLIBCXX_NOTHROW
{
+ if (__builtin_expect(__len > (size_t(-1) / sizeof(_Tp)), 0))
+ return 0;
+
#if __cpp_aligned_new
if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
return (_Tp*) _GLIBCXX_OPERATOR_NEW(__len * sizeof(_Tp),
-- >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 | 131 ++++++++++++------
.../testsuite/20_util/temporary_buffer.cc | 2 +-
.../25_algorithms/stable_sort/overaligned.cc | 29 ++++
3 files changed, 116 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..fa03fd27704 100644
--- a/libstdc++-v3/include/bits/stl_tempbuf.h
+++ b/libstdc++-v3/include/bits/stl_tempbuf.h
@@ -66,19 +66,58 @@ 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 (__builtin_expect(__len > (size_t(-1) / sizeof(_Tp)), 0))
+ return 0;
+
+#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 +129,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 +149,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 +164,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 +198,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 +230,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 +244,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 +263,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 +303,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 +320,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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258]
2024-06-01 11:19 ` [PATCH v2 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258] Jonathan Wakely
@ 2024-06-03 20:22 ` Jonathan Wakely
2024-06-18 18:04 ` Stephan Bergmann
0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2024-06-03 20:22 UTC (permalink / raw)
To: libstdc++, gcc-patches
On Sat, 1 Jun 2024 at 12:23, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> Although it's only used from places where we are allocating a sensible
> size, __detail::__get_temporary_buffer should really still check that
> len * sizeof(T) doesn't wrap around to zero and allocate a buffer that's
> smaller than expected.
>
> This v2 patch is the same as v1 except for adding this check:
>
> inline _Tp*
> __get_temporary_buffer(ptrdiff_t __len) _GLIBCXX_NOTHROW
> {
> + if (__builtin_expect(__len > (size_t(-1) / sizeof(_Tp)), 0))
> + return 0;
> +
> #if __cpp_aligned_new
> if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
> return (_Tp*) _GLIBCXX_OPERATOR_NEW(__len * sizeof(_Tp),
Pushed to trunk now.
> -- >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 | 131 ++++++++++++------
> .../testsuite/20_util/temporary_buffer.cc | 2 +-
> .../25_algorithms/stable_sort/overaligned.cc | 29 ++++
> 3 files changed, 116 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..fa03fd27704 100644
> --- a/libstdc++-v3/include/bits/stl_tempbuf.h
> +++ b/libstdc++-v3/include/bits/stl_tempbuf.h
> @@ -66,19 +66,58 @@ 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 (__builtin_expect(__len > (size_t(-1) / sizeof(_Tp)), 0))
> + return 0;
> +
> +#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 +129,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 +149,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 +164,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 +198,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 +230,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 +244,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 +263,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 +303,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 +320,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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libstdc++: Reuse temporary buffer utils in <stacktrace>
2024-06-01 10:25 ` [PATCH 2/2] libstdc++: Reuse temporary buffer utils in <stacktrace> Jonathan Wakely
@ 2024-06-03 20:22 ` Jonathan Wakely
0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2024-06-03 20:22 UTC (permalink / raw)
To: libstdc++, gcc-patches
On Sat, 1 Jun 2024 at 11:29, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> The non-throwing allocation logic in std::stacktrace duplicates the
> logic in <bits/stl_tempbuf.h>, so we can just reuse those utilities.
Pushed to trunk now.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/stacktrace (basic_stacktrace::_Impl::_M_allocate):
> Use __detail::__get_temporary_buffer.
> (basic_stacktrace::_Impl::_M_deallocate): Use
> __detail::__return_temporary_buffer.
> ---
> libstdc++-v3/include/std/stacktrace | 29 ++++-------------------------
> 1 file changed, 4 insertions(+), 25 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
> index 962dbed7a41..e0a543920bc 100644
> --- a/libstdc++-v3/include/std/stacktrace
> +++ b/libstdc++-v3/include/std/stacktrace
> @@ -45,6 +45,7 @@
> #include <bits/stl_algo.h>
> #include <bits/stl_iterator.h>
> #include <bits/stl_uninitialized.h>
> +#include <bits/stl_tempbuf.h> // __get_temporary_buffer
> #include <ext/numeric_traits.h>
>
> namespace std _GLIBCXX_VISIBILITY(default)
> @@ -545,21 +546,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> return std::min(__size_max, __alloc_max);
> }
>
> -#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
> -
> -#if __cpp_sized_deallocation
> -# define _GLIBCXX_SIZED_DELETE(T, p, n) \
> - _GLIBCXX_OPERATOR_DELETE((p), (n) * sizeof(T))
> -#else
> -# define _GLIBCXX_SIZED_DELETE(T, p, n) _GLIBCXX_OPERATOR_DELETE(p)
> -#endif
> -
> // Precondition: _M_frames == nullptr && __n != 0
> pointer
> _M_allocate(allocator_type& __alloc, size_type __n) noexcept
> @@ -570,11 +556,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> {
> // For std::allocator we use nothrow-new directly so we
> // don't need to handle bad_alloc exceptions.
> - size_t __nb = __n * sizeof(value_type);
> - void* const __p = _GLIBCXX_OPERATOR_NEW (__nb, nothrow_t{});
> + auto __p = __detail::__get_temporary_buffer<value_type>(__n);
> if (__p == nullptr) [[unlikely]]
> return nullptr;
> - _M_frames = static_cast<pointer>(__p);
> + _M_frames = __p;
> }
> else
> {
> @@ -599,9 +584,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> if (_M_capacity)
> {
> if constexpr (is_same_v<allocator_type, allocator<value_type>>)
> - _GLIBCXX_SIZED_DELETE(value_type,
> - static_cast<void*>(_M_frames),
> - _M_capacity);
> + __detail::__return_temporary_buffer(_M_frames, _M_capacity);
> else
> __alloc.deallocate(_M_frames, _M_capacity);
> _M_frames = nullptr;
> @@ -609,10 +592,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
> }
>
> -#undef _GLIBCXX_SIZED_DELETE
> -#undef _GLIBCXX_OPERATOR_DELETE
> -#undef _GLIBCXX_OPERATOR_NEW
> -
> // Precondition: __n <= _M_size
> void
> _M_resize(size_type __n, allocator_type& __alloc) noexcept
> --
> 2.45.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258]
2024-06-03 20:22 ` Jonathan Wakely
@ 2024-06-18 18:04 ` Stephan Bergmann
2024-06-18 19:38 ` Jonathan Wakely
0 siblings, 1 reply; 8+ messages in thread
From: Stephan Bergmann @ 2024-06-18 18:04 UTC (permalink / raw)
To: Jonathan Wakely, libstdc++, gcc-patches
On 6/3/24 22:22, Jonathan Wakely wrote:
> Pushed to trunk now.
Just a heads-up that this started to cause Clang (at least 18/19) to
emit a -Wdeprecated-declarations now,
> $ cat test.cc
> #include <algorithm>
> void f(int * p1, int * p2) { std::stable_sort(p1, p2); }
> $ clang++ --gcc-install-dir=/home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0 -fsyntax-only test.cc
> In file included from test.cc:1:
> In file included from /home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0/../../../../include/c++/15.0.0/algorithm:61:
> In file included from /home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0/../../../../include/c++/15.0.0/bits/stl_algo.h:69:
> /home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0/../../../../include/c++/15.0.0/bits/stl_tempbuf.h:207:11: warning: 'get_temporary_buffer<int>' is deprecated [-Wdeprecated-declarations]
> 207 | std::get_temporary_buffer<value_type>(__original_len));
> | ^
> /home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0/../../../../include/c++/15.0.0/bits/stl_tempbuf.h:323:40: note: in instantiation of member function 'std::_Temporary_buffer<__gnu_cxx::__normal_iterator<int *, std::vector<int>>, int>::_Impl::_Impl' requested here
> 323 | : _M_original_len(__original_len), _M_impl(__original_len)
> | ^
> /home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0/../../../../include/c++/15.0.0/bits/stl_algo.h:4948:15: note: in instantiation of member function 'std::_Temporary_buffer<__gnu_cxx::__normal_iterator<int *, std::vector<int>>, int>::_Temporary_buffer' requested here
> 4948 | _TmpBuf __buf(__first, (__last - __first + 1) / 2);
> | ^
> /home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0/../../../../include/c++/15.0.0/bits/stl_algo.h:4993:23: note: in instantiation of function template specialization 'std::__stable_sort<__gnu_cxx::__normal_iterator<int *, std::vector<int>>, __gnu_cxx::__ops::_Iter_less_iter>' requested here
> 4993 | _GLIBCXX_STD_A::__stable_sort(__first, __last,
> | ^
> test.cc:3:37: note: in instantiation of function template specialization 'std::stable_sort<__gnu_cxx::__normal_iterator<int *, std::vector<int>>>' requested here
> 3 | void f(std::vector<int> & v) { std::stable_sort(v.begin(), v.end()); }
> | ^
> /home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0/../../../../include/c++/15.0.0/bits/stl_tempbuf.h:141:5: note: 'get_temporary_buffer<int>' has been explicitly marked deprecated here
> 141 | _GLIBCXX17_DEPRECATED
> | ^
> /home/sberg/gcc/inst/lib/gcc/x86_64-pc-linux-gnu/15.0.0/../../../../include/c++/15.0.0/x86_64-pc-linux-gnu/bits/c++config.h:123:34: note: expanded from macro '_GLIBCXX17_DEPRECATED'
> 123 | # define _GLIBCXX17_DEPRECATED [[__deprecated__]]
> | ^
> 1 warning generated.
which could be silenced with
> --- a/libstdc++-v3/include/bits/stl_tempbuf.h
> +++ b/libstdc++-v3/include/bits/stl_tempbuf.h
> @@ -203,8 +203,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> explicit
> _Impl(ptrdiff_t __original_len)
> {
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> pair<pointer, size_type> __p(
> std::get_temporary_buffer<value_type>(__original_len));
> +#pragma GCC diagnostic pop
> _M_len = __p.second;
> _M_buffer = __p.first;
> }
(There already is another such pragma diagnostic ignored a bit further
down in that file, so I assume that's the way to go there?)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258]
2024-06-18 18:04 ` Stephan Bergmann
@ 2024-06-18 19:38 ` Jonathan Wakely
2024-06-19 13:08 ` [committed] libstdc++: Fix warning regressions in <bits/stl_tempbuf.h> Jonathan Wakely
0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2024-06-18 19:38 UTC (permalink / raw)
To: Stephan Bergmann; +Cc: Jonathan Wakely, libstdc++, gcc-patches
On Tue, 18 Jun 2024 at 19:05, Stephan Bergmann <sberg.fun@gmail.com> wrote:
>
> On 6/3/24 22:22, Jonathan Wakely wrote:
> > Pushed to trunk now.
>
> Just a heads-up that this started to cause Clang (at least 18/19) to
> emit a -Wdeprecated-declarations now,
Yes, I saw this too.
> (There already is another such pragma diagnostic ignored a bit further
> down in that file, so I assume that's the way to go there?)
Yes, the call to get_temporary_buffer used to be in that function
further down in the file. I moved it elsewhere in the file, but didn't
move the pragma to go with it, oops.
So we can remove those later pragmas, and move them earlier in the
file. I'm already testing a patch locally.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [committed] libstdc++: Fix warning regressions in <bits/stl_tempbuf.h>
2024-06-18 19:38 ` Jonathan Wakely
@ 2024-06-19 13:08 ` Jonathan Wakely
0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2024-06-19 13:08 UTC (permalink / raw)
To: libstdc++, gcc-patches; +Cc: Stephan Bergmann
Tested x86_64-linux. Pushed to trunk.
-- >8 --
I caused some new warnings with -Wsystem-headers with my recent changes
to std::get_temporary_buffer and std::_Temporary_buffer. There's a
-Wsign-compare warning which can be avoided by casting the ptrdiff_t
argument to size_t (which also conveniently rejects negative values).
There's also a -Wdeprecated-declarations warning because I moved where
std::get_temporary_buffer is called, but didn't move the diagnostic
pragmas that suppress the warning for calling it.
libstdc++-v3/ChangeLog:
* include/bits/stl_tempbuf.h (__get_temporary_buffer): Cast
argument to size_t to handle negative values and suppress
-Wsign-compare warning.
(_Temporary_buffer): Move diagnostic pragmas to new location of
call to std::get_temporary_buffer.
---
libstdc++-v3/include/bits/stl_tempbuf.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libstdc++-v3/include/bits/stl_tempbuf.h b/libstdc++-v3/include/bits/stl_tempbuf.h
index fa03fd27704..759c4937744 100644
--- a/libstdc++-v3/include/bits/stl_tempbuf.h
+++ b/libstdc++-v3/include/bits/stl_tempbuf.h
@@ -82,7 +82,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
inline _Tp*
__get_temporary_buffer(ptrdiff_t __len) _GLIBCXX_NOTHROW
{
- if (__builtin_expect(__len > (size_t(-1) / sizeof(_Tp)), 0))
+ if (__builtin_expect(size_t(__len) > (size_t(-1) / sizeof(_Tp)), 0))
return 0;
#if __cpp_aligned_new
@@ -200,6 +200,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
size_type _M_original_len;
struct _Impl
{
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
explicit
_Impl(ptrdiff_t __original_len)
{
@@ -208,6 +210,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_len = __p.second;
_M_buffer = __p.first;
}
+#pragma GCC diagnostic pop
~_Impl()
{ std::__detail::__return_temporary_buffer(_M_buffer, _M_len); }
@@ -315,8 +318,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__ucr(__first, __last, __seed);
}
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
template<typename _ForwardIterator, typename _Tp>
_Temporary_buffer<_ForwardIterator, _Tp>::
_Temporary_buffer(_ForwardIterator __seed, size_type __original_len)
@@ -324,7 +325,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
std::__uninitialized_construct_buf(begin(), end(), __seed);
}
-#pragma GCC diagnostic pop
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
--
2.45.1
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-06-19 13:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-01 10:25 [PATCH 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258] Jonathan Wakely
2024-06-01 10:25 ` [PATCH 2/2] libstdc++: Reuse temporary buffer utils in <stacktrace> Jonathan Wakely
2024-06-03 20:22 ` Jonathan Wakely
2024-06-01 11:19 ` [PATCH v2 1/2] libstdc++: Handle extended alignment in std::get_temporary_buffer [PR105258] Jonathan Wakely
2024-06-03 20:22 ` Jonathan Wakely
2024-06-18 18:04 ` Stephan Bergmann
2024-06-18 19:38 ` Jonathan Wakely
2024-06-19 13:08 ` [committed] libstdc++: Fix warning regressions in <bits/stl_tempbuf.h> Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).