From 194c82e4b662492faa2a72334ceefdebc1ec7d35 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Fri, 3 May 2024 14:22:02 -0500 Subject: [PATCH] Generalize the notion of bitwise serialization - flyby: fix setting HPX_DEBUG on Windows platform - flyby: protect against out-of-bounds access in schedulers during early parcel exceptions --- cmake/HPX_CompilerFlagsTargets.cmake | 25 +++++++---- .../local_priority_queue_scheduler.hpp | 14 +++++-- .../hpx/schedulers/local_queue_scheduler.hpp | 10 ++++- .../local_workrequesting_scheduler.hpp | 31 ++++++++------ .../include/hpx/serialization/access.hpp | 16 +++---- .../hpx/serialization/input_archive.hpp | 42 +++++++++++++++---- .../hpx/serialization/output_archive.hpp | 42 +++++++++++++++---- .../traits/brace_initializable_traits.hpp | 8 +++- .../traits/is_bitwise_serializable.hpp | 13 ++++-- .../tests/unit/serialization_raw_pointer.cpp | 12 +++++- .../trivially_copyable_all_gather.cpp | 10 ++--- 11 files changed, 160 insertions(+), 63 deletions(-) diff --git a/cmake/HPX_CompilerFlagsTargets.cmake b/cmake/HPX_CompilerFlagsTargets.cmake index 66410dd6369d..5ffdec89af77 100644 --- a/cmake/HPX_CompilerFlagsTargets.cmake +++ b/cmake/HPX_CompilerFlagsTargets.cmake @@ -14,14 +14,23 @@ target_compile_features(hpx_private_flags INTERFACE cxx_std_${HPX_CXX_STANDARD}) target_compile_features(hpx_public_flags INTERFACE cxx_std_${HPX_CXX_STANDARD}) # Set other flags that should always be set - -# HPX_DEBUG must be set without a generator expression as it determines ABI -# compatibility. Projects in Release mode using HPX in Debug mode must have -# HPX_DEBUG set, and projects in Debug mode using HPX in Release mode must not -# have HPX_DEBUG set. HPX_DEBUG must also not be set by projects using HPX. -if(${CMAKE_BUILD_TYPE} STREQUAL "Debug") - target_compile_definitions(hpx_private_flags INTERFACE HPX_DEBUG) - target_compile_definitions(hpx_public_flags INTERFACE HPX_DEBUG) +get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) +if(is_multi_config) + target_compile_definitions( + hpx_private_flags INTERFACE $<$:HPX_DEBUG> + ) + target_compile_definitions( + hpx_public_flags INTERFACE $<$:HPX_DEBUG> + ) +else() + # HPX_DEBUG must be set without a generator expression as it determines ABI + # compatibility. Projects in Release mode using HPX in Debug mode must have + # HPX_DEBUG set, and projects in Debug mode using HPX in Release mode must not + # have HPX_DEBUG set. HPX_DEBUG must also not be set by projects using HPX. + if(${CMAKE_BUILD_TYPE} STREQUAL "Debug") + target_compile_definitions(hpx_private_flags INTERFACE HPX_DEBUG) + target_compile_definitions(hpx_public_flags INTERFACE HPX_DEBUG) + endif() endif() target_compile_definitions( diff --git a/libs/core/schedulers/include/hpx/schedulers/local_priority_queue_scheduler.hpp b/libs/core/schedulers/include/hpx/schedulers/local_priority_queue_scheduler.hpp index f2b42fe1fb84..80ea9978d5c2 100644 --- a/libs/core/schedulers/include/hpx/schedulers/local_priority_queue_scheduler.hpp +++ b/libs/core/schedulers/include/hpx/schedulers/local_priority_queue_scheduler.hpp @@ -1613,8 +1613,11 @@ namespace hpx::threads::policies { if (num_thread == num_queues_ - 1) low_priority_queue_.on_stop_thread(num_thread); - bound_queues_[num_thread].data_->on_stop_thread(num_thread); - queues_[num_thread].data_->on_stop_thread(num_thread); + if (num_thread < bound_queues_.size()) + { + bound_queues_[num_thread].data_->on_stop_thread(num_thread); + queues_[num_thread].data_->on_stop_thread(num_thread); + } } void on_error( @@ -1628,8 +1631,11 @@ namespace hpx::threads::policies { if (num_thread == num_queues_ - 1) low_priority_queue_.on_error(num_thread, e); - bound_queues_[num_thread].data_->on_error(num_thread, e); - queues_[num_thread].data_->on_error(num_thread, e); + if (num_thread < bound_queues_.size()) + { + bound_queues_[num_thread].data_->on_error(num_thread, e); + queues_[num_thread].data_->on_error(num_thread, e); + } } void reset_thread_distribution() noexcept override diff --git a/libs/core/schedulers/include/hpx/schedulers/local_queue_scheduler.hpp b/libs/core/schedulers/include/hpx/schedulers/local_queue_scheduler.hpp index 4f3b0c708179..280ec4faf16c 100644 --- a/libs/core/schedulers/include/hpx/schedulers/local_queue_scheduler.hpp +++ b/libs/core/schedulers/include/hpx/schedulers/local_queue_scheduler.hpp @@ -909,13 +909,19 @@ namespace hpx::threads::policies { void on_stop_thread(std::size_t num_thread) override { - queues_[num_thread]->on_stop_thread(num_thread); + if (num_thread < queues_.size()) + { + queues_[num_thread]->on_stop_thread(num_thread); + } } void on_error( std::size_t num_thread, std::exception_ptr const& e) override { - queues_[num_thread]->on_error(num_thread, e); + if (num_thread < queues_.size()) + { + queues_[num_thread]->on_error(num_thread, e); + } } protected: diff --git a/libs/core/schedulers/include/hpx/schedulers/local_workrequesting_scheduler.hpp b/libs/core/schedulers/include/hpx/schedulers/local_workrequesting_scheduler.hpp index a740cffde3a6..0f8cf8c7f536 100644 --- a/libs/core/schedulers/include/hpx/schedulers/local_workrequesting_scheduler.hpp +++ b/libs/core/schedulers/include/hpx/schedulers/local_workrequesting_scheduler.hpp @@ -1793,13 +1793,16 @@ namespace hpx::threads::policies { void on_stop_thread(std::size_t num_thread) override { - auto& d = data_[num_thread].data_; - - d.queue_->on_stop_thread(num_thread); - d.bound_queue_->on_stop_thread(num_thread); - if (num_thread < num_high_priority_queues_) + if (num_thread < data_.size()) { - d.high_priority_queue_->on_stop_thread(num_thread); + auto& d = data_[num_thread].data_; + + d.queue_->on_stop_thread(num_thread); + d.bound_queue_->on_stop_thread(num_thread); + if (num_thread < num_high_priority_queues_) + { + d.high_priority_queue_->on_stop_thread(num_thread); + } } if (num_thread == num_queues_ - 1) @@ -1811,13 +1814,17 @@ namespace hpx::threads::policies { void on_error( std::size_t num_thread, std::exception_ptr const& e) override { - auto& d = data_[num_thread].data_; - - d.queue_->on_error(num_thread, e); - d.bound_queue_->on_error(num_thread, e); - if (num_thread < num_high_priority_queues_) + if (num_thread < data_.size()) { - d.high_priority_queue_->on_error(num_thread, e); + auto& d = data_[num_thread].data_; + + d.queue_->on_error(num_thread, e); + d.bound_queue_->on_error(num_thread, e); + + if (num_thread < num_high_priority_queues_) + { + d.high_priority_queue_->on_error(num_thread, e); + } } if (num_thread == num_queues_ - 1) diff --git a/libs/core/serialization/include/hpx/serialization/access.hpp b/libs/core/serialization/include/hpx/serialization/access.hpp index 7de2e7b2d950..bf3379993c12 100644 --- a/libs/core/serialization/include/hpx/serialization/access.hpp +++ b/libs/core/serialization/include/hpx/serialization/access.hpp @@ -94,14 +94,7 @@ namespace hpx::serialization { else if constexpr (!std::is_empty_v
) { // non_intrusive - if constexpr (hpx::traits::is_bitwise_serializable_v
|| - !hpx::traits::is_not_bitwise_serializable_v
) - { - // bitwise serializable types can be directly dispatched to - // the archive functions - ar.invoke(t); - } - else if constexpr (hpx::traits::has_serialize_adl_v
) + if constexpr (hpx::traits::has_serialize_adl_v
) { // this additional indirection level is needed to force ADL // on the second phase of template lookup. call of serialize @@ -109,6 +102,13 @@ namespace hpx::serialization { // serialize-member function and doesn't perform ADL detail::serialize_force_adl(ar, t, 0); } + else if constexpr (hpx::traits::is_bitwise_serializable_v
|| + !hpx::traits::is_not_bitwise_serializable_v
) + { + // bitwise serializable types can be directly dispatched to + // the archive functions + ar.invoke(t); + } else if constexpr (hpx::traits::has_struct_serialization_v
) { // This is automatic serialization for types that are simple diff --git a/libs/core/serialization/include/hpx/serialization/input_archive.hpp b/libs/core/serialization/include/hpx/serialization/input_archive.hpp index cca2db9b1991..23d7311efbe4 100644 --- a/libs/core/serialization/include/hpx/serialization/input_archive.hpp +++ b/libs/core/serialization/include/hpx/serialization/input_archive.hpp @@ -1,6 +1,6 @@ // Copyright (c) 2014 Thomas Heller // Copyright (c) 2015 Anton Bikineev -// Copyright (c) 2022-2023 Hartmut Kaiser +// Copyright (c) 2022-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -111,8 +112,28 @@ namespace hpx::serialization { #endif if constexpr (!std::is_integral_v && !std::is_enum_v) { - if constexpr (hpx::traits::is_bitwise_serializable_v || - !hpx::traits::is_not_bitwise_serializable_v) + // check for normal serialization first + constexpr bool has_serialize = + hpx::traits::is_intrusive_polymorphic_v || + access::has_serialize_v || std::is_empty_v || + hpx::traits::has_serialize_adl_v; + + constexpr bool optimized = + hpx::traits::is_bitwise_serializable_v || + !hpx::traits::is_not_bitwise_serializable_v; + + if constexpr (traits::is_nonintrusive_polymorphic_v) + { + // non-bitwise polymorphic serialization + detail::polymorphic_nonintrusive_factory::instance().load( + *this, t); + } + else if constexpr (has_serialize) + { + // non-bitwise normal serialization + access::serialize(*this, t, 0); + } + else if constexpr (optimized) { // bitwise serialization static_assert(!std::is_abstract_v, @@ -130,16 +151,19 @@ namespace hpx::serialization { #endif load_binary(&t, sizeof(t)); } - else if constexpr (traits::is_nonintrusive_polymorphic_v) + else if constexpr (hpx::traits::has_struct_serialization_v) { - // non-bitwise polymorphic serialization - detail::polymorphic_nonintrusive_factory::instance().load( - *this, t); + // struct serialization + access::serialize(*this, t, 0); } else { - // non-bitwise normal serialization - access::serialize(*this, t, 0); + static_assert(traits::is_nonintrusive_polymorphic_v || + has_serialize || optimized || + hpx::traits::has_struct_serialization_v, + "traits::is_nonintrusive_polymorphic_v || " + "has_serialize || optimized || " + "hpx::traits::has_struct_serialization_v"); } } #if defined(HPX_SERIALIZATION_HAVE_SUPPORTS_ENDIANESS) diff --git a/libs/core/serialization/include/hpx/serialization/output_archive.hpp b/libs/core/serialization/include/hpx/serialization/output_archive.hpp index a6e0964b5011..eacae3d90ae3 100644 --- a/libs/core/serialization/include/hpx/serialization/output_archive.hpp +++ b/libs/core/serialization/include/hpx/serialization/output_archive.hpp @@ -1,6 +1,6 @@ // Copyright (c) 2014 Thomas Heller // Copyright (c) 2015 Anton Bikineev -// Copyright (c) 2022-2023 Hartmut Kaiser +// Copyright (c) 2022-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -210,8 +211,28 @@ namespace hpx::serialization { #endif if constexpr (!std::is_integral_v && !std::is_enum_v) { - if constexpr (hpx::traits::is_bitwise_serializable_v || - !hpx::traits::is_not_bitwise_serializable_v) + // check for normal serialization first + constexpr bool has_serialize = + hpx::traits::is_intrusive_polymorphic_v || + access::has_serialize_v || std::is_empty_v || + hpx::traits::has_serialize_adl_v; + + constexpr bool optimized = + hpx::traits::is_bitwise_serializable_v || + !hpx::traits::is_not_bitwise_serializable_v; + + if constexpr (traits::is_nonintrusive_polymorphic_v) + { + // non-bitwise polymorphic serialization + detail::polymorphic_nonintrusive_factory::instance().save( + *this, t); + } + else if constexpr (has_serialize) + { + // non-bitwise normal serialization + access::serialize(*this, t, 0); + } + else if constexpr (optimized) { // bitwise serialization static_assert(!std::is_abstract_v, @@ -229,16 +250,19 @@ namespace hpx::serialization { #endif save_binary(&t, sizeof(t)); } - else if constexpr (traits::is_nonintrusive_polymorphic_v) + else if constexpr (hpx::traits::has_struct_serialization_v) { - // non-bitwise polymorphic serialization - detail::polymorphic_nonintrusive_factory::instance().save( - *this, t); + // struct serialization + access::serialize(*this, t, 0); } else { - // non-bitwise normal serialization - access::serialize(*this, t, 0); + static_assert(traits::is_nonintrusive_polymorphic_v || + has_serialize || optimized || + hpx::traits::has_struct_serialization_v, + "traits::is_nonintrusive_polymorphic_v || " + "has_serialize || optimized || " + "hpx::traits::has_struct_serialization_v"); } } #if defined(HPX_SERIALIZATION_HAVE_SUPPORTS_ENDIANESS) diff --git a/libs/core/serialization/include/hpx/serialization/traits/brace_initializable_traits.hpp b/libs/core/serialization/include/hpx/serialization/traits/brace_initializable_traits.hpp index f18a0491aa5f..4d5d000488f5 100644 --- a/libs/core/serialization/include/hpx/serialization/traits/brace_initializable_traits.hpp +++ b/libs/core/serialization/include/hpx/serialization/traits/brace_initializable_traits.hpp @@ -41,12 +41,16 @@ namespace hpx::traits { static constexpr const wildcard _wildcard{}; /////////////////////////////////////////////////////////////////////// + // clang-format off template - constexpr auto is_brace_constructible(std::index_sequence, - T*) noexcept -> decltype(T{_wildcard...}, std::true_type{}) + constexpr auto is_brace_constructible(std::index_sequence, T*) + // older versions of clang get confused by this + // NOLINTNEXTLINE(bugprone-throw-keyword-missing) + noexcept -> decltype(T{_wildcard...}, std::true_type{}) { return {}; } + // clang-format on template constexpr std::false_type is_brace_constructible( diff --git a/libs/core/serialization/include/hpx/serialization/traits/is_bitwise_serializable.hpp b/libs/core/serialization/include/hpx/serialization/traits/is_bitwise_serializable.hpp index c18bdaaf513c..c6c5ed642f7d 100644 --- a/libs/core/serialization/include/hpx/serialization/traits/is_bitwise_serializable.hpp +++ b/libs/core/serialization/include/hpx/serialization/traits/is_bitwise_serializable.hpp @@ -1,5 +1,5 @@ // Copyright (c) 2014 Thomas Heller -// Copyright (c) 2022-2023 Hartmut Kaiser +// Copyright (c) 2022-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -17,14 +17,21 @@ namespace hpx::traits { #if !defined(HPX_SERIALIZATION_HAVE_ALLOW_RAW_POINTER_SERIALIZATION) template - struct is_bitwise_serializable : std::is_arithmetic + struct is_bitwise_serializable + : std::integral_constant || + (std::is_copy_assignable_v && + std::is_trivially_copy_constructible_v) ) && + !std::is_pointer_v> { }; #else template struct is_bitwise_serializable : std::integral_constant || std::is_pointer_v> + std::is_trivially_copy_assignable_v || + (std::is_copy_assignable_v && + std::is_trivially_copy_constructible_v)> { }; #endif diff --git a/libs/core/serialization/tests/unit/serialization_raw_pointer.cpp b/libs/core/serialization/tests/unit/serialization_raw_pointer.cpp index de4ea643b03b..73deb2b4b07b 100644 --- a/libs/core/serialization/tests/unit/serialization_raw_pointer.cpp +++ b/libs/core/serialization/tests/unit/serialization_raw_pointer.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2021-2022 Hartmut Kaiser +// Copyright (c) 2021-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -22,6 +22,7 @@ #include #include #include +#include #include // non-bitwise copyable type @@ -38,6 +39,15 @@ struct A { return true; } + friend bool operator!=(A const& lhs, A const& rhs) + { + return !(lhs == rhs); + } +}; + +template <> +struct hpx::traits::is_bitwise_serializable : std::false_type +{ }; int hpx_main() diff --git a/libs/full/collectives/tests/regressions/trivially_copyable_all_gather.cpp b/libs/full/collectives/tests/regressions/trivially_copyable_all_gather.cpp index 5a4e855ace17..01af58ab9270 100644 --- a/libs/full/collectives/tests/regressions/trivially_copyable_all_gather.cpp +++ b/libs/full/collectives/tests/regressions/trivially_copyable_all_gather.cpp @@ -44,11 +44,6 @@ class dimensioned_array template using point = dimensioned_array; -template -struct hpx::traits::is_bitwise_serializable> : std::true_type -{ -}; - template struct BBox { @@ -57,6 +52,11 @@ struct BBox void test_local_use() { + static_assert(hpx::traits::is_bitwise_serializable_v>, + "hpx::traits::is_bitwise_serializable_v>"); + static_assert(hpx::traits::is_bitwise_serializable_v>, + "hpx::traits::is_bitwise_serializable_v>"); + using namespace hpx::collectives; constexpr char const* all_gather_direct_basename =