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
STEllAR-GROUP · May 4, 2024 · e444594 · e444594
1 parent 87653e1
commit e444594
Show file tree

Hide file tree

Showing 10 changed files with 143 additions and 59 deletions.
diff --git 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 $<$<CONFIG:Debug>:HPX_DEBUG>
+  )
+  target_compile_definitions(
+    hpx_public_flags INTERFACE $<$<CONFIG:Debug>: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
@@ -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
@@ -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
@@ -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
@@ -94,21 +94,21 @@ namespace hpx::serialization {
             else if constexpr (!std::is_empty_v<dT>)
             {
                 // non_intrusive
-                if constexpr (hpx::traits::is_bitwise_serializable_v<dT> ||
-                    !hpx::traits::is_not_bitwise_serializable_v<dT>)
-                {
-                    // bitwise serializable types can be directly dispatched to
-                    // the archive functions
-                    ar.invoke(t);
-                }
-                else if constexpr (hpx::traits::has_serialize_adl_v<dT>)
+                if constexpr (hpx::traits::has_serialize_adl_v<dT>)
                 {
                     // this additional indirection level is needed to force ADL
                     // on the second phase of template lookup. call of serialize
                     // function directly from base_object finds only
                     // serialize-member function and doesn't perform ADL
                     detail::serialize_force_adl(ar, t, 0);
                 }
+                else if constexpr (hpx::traits::is_bitwise_serializable_v<dT> ||
+                    !hpx::traits::is_not_bitwise_serializable_v<dT>)
+                {
+                    // bitwise serializable types can be directly dispatched to
+                    // the archive functions
+                    ar.invoke(t);
+                }
                 else if constexpr (hpx::traits::has_struct_serialization_v<dT>)
                 {
                     // 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
@@ -13,6 +13,7 @@
 #include <hpx/serialization/config/defines.hpp>
 #include <hpx/assert.hpp>
 #include <hpx/modules/errors.hpp>
+#include <hpx/serialization/access.hpp>
 #include <hpx/serialization/basic_archive.hpp>
 #include <hpx/serialization/detail/polymorphic_nonintrusive_factory.hpp>
 #include <hpx/serialization/detail/raw_ptr.hpp>
@@ -111,8 +112,29 @@ namespace hpx::serialization {
 #endif
             if constexpr (!std::is_integral_v<T> && !std::is_enum_v<T>)
             {
-                if constexpr (hpx::traits::is_bitwise_serializable_v<T> ||
-                    !hpx::traits::is_not_bitwise_serializable_v<T>)
+                // check for normal serialization first
+                constexpr bool has_serialize =
+                    hpx::traits::is_intrusive_polymorphic_v<T> ||
+                    access::has_serialize_v<T> || std::is_empty_v<T> ||
+                    hpx::traits::has_serialize_adl_v<T> ||
+                    hpx::traits::has_struct_serialization_v<T>;
+
+                constexpr bool optimized =
+                    hpx::traits::is_bitwise_serializable_v<T> ||
+                    !hpx::traits::is_not_bitwise_serializable_v<T>;
+
+                if constexpr (traits::is_nonintrusive_polymorphic_v<T>)
+                {
+                    // 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<T>,
@@ -130,16 +152,12 @@ namespace hpx::serialization {
 #endif
                     load_binary(&t, sizeof(t));
                 }
-                else if constexpr (traits::is_nonintrusive_polymorphic_v<T>)
-                {
-                    // non-bitwise polymorphic serialization
-                    detail::polymorphic_nonintrusive_factory::instance().load(
-                        *this, t);
-                }
                 else
                 {
-                    // non-bitwise normal serialization
-                    access::serialize(*this, t, 0);
+                    static_assert(traits::is_nonintrusive_polymorphic_v<T> ||
+                            optimized || has_serialize,
+                        "traits::is_nonintrusive_polymorphic_v<T> || "
+                        "optimized || has_serialize");
                 }
             }
 #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
@@ -12,6 +12,7 @@
 #include <hpx/config/endian.hpp>
 #include <hpx/serialization/config/defines.hpp>
 #include <hpx/assert.hpp>
+#include <hpx/serialization/access.hpp>
 #include <hpx/serialization/basic_archive.hpp>
 #include <hpx/serialization/detail/polymorphic_nonintrusive_factory.hpp>
 #include <hpx/serialization/detail/raw_ptr.hpp>
@@ -210,8 +211,29 @@ namespace hpx::serialization {
 #endif
             if constexpr (!std::is_integral_v<T> && !std::is_enum_v<T>)
             {
-                if constexpr (hpx::traits::is_bitwise_serializable_v<T> ||
-                    !hpx::traits::is_not_bitwise_serializable_v<T>)
+                // check for normal serialization first
+                constexpr bool has_serialize =
+                    hpx::traits::is_intrusive_polymorphic_v<T> ||
+                    access::has_serialize_v<T> || std::is_empty_v<T> ||
+                    hpx::traits::has_serialize_adl_v<T> ||
+                    hpx::traits::has_struct_serialization_v<T>;
+
+                constexpr bool optimized =
+                    hpx::traits::is_bitwise_serializable_v<T> ||
+                    !hpx::traits::is_not_bitwise_serializable_v<T>;
+
+                if constexpr (traits::is_nonintrusive_polymorphic_v<T>)
+                {
+                    // 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<T>,
@@ -229,16 +251,12 @@ namespace hpx::serialization {
 #endif
                     save_binary(&t, sizeof(t));
                 }
-                else if constexpr (traits::is_nonintrusive_polymorphic_v<T>)
-                {
-                    // non-bitwise polymorphic serialization
-                    detail::polymorphic_nonintrusive_factory::instance().save(
-                        *this, t);
-                }
                 else
                 {
-                    // non-bitwise normal serialization
-                    access::serialize(*this, t, 0);
+                    static_assert(traits::is_nonintrusive_polymorphic_v<T> ||
+                            optimized || has_serialize,
+                        "traits::is_nonintrusive_polymorphic_v<T> || "
+                        "optimized || has_serialize");
                 }
             }
 #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
@@ -41,12 +41,16 @@ namespace hpx::traits {
         static constexpr const wildcard _wildcard{};
 
         ///////////////////////////////////////////////////////////////////////
+        // clang-format off
         template <typename T, std::size_t... I>
-        constexpr auto is_brace_constructible(std::index_sequence<I...>,
-            T*) noexcept -> decltype(T{_wildcard<I>...}, std::true_type{})
+        constexpr auto is_brace_constructible(std::index_sequence<I...>, T*)
+            // older versions of clang get confused by this
+            // NOLINTNEXTLINE(bugprone-throw-keyword-missing)
+            noexcept -> decltype(T{_wildcard<I>...}, std::true_type{})
         {
             return {};
         }
+        // clang-format on
 
         template <std::size_t... I>
         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
@@ -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 <typename T, typename Enable = void>
-    struct is_bitwise_serializable : std::is_arithmetic<T>
+    struct is_bitwise_serializable
+      : std::integral_constant<bool,
+            (std::is_trivially_copy_assignable_v<T> ||
+                (std::is_copy_assignable_v<T> &&
+                    std::is_trivially_copy_constructible_v<T>) ) &&
+                !std::is_pointer_v<T>>
     {
     };
 #else
     template <typename T, typename Enable = void>
     struct is_bitwise_serializable
       : std::integral_constant<bool,
-            std::is_arithmetic_v<T> || std::is_pointer_v<T>>
+            std::is_trivially_copy_assignable_v<T> ||
+                (std::is_copy_assignable_v<T> &&
+                    std::is_trivially_copy_constructible_v<T>)>
     {
     };
 #endif

diff --git a/libs/core/serialization/tests/unit/serialization_raw_pointer.cpp b/libs/core/serialization/tests/unit/serialization_raw_pointer.cpp
@@ -38,6 +38,15 @@ struct A
     {
         return true;
     }
+    friend bool operator!=(A const& lhs, A const& rhs)
+    {
+        return !(lhs == rhs);
+    }
+};
+
+template <>
+struct hpx::traits::is_bitwise_serializable<A> : std::false_type
+{
 };
 
 int hpx_main()