https://bugs.gentoo.org/933289 (and https://bugs.gentoo.org/932780, https://bugs.gentoo.org/931587) https://github.com/freeorion/freeorion/issues/4949 https://github.com/boostorg/container/issues/252 https://github.com/boostorg/container/issues/281 https://github.com/boostorg/container/commit/20ad12f20e661978e90dc7f36d8ab8ac05e5a5a9 From 20ad12f20e661978e90dc7f36d8ab8ac05e5a5a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Sun, 28 Apr 2024 23:29:59 +0200 Subject: [PATCH] Remove UB in flat_map implementation when the implementation has a movable std::pair --- a/boost/container/allocator_traits.hpp +++ b/boost/container/allocator_traits.hpp @@ -32,6 +32,8 @@ #include #include //is_empty #include +#include +#include #ifndef BOOST_CONTAINER_DETAIL_STD_FWD_HPP #include #endif @@ -81,6 +83,144 @@ namespace boost { namespace container { +namespace dtl { + +#if !defined(BOOST_NO_CXX11_VARIADIC_TEMPLATES) + +template +BOOST_CONTAINER_FORCEINLINE void construct_type(T *p, BOOST_FWD_REF(Args) ...args) +{ + ::new((void*)p, boost_container_new_t()) T(::boost::forward(args)...); +} + +template < class Pair, class KeyType, class ... Args> +typename dtl::enable_if< dtl::is_pair, void >::type +construct_type + (Pair* p, try_emplace_t, BOOST_FWD_REF(KeyType) k, BOOST_FWD_REF(Args) ...args) +{ + construct_type(dtl::addressof(p->first), ::boost::forward(k)); + BOOST_CONTAINER_TRY{ + construct_type(dtl::addressof(p->second), ::boost::forward(args)...); + } + BOOST_CONTAINER_CATCH(...) { + typedef typename Pair::first_type first_type; + dtl::addressof(p->first)->~first_type(); + BOOST_CONTAINER_RETHROW + } + BOOST_CONTAINER_CATCH_END +} + +#else + +#define BOOST_CONTAINER_ALLOCATOR_TRAITS_CONSTRUCT_TYPEJ(N) \ +template\ +BOOST_CONTAINER_FORCEINLINE \ + typename dtl::disable_if_c::value, void >::type \ +construct_type(T *p BOOST_MOVE_I##N BOOST_MOVE_UREF##N)\ +{\ + ::new((void*)p, boost_container_new_t()) T( BOOST_MOVE_FWD##N );\ +}\ +// +BOOST_MOVE_ITERATE_0TO8(BOOST_CONTAINER_ALLOCATOR_TRAITS_CONSTRUCT_TYPEJ) +#undef BOOST_CONTAINER_ALLOCATOR_TRAITS_CONSTRUCT_TYPEJ + +#define BOOST_CONTAINER_ALLOCATOR_TRAITS_CONSTRUCT_TYPE(N) \ +template < class Pair, class KeyType BOOST_MOVE_I##N BOOST_MOVE_CLASS##N>\ +typename dtl::enable_if< dtl::is_pair, void >::type construct_type\ + (Pair* p, try_emplace_t, BOOST_FWD_REF(KeyType) k BOOST_MOVE_I##N BOOST_MOVE_UREF##N)\ +{\ + construct_type(dtl::addressof(p->first), ::boost::forward(k));\ + BOOST_CONTAINER_TRY{\ + construct_type(dtl::addressof(p->second) BOOST_MOVE_I##N BOOST_MOVE_FWD##N);\ + }\ + BOOST_CONTAINER_CATCH(...) {\ + typedef typename Pair::first_type first_type;\ + dtl::addressof(p->first)->~first_type();\ + BOOST_CONTAINER_RETHROW\ + }\ + BOOST_CONTAINER_CATCH_END\ +}\ +// +BOOST_MOVE_ITERATE_0TO8(BOOST_CONTAINER_ALLOCATOR_TRAITS_CONSTRUCT_TYPE) +#undef BOOST_CONTAINER_ALLOCATOR_TRAITS_CONSTRUCT_TYPE + +#endif + +template +inline +typename dtl::enable_if, void >::type +construct_type(T* p) +{ + dtl::construct_type(dtl::addressof(p->first)); + BOOST_CONTAINER_TRY{ + dtl::construct_type(dtl::addressof(p->second)); + } + BOOST_CONTAINER_CATCH(...) { + typedef typename T::first_type first_type; + dtl::addressof(p->first)->~first_type(); + BOOST_CONTAINER_RETHROW + } + BOOST_CONTAINER_CATCH_END +} + + +template +inline +typename dtl::enable_if_c + < dtl::is_pair::value + , void >::type +construct_type(T* p, U &u) +{ + dtl::construct_type(dtl::addressof(p->first), u.first); + BOOST_CONTAINER_TRY{ + dtl::construct_type(dtl::addressof(p->second), u.second); + } + BOOST_CONTAINER_CATCH(...) { + typedef typename T::first_type first_type; + dtl::addressof(p->first)->~first_type(); + BOOST_CONTAINER_RETHROW + } + BOOST_CONTAINER_CATCH_END +} + +template +inline +typename dtl::enable_if_c + < dtl::is_pair::type>::value && + !boost::move_detail::is_reference::value //This is needed for MSVC10 and ambiguous overloads + , void >::type +construct_type(T* p, BOOST_RV_REF(U) u) +{ + dtl::construct_type(dtl::addressof(p->first), ::boost::move(u.first)); + BOOST_CONTAINER_TRY{ + dtl::construct_type(dtl::addressof(p->second), ::boost::move(u.second)); + } + BOOST_CONTAINER_CATCH(...) { + typedef typename T::first_type first_type; + dtl::addressof(p->first)->~first_type(); + BOOST_CONTAINER_RETHROW + } + BOOST_CONTAINER_CATCH_END +} + +template +inline +typename dtl::enable_if, void >::type +construct_type(T* p, BOOST_FWD_REF(U) x, BOOST_FWD_REF(V) y) +{ + dtl::construct_type(dtl::addressof(p->first), ::boost::forward(x)); + BOOST_CONTAINER_TRY{ + dtl::construct_type(dtl::addressof(p->second), ::boost::forward(y)); + } + BOOST_CONTAINER_CATCH(...) { + typedef typename T::first_type first_type; + dtl::addressof(p->first)->~first_type(); + BOOST_CONTAINER_RETHROW + } + BOOST_CONTAINER_CATCH_END +} + +} //namespace dtl #ifndef BOOST_CONTAINER_DOXYGEN_INVOKED @@ -419,7 +559,7 @@ struct allocator_traits template inline static void priv_construct(dtl::false_type, Allocator &, T *p, BOOST_FWD_REF(Args) ...args) - { ::new((void*)p, boost_container_new_t()) T(::boost::forward(args)...); } + { dtl::construct_type(p, ::boost::forward(args)...); } #else // #if !defined(BOOST_NO_CXX11_VARIADIC_TEMPLATES) public: @@ -450,7 +590,7 @@ struct allocator_traits \ template\ inline static void priv_construct(dtl::false_type, Allocator &, T *p BOOST_MOVE_I##N BOOST_MOVE_UREF##N)\ - { ::new((void*)p, boost_container_new_t()) T(BOOST_MOVE_FWD##N); }\ + { dtl::construct_type(p BOOST_MOVE_I##N BOOST_MOVE_FWD##N); }\ // BOOST_MOVE_ITERATE_0TO8(BOOST_CONTAINER_ALLOCATOR_TRAITS_PRIV_CONSTRUCT_IMPL) #undef BOOST_CONTAINER_ALLOCATOR_TRAITS_PRIV_CONSTRUCT_IMPL --- a/boost/container/detail/construct_in_place.hpp +++ b/boost/container/detail/construct_in_place.hpp @@ -24,6 +24,7 @@ #include #include #include +#include namespace boost { namespace container { @@ -62,9 +63,42 @@ BOOST_CONTAINER_FORCEINLINE void construct_in_place(Allocator &a, T *dest, empla //Assignment +template +BOOST_CONTAINER_FORCEINLINE + typename dtl::disable_if_c + < dtl::is_pair::type>::value + && dtl::is_pair::type>::value + , void>::type +assign_in_place_ref(T &t, BOOST_FWD_REF(U) u) +{ t = ::boost::forward(u); } + +template +BOOST_CONTAINER_FORCEINLINE + typename dtl::enable_if_c + < dtl::is_pair::type>::value + && dtl::is_pair::type>::value + , void>::type +assign_in_place_ref(T &t, const U &u) +{ + assign_in_place_ref(t.first, u.first); + assign_in_place_ref(t.second, u.second); +} + +template +BOOST_CONTAINER_FORCEINLINE + typename dtl::enable_if_c + < dtl::is_pair::type>::value + && dtl::is_pair::type>::value + , void>::type +assign_in_place_ref(T &t, BOOST_RV_REF(U) u) +{ + assign_in_place_ref(t.first, ::boost::move(u.first)); + assign_in_place_ref(t.second, ::boost::move(u.second)); +} + template BOOST_CONTAINER_FORCEINLINE void assign_in_place(DstIt dest, InpIt source) -{ *dest = *source; } +{ assign_in_place_ref(*dest, *source); } template BOOST_CONTAINER_FORCEINLINE void assign_in_place(DstIt dest, value_init_construct_iterator) --- a/boost/container/flat_map.hpp +++ b/boost/container/flat_map.hpp @@ -48,6 +48,11 @@ #include #endif +#if !defined(BOOST_NO_CXX11_RVALUE_REFERENCES) +#define BOOST_CONTAINER_STD_PAIR_IS_MOVABLE +#endif + + namespace boost { namespace container { @@ -58,21 +63,37 @@ class flat_multimap; namespace dtl{ +#if defined(BOOST_CONTAINER_STD_PAIR_IS_MOVABLE) template BOOST_CONTAINER_FORCEINLINE static D &force(S &s) -{ return *move_detail::force_ptr(&s); } +{ return s; } template BOOST_CONTAINER_FORCEINLINE static const D &force(const S &s) -{ return *move_detail::force_ptr(&s); } +{ return s; } + +template +BOOST_CONTAINER_FORCEINLINE static D force_copy(D s) +{ return s; } + +#else //!BOOST_CONTAINER_DOXYGEN_INVOKED + +template +BOOST_CONTAINER_FORCEINLINE static D &force(S &s) +{ return *move_detail::launder_cast(&s); } + +template +BOOST_CONTAINER_FORCEINLINE static const D &force(const S &s) +{ return *move_detail::launder_cast(&s); } template BOOST_CONTAINER_FORCEINLINE static D force_copy(const S &s) { - const D *const vp = move_detail::force_ptr(&s); + const D *const vp = move_detail::launder_cast(&s); D ret_val(*vp); return ret_val; } +#endif //BOOST_CONTAINER_DOXYGEN_INVOKED } //namespace dtl{ @@ -118,18 +139,27 @@ class flat_map private: BOOST_COPYABLE_AND_MOVABLE(flat_map) //This is the tree that we should store if pair was movable + typedef std::pair std_pair_t; typedef dtl::flat_tree< - std::pair, + std_pair_t, dtl::select1st, Compare, AllocatorOrContainer> tree_t; //This is the real tree stored here. It's based on a movable pair + typedef dtl::pair dtl_pair_t; + + #ifdef BOOST_CONTAINER_STD_PAIR_IS_MOVABLE + typedef std_pair_t impl_pair_t; + #else + typedef dtl_pair_t impl_pair_t; + #endif + typedef dtl::flat_tree< - dtl::pair, + impl_pair_t, dtl::select1st, Compare, - typename dtl::container_or_allocator_rebind >::type + typename dtl::container_or_allocator_rebind::type > impl_tree_t; impl_tree_t m_flat_tree; // flat tree representing flat_map @@ -851,7 +881,7 @@ class flat_map //! @copydoc ::boost::container::flat_set::nth(size_type) const BOOST_CONTAINER_ATTRIBUTE_NODISCARD inline const_iterator nth(size_type n) const BOOST_NOEXCEPT_OR_NOTHROW - { return dtl::force_copy(m_flat_tree.nth(n)); } + { return dtl::force_copy(m_flat_tree.nth(n)); } //! @copydoc ::boost::container::flat_set::index_of(iterator) BOOST_CONTAINER_ATTRIBUTE_NODISCARD inline @@ -1099,7 +1129,7 @@ class flat_map template inline BOOST_CONTAINER_DOC1ST ( std::pair - , typename dtl::enable_if_c::value + , typename dtl::enable_if_c::value BOOST_MOVE_I std::pair >::type) insert(BOOST_FWD_REF(Pair) x) { @@ -1153,7 +1183,7 @@ class flat_map template inline BOOST_CONTAINER_DOC1ST ( iterator - , typename dtl::enable_if_c::value + , typename dtl::enable_if_c::value BOOST_MOVE_I iterator>::type) insert(const_iterator p, BOOST_FWD_REF(Pair) x) { @@ -1777,17 +1807,24 @@ class flat_multimap #ifndef BOOST_CONTAINER_DOXYGEN_INVOKED private: BOOST_COPYABLE_AND_MOVABLE(flat_multimap) + typedef std::pair std_pair_t; typedef dtl::flat_tree< - std::pair, + std_pair_t, dtl::select1st, Compare, AllocatorOrContainer> tree_t; //This is the real tree stored here. It's based on a movable pair + typedef dtl::pair dtl_pair_t; + #ifdef BOOST_CONTAINER_STD_PAIR_IS_MOVABLE + typedef std_pair_t impl_pair_t; + #else + typedef dtl_pair_t impl_pair_t; + #endif typedef dtl::flat_tree< - dtl::pair, + impl_pair_t, dtl::select1st, Compare, - typename dtl::container_or_allocator_rebind >::type + typename dtl::container_or_allocator_rebind::type > impl_tree_t; impl_tree_t m_flat_tree; // flat tree representing flat_map @@ -2388,7 +2425,7 @@ class flat_multimap //! @copydoc ::boost::container::flat_set::nth(size_type) const BOOST_CONTAINER_ATTRIBUTE_NODISCARD inline const_iterator nth(size_type n) const BOOST_NOEXCEPT_OR_NOTHROW - { return dtl::force_copy(m_flat_tree.nth(n)); } + { return dtl::force_copy(m_flat_tree.nth(n)); } //! @copydoc ::boost::container::flat_set::index_of(iterator) BOOST_CONTAINER_ATTRIBUTE_NODISCARD inline @@ -2477,7 +2514,7 @@ class flat_multimap template inline BOOST_CONTAINER_DOC1ST ( iterator - , typename dtl::enable_if_c::value + , typename dtl::enable_if_c::value BOOST_MOVE_I iterator >::type) insert(BOOST_FWD_REF(Pair) x) { return dtl::force_copy(m_flat_tree.emplace_equal(boost::forward(x))); } @@ -2514,7 +2551,7 @@ class flat_multimap template inline BOOST_CONTAINER_DOC1ST ( iterator - , typename dtl::enable_if_c::value + , typename dtl::enable_if_c::value BOOST_MOVE_I iterator>::type) insert(const_iterator p, BOOST_FWD_REF(Pair) x) {