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 --- .gitignore | 128 ++-------------- include/boost/container/allocator_traits.hpp | 144 +++++++++++++++++- .../container/detail/construct_in_place.hpp | 36 ++++- include/boost/container/flat_map.hpp | 67 ++++++-- 4 files changed, 241 insertions(+), 134 deletions(-) diff --git a/.gitignore b/.gitignore index 8dbcd63..737a4de 100644 --- a/.gitignore +++ b/.gitignore @@ -2,121 +2,17 @@ # This .gitignore file was automatically created by Microsoft(R) Visual Studio. ################################################################################ -/proj/vs/.vs -/proj/vs/x64 -/proj/vs/Win32 -/proj/vs/allocator_traits_test.vcxproj.user -/proj/vs/alloc_basic_test.vcxproj.user -/proj/vs/alloc_full_test.vcxproj.user -/proj/vs/alloc_lib.vcxproj.user -/proj/vs/bench_adaptive_node_pool.vcxproj.user -/proj/vs/bench_alloc.vcxproj.user -/proj/vs/bench_alloc_expand_bwd.vcxproj.user -/proj/vs/bench_alloc_expand_fwd.vcxproj.user -/proj/vs/bench_alloc_shrink_to_fit.vcxproj.user -/proj/vs/bench_alloc_stable_vector_burst.vcxproj.user -/proj/vs/bench_flat_multiset.vcxproj.user -/proj/vs/bench_flat_set.vcxproj.user -/proj/vs/bench_hash_map_string_test.vcxproj -/proj/vs/bench_hash_map_string_test.vcxproj.user -/proj/vs/bench_hash_map_test.vcxproj -/proj/vs/bench_hash_map_test.vcxproj.user -/proj/vs/bench_hash_map_uint32_test.vcxproj -/proj/vs/bench_hash_map_uint32_test.vcxproj.user -/proj/vs/bench_hash_map_uint64_test.vcxproj -/proj/vs/bench_hash_map_uint64_test.vcxproj.user -/proj/vs/bench_hash_set_test.vcxproj -/proj/vs/bench_set.vcxproj.user -/proj/vs/bench_set_adaptive_pool.vcxproj.user -/proj/vs/bench_set_alloc_v2.vcxproj.user -/proj/vs/bench_set_avl.vcxproj.user -/proj/vs/bench_set_multi.vcxproj.user -/proj/vs/bench_set_sg.vcxproj.user -/proj/vs/bench_set_sp.vcxproj.user -/proj/vs/bench_static_vector.vcxproj.user -/proj/vs/bench_vectors.vcxproj.user -/proj/vs/boost_iterator_comp_test.vcxproj.user -/proj/vs/common_iterator_test.vcxproj.user -/proj/vs/container.vcxproj -/proj/vs/container.vcxproj.filters -/proj/vs/container.vcxproj.user -/proj/vs/copy_move_algo_test.vcxproj.user -/proj/vs/deque_options_test.vcxproj.user -/proj/vs/deque_test.vcxproj.user -/proj/vs/devector_options_test.vcxproj.user -/proj/vs/devector_test.vcxproj.user -/proj/vs/doc_custom_deque.vcxproj.user -/proj/vs/doc_custom_devector.vcxproj.user -/proj/vs/doc_custom_small_vector.vcxproj.user -/proj/vs/doc_custom_static_vector.vcxproj.user -/proj/vs/doc_custom_tree.vcxproj.user -/proj/vs/doc_custom_vector.vcxproj.user -/proj/vs/doc_emplace.vcxproj.user -/proj/vs/doc_extended_allocators.vcxproj.user -/proj/vs/doc_move_containers.vcxproj.user -/proj/vs/doc_pmr.vcxproj.user -/proj/vs/doc_recursive_containers.vcxproj.user -/proj/vs/doc_type_erasure.vcxproj.user -/proj/vs/explicit_inst_deque_test.vcxproj.user -/proj/vs/explicit_inst_devector_test.vcxproj.user -/proj/vs/explicit_inst_flat_map_test.vcxproj.user -/proj/vs/explicit_inst_flat_set_test.vcxproj.user -/proj/vs/explicit_inst_list_test.vcxproj.user -/proj/vs/explicit_inst_map_test.vcxproj.user -/proj/vs/explicit_inst_set_test.vcxproj.user -/proj/vs/explicit_inst_slist_test.vcxproj.user -/proj/vs/explicit_inst_small_vector_test.vcxproj.user -/proj/vs/explicit_inst_stable_vector_test.vcxproj.user -/proj/vs/explicit_inst_static_vector_test.vcxproj.user -/proj/vs/explicit_inst_string_test.vcxproj.user -/proj/vs/explicit_inst_vector_test.vcxproj.user -/proj/vs/flat_map_adaptor_test.vcxproj.user -/proj/vs/flat_map_test.vcxproj.user -/proj/vs/flat_set_adaptor_test.vcxproj.user -/proj/vs/flat_set_test.vcxproj.user -/proj/vs/flat_tree_test.vcxproj.user -/proj/vs/global_resource.vcxproj.user -/proj/vs/insert_vs_emplace_test.vcxproj.user -/proj/vs/list_test.vcxproj.user -/proj/vs/map_test.vcxproj.user -/proj/vs/memory_resource_test.vcxproj.user -/proj/vs/monotonic_buffer_resource_test.vcxproj.user -/proj/vs/node_handle_test.vcxproj.user -/proj/vs/null_iterators_test.vcxproj.user -/proj/vs/pair_test.vcxproj.user -/proj/vs/pmr_deque_test.vcxproj.user -/proj/vs/pmr_devector_test.vcxproj.user -/proj/vs/pmr_flat_map_test.vcxproj.user -/proj/vs/pmr_flat_set_test.vcxproj.user -/proj/vs/pmr_list_test.vcxproj.user -/proj/vs/pmr_map_test.vcxproj.user -/proj/vs/pmr_set_test.vcxproj.user -/proj/vs/pmr_slist_test.vcxproj.user -/proj/vs/pmr_small_vector_test.vcxproj.user -/proj/vs/pmr_stable_vector_test.vcxproj.user -/proj/vs/pmr_string_test.vcxproj.user -/proj/vs/pmr_vector_test.vcxproj.user -/proj/vs/polymorphic_allocator_test.vcxproj.user -/proj/vs/resource_adaptor.vcxproj.user -/proj/vs/scoped_allocator_adaptor_test.vcxproj.user -/proj/vs/scoped_allocator_usage_test.vcxproj.user -/proj/vs/set_test.vcxproj.user -/proj/vs/slist_test.vcxproj.user -/proj/vs/small_vector_options_test.vcxproj.user -/proj/vs/small_vector_test.vcxproj.user -/proj/vs/stable_vector_test.vcxproj.user -/proj/vs/static_vector_options_test.vcxproj.user -/proj/vs/static_vector_test.vcxproj.user -/proj/vs/string_test.vcxproj.user -/proj/vs/string_view_compat_test.vcxproj.user -/proj/vs/synchronized_pool_resource_test.vcxproj.user -/proj/vs/throw_exception_test.vcxproj.user -/proj/vs/tree_test.vcxproj.user -/proj/vs/unsynchronized_pool_resource_test.vcxproj.user -/proj/vs/uses_allocator_test.vcxproj.user -/proj/vs/vector_options_test.vcxproj.user -/proj/vs/vector_test.vcxproj.user +/proj/ /doc/html /doc/autodoc.xml -/proj/vs/deque_test.vcxproj -/proj/vs/stable_vector_test.vcxproj +/bench/bench_hash_common.hpp +/bench/bench_hash_map_test.cpp.disabled +/bench/bench_hash_string.cpp.disabled +/bench/bench_hash_uint32.cpp.disabled +/bench/bench_hash_uint64.cpp.disabled +/bench/fca_simple_unordered.hpp +/bench/fca_unordered.hpp +/bench/fxa_common.hpp +/bench/hash_map.hpp +/bench/hash_set.hpp +/bench/detail/hash_table.hpp diff --git a/include/boost/container/allocator_traits.hpp b/include/boost/container/allocator_traits.hpp index cdb6683..8e2fafa 100644 --- a/include/boost/container/allocator_traits.hpp +++ b/include/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 diff --git a/include/boost/container/detail/construct_in_place.hpp b/include/boost/container/detail/construct_in_place.hpp index d824d81..bfdf05c 100644 --- a/include/boost/container/detail/construct_in_place.hpp +++ b/include/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) diff --git a/include/boost/container/flat_map.hpp b/include/boost/container/flat_map.hpp index b092fb2..e4843f5 100644 --- a/include/boost/container/flat_map.hpp +++ b/include/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) {