https://gcc.gnu.org/g:2ce99c0088ed97991f61cbdefa83f682c2ef4364
commit r15-6272-g2ce99c0088ed97991f61cbdefa83f682c2ef4364 Author: Jonathan Wakely <jwak...@redhat.com> Date: Sun Dec 8 14:34:01 2024 +0000 libstdc++: Fix fancy pointer support in linked lists [PR57272] The union members I used in the new _Node types for fancy pointers only work for value types that are trivially default constructible. This change replaces the anonymous union with a named union so it can be given a default constructor and destructor, to leave the variant member uninitialized. This also fixes the incorrect macro names in the alloc_ptr_ignored.cc tests as pointed out by François, and fixes some std::list pointer confusions that the fixed alloc_ptr_ignored.cc test revealed. libstdc++-v3/ChangeLog: PR libstdc++/57272 * include/bits/forward_list.h (__fwd_list::_Node): Add user-provided special member functions to union. * include/bits/stl_list.h (__list::_Node): Likewise. (_Node_base::_M_hook, _Node_base::swap): Use _M_base() instead of std::pointer_traits::pointer_to. (_Node_base::_M_transfer): Likewise. Add noexcept. (_List_base::_M_put_node): Use 'if constexpr' to avoid using pointer_traits::pointer_to when not necessary. (_List_base::_M_destroy_node): Fix parameter to be the pointer type used internally, not the allocator's pointer. (list::_M_create_node): Likewise. * testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc: Check explicit instantiation of non-trivial value type. * testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc: Likewise. * testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc: Fix macro name. * testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc: Likewise. Diff: --- libstdc++-v3/include/bits/forward_list.h | 15 ++-- libstdc++-v3/include/bits/stl_list.h | 92 +++++++++++++--------- .../explicit_instantiation/alloc_ptr.cc | 11 +++ .../explicit_instantiation/alloc_ptr_ignored.cc | 2 +- .../explicit_instantiation/alloc_ptr.cc | 11 +++ .../explicit_instantiation/alloc_ptr_ignored.cc | 2 +- 6 files changed, 88 insertions(+), 45 deletions(-) diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h index f4a53bb4d7ef..3070ef6b1a78 100644 --- a/libstdc++-v3/include/bits/forward_list.h +++ b/libstdc++-v3/include/bits/forward_list.h @@ -420,25 +420,30 @@ namespace __fwdlist using value_type = typename pointer_traits<_ValPtr>::element_type; using _Node_ptr = __ptr_rebind<_ValPtr, _Node>; - _Node() { } + _Node() noexcept { } ~_Node() { } _Node(_Node&&) = delete; - union { + union _Uninit_storage + { + _Uninit_storage() noexcept { } + ~_Uninit_storage() { } + #if ! _GLIBCXX_INLINE_VERSION // For ABI compatibility we need to overalign this member. alignas(__alignof__(value_type)) // XXX GLIBCXX_ABI Deprecated #endif value_type _M_data; }; + _Uninit_storage _M_u; value_type* _M_valptr() noexcept - { return std::__addressof(_M_data); } + { return std::__addressof(_M_u._M_data); } const value_type* _M_valptr() const noexcept - { return std::__addressof(_M_data); } + { return std::__addressof(_M_u._M_data); } _Node_ptr _M_node_ptr() @@ -486,7 +491,7 @@ namespace __fwdlist [[__nodiscard__]] constexpr reference operator*() const noexcept - { return static_cast<_Node&>(*this->_M_node)._M_data; } + { return static_cast<_Node&>(*this->_M_node)._M_u._M_data; } [[__nodiscard__]] constexpr pointer diff --git a/libstdc++-v3/include/bits/stl_list.h b/libstdc++-v3/include/bits/stl_list.h index 9f7d27242b60..03c4be79416f 100644 --- a/libstdc++-v3/include/bits/stl_list.h +++ b/libstdc++-v3/include/bits/stl_list.h @@ -199,12 +199,12 @@ namespace __list swap(_Node_base& __x, _Node_base& __y) noexcept; void - _M_transfer(_Base_ptr const __first, _Base_ptr const __last); + _M_transfer(_Base_ptr const __first, _Base_ptr const __last) noexcept; void _M_hook(_Base_ptr const __position) noexcept { - auto __self = pointer_traits<_Base_ptr>::pointer_to(*this); + auto __self = this->_M_base(); this->_M_next = __position; this->_M_prev = __position->_M_prev; __position->_M_prev->_M_next = __self; @@ -224,6 +224,8 @@ namespace __list // by std::list::empty(), where it doesn't escape, and by // std::list::end() const, where the pointer is used to initialize a // const_iterator and so constness is restored. + // The standard allows pointer_to to be potentially-throwing, + // but we have to assume it doesn't throw to implement std::list. _Base_ptr _M_base() const noexcept { @@ -290,16 +292,24 @@ namespace __list using value_type = typename pointer_traits<_ValPtr>::element_type; using _Node_ptr = __ptr_rebind<_ValPtr, _Node>; - union { + _Node() noexcept { } + ~_Node() { } + _Node(_Node&&) = delete; + + union _Uninit_storage + { + _Uninit_storage() noexcept { } + ~_Uninit_storage() { } + value_type _M_data; }; + _Uninit_storage _M_u; - _Node() { } - ~_Node() { } - _Node(_Node&&) = delete; + value_type* + _M_valptr() noexcept { return std::__addressof(_M_u._M_data); } - value_type* _M_valptr() { return std::__addressof(_M_data); } - value_type const* _M_valptr() const { return std::__addressof(_M_data); } + value_type const* + _M_valptr() const noexcept { return std::__addressof(_M_u._M_data); } _Node_ptr _M_node_ptr() @@ -349,7 +359,7 @@ namespace __list [[__nodiscard__]] constexpr reference operator*() const noexcept - { return static_cast<_Node&>(*_M_node)._M_data; } + { return static_cast<_Node&>(*_M_node)._M_u._M_data; } [[__nodiscard__]] constexpr pointer @@ -748,6 +758,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 rebind<typename _Node_traits::_Node>::other _Node_alloc_type; typedef __gnu_cxx::__alloc_traits<_Node_alloc_type> _Node_alloc_traits; +#if __cplusplus < 201103L || ! _GLIBCXX_USE_ALLOC_PTR_FOR_LIST + typedef _List_node<_Tp>* _Node_ptr; +#else + using _Node_ptr = typename _Node_alloc_traits::pointer; +#endif + struct _List_impl : public _Node_alloc_type { @@ -797,42 +813,47 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 _M_get_node() { return _Node_alloc_traits::allocate(_M_impl, 1); } -#if __cplusplus < 201103L || _GLIBCXX_USE_ALLOC_PTR_FOR_LIST - void - _M_put_node(typename _Node_alloc_traits::pointer __p) _GLIBCXX_NOEXCEPT - { _Node_alloc_traits::deallocate(_M_impl, __p, 1); } -#else void - _M_put_node(_List_node<_Tp>* __p) + _M_put_node(_Node_ptr __p) _GLIBCXX_NOEXCEPT { - // When not using the allocator's pointer type internally we must - // convert between _Node_ptr (i.e. _List_node*) and the allocator's - // pointer type. +#if __cplusplus < 201103L || _GLIBCXX_USE_ALLOC_PTR_FOR_LIST + _Node_alloc_traits::deallocate(_M_impl, __p, 1); +#else +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr using __alloc_pointer = typename _Node_alloc_traits::pointer; - auto __ap = pointer_traits<__alloc_pointer>::pointer_to(*__p); - _Node_alloc_traits::deallocate(_M_impl, __ap, 1); - } + if constexpr (is_same<_Node_ptr, __alloc_pointer>::value) + _Node_alloc_traits::deallocate(_M_impl, __p, 1); + else + { + // When not using the allocator's pointer type internally we must + // convert __p to __alloc_pointer so it can be deallocated. + auto __ap = pointer_traits<__alloc_pointer>::pointer_to(*__p); + _Node_alloc_traits::deallocate(_M_impl, __ap, 1); + } +#pragma GCC diagnostic pop #endif + } -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr void - _M_destroy_node(typename _Node_alloc_traits::pointer __p) + _M_destroy_node(_Node_ptr __p) { -#if __cplusplus >= 201103L // Destroy the element +#if __cplusplus < 201103L + _Tp_alloc_type(_M_impl).destroy(__p->_M_valptr()); +#else _Node_alloc_traits::destroy(_M_impl, __p->_M_valptr()); // Only destroy the node if the pointers require it. using _Node = typename _Node_traits::_Node; using _Base_ptr = typename _Node_traits::_Node_base::_Base_ptr; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr if constexpr (!is_trivially_destructible<_Base_ptr>::value) __p->~_Node(); -#else - _Tp_alloc_type(_M_impl).destroy(__p->_M_valptr()); +#pragma GCC diagnostic pop #endif this->_M_put_node(__p); } -#pragma GCC diagnostic pop public: typedef _Alloc allocator_type; @@ -1084,12 +1105,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 auto __guard = std::__allocate_guarded_obj(__alloc); _Node_alloc_traits::construct(__alloc, __guard->_M_valptr(), std::forward<_Args>(__args)...); - auto __p = __guard.release(); -#if _GLIBCXX_USE_ALLOC_PTR_FOR_LIST - return __p; -#else - return std::__to_address(__p); -#endif + return __guard.release(); } #endif @@ -2756,8 +2772,8 @@ namespace __list void _Node_base<_VoidPtr>::swap(_Node_base& __x, _Node_base& __y) noexcept { - auto __px = pointer_traits<_Base_ptr>::pointer_to(__x); - auto __py = pointer_traits<_Base_ptr>::pointer_to(__y); + auto __px = __x._M_base(); + auto __py = __x._M_base(); if (__x._M_next != __px) { @@ -2792,11 +2808,11 @@ namespace __list template<typename _VoidPtr> void _Node_base<_VoidPtr>::_M_transfer(_Base_ptr const __first, - _Base_ptr const __last) + _Base_ptr const __last) noexcept { __glibcxx_assert(__first != __last); - auto __self = pointer_traits<_Base_ptr>::pointer_to(*this); + auto __self = _M_base(); if (__self != __last) { // Remove [first, last) from its old position. diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc b/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc index 621ff4c652cb..e3bf8a13095a 100644 --- a/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc +++ b/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc @@ -59,6 +59,17 @@ struct Allocator template class std::forward_list<int, Allocator<int>>; +struct NonTrivial +{ + NonTrivial() { } + NonTrivial(const NonTrivial&) { } + ~NonTrivial() { } + bool operator==(const NonTrivial&) const { return true; } // for remove(T) + bool operator<(const NonTrivial&) const { return false; } // for sort() +}; + +template class std::forward_list<NonTrivial, Allocator<NonTrivial>>; + #include <testsuite_iterators.h> void diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc b/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc index 6205a2ff3bf2..19df931a6a65 100644 --- a/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc +++ b/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc @@ -1,4 +1,4 @@ -// { dg-options "-D_GLIBCXX_FWDLIST_USE_ALLOC_PTR=0" } +// { dg-options "-D_GLIBCXX_USE_ALLOC_PTR_FOR_FWD_LIST=0" } // { dg-do compile { target c++11 } } #include "alloc_ptr.cc" diff --git a/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc b/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc index d3b2cfe6d923..c52591b3cf8b 100644 --- a/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc +++ b/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc @@ -57,6 +57,17 @@ struct Allocator template class std::list<int, Allocator<int>>; +struct NonTrivial +{ + NonTrivial() { } + NonTrivial(const NonTrivial&) { } + ~NonTrivial() { } + bool operator==(const NonTrivial&) const { return true; } // for remove(T) + bool operator<(const NonTrivial&) const { return false; } // for sort() +}; + +template class std::list<NonTrivial, Allocator<NonTrivial>>; + #include <testsuite_iterators.h> void diff --git a/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc b/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc index e6a499d27160..f5bbf2cb944e 100644 --- a/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc +++ b/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc @@ -1,4 +1,4 @@ -// { dg-options "-D_GLIBCXX_LIST_USE_ALLOC_PTR=0" } +// { dg-options "-D_GLIBCXX_USE_ALLOC_PTR_FOR_LIST=0" } // { dg-do compile { target c++11 } } #include "alloc_ptr.cc"