Skip to content

Commit

Permalink
Fixing component handling for lcos
Browse files Browse the repository at this point in the history 
This patch fixes a UB where the LCOs using (simple) components where cast to
wrong types.
  • Loading branch information
Thomas Heller committed Aug 22, 2017
1 parent 19453bd commit 1839bda
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 19 deletions.
12 changes: 7 additions & 5 deletions hpx/lcos/base_lco_with_value.hpp
View file
Expand Up @@ -12,8 +12,9 @@
#include <hpx/runtime/actions/basic_action.hpp>
#include <hpx/runtime/actions/component_action.hpp>
#include <hpx/runtime/components/component_type.hpp>
#include <hpx/runtime/components/server/simple_component_base.hpp>
#include <hpx/runtime/components/server/component.hpp>
#include <hpx/runtime/components/server/managed_component_base.hpp>
#include <hpx/runtime/components/server/simple_component_base.hpp>
#include <hpx/runtime/components_fwd.hpp>
#include <hpx/runtime/naming/id_type.hpp>
#include <hpx/traits/is_component.hpp>
Expand Down Expand Up @@ -42,7 +43,7 @@ namespace hpx { namespace lcos
template <typename BaseLco>
struct base_lco_wrapping_type<traits::detail::component_tag, BaseLco>
{
typedef components::simple_component<BaseLco> type;
typedef components::component<BaseLco> type;
};

template <typename BaseLco>
Expand Down Expand Up @@ -174,7 +175,8 @@ namespace hpx { namespace lcos
// components must contain a typedef for wrapping_type defining the
// managed_component type used to encapsulate instances of this
// component
typedef components::managed_component<base_lco_with_value> wrapping_type;
typedef typename detail::base_lco_wrapping_type<ComponentTag,
base_lco_with_value>::type wrapping_type;
typedef base_lco_with_value base_type_holder;

// refer to base type for the corresponding implementation
Expand All @@ -199,7 +201,7 @@ namespace hpx { namespace traits
{
static components::component_type get()
{
return components::component_base_lco_with_value;
return components::component_base_lco_with_value_unmanaged;
}

static void set(components::component_type)
Expand All @@ -216,7 +218,7 @@ namespace hpx { namespace traits
{
static components::component_type get()
{
return components::component_base_lco_with_value_unmanaged;
return components::component_base_lco_with_value_simple_unmanaged;
}

static void set(components::component_type)
Expand Down
14 changes: 14 additions & 0 deletions hpx/runtime/actions/set_lco_value_continuation.hpp
View file
Expand Up @@ -29,6 +29,20 @@ namespace hpx { namespace actions
}
};

///////////////////////////////////////////////////////////////////////////
struct set_lco_value_simple_unmanaged_continuation
{
template <typename T>
HPX_FORCEINLINE T operator()(naming::id_type const& lco, T&& t) const
{
hpx::set_lco_value_simple_unmanaged(lco, std::forward<T>(t));

// Yep, 't' is a zombie, however we don't use the returned value
// anyways. We need it for result type calculation, though.
return std::move(t);
}
};

///////////////////////////////////////////////////////////////////////////
struct set_lco_value_unmanaged_continuation
{
Expand Down
25 changes: 14 additions & 11 deletions hpx/runtime/components/component_type.hpp
View file
Expand Up @@ -41,36 +41,39 @@ namespace hpx { namespace components
component_base_lco = 4,

// Base component for LCOs that produce values.
component_base_lco_with_value_unmanaged = 5,
component_base_lco_with_value_simple_unmanaged = 5,

// Base component for LCOs that produce values.
component_base_lco_with_value_unmanaged = 6,

// (Managed) base component for LCOs that produce values.
component_base_lco_with_value = 6,
component_base_lco_with_value = 7,

// Synchronization latch, barrier, and flex_barrier LCOs.
component_latch = ((7 << 16) | component_base_lco_with_value),
component_barrier = ((8 << 16) | component_base_lco),
component_flex_barrier = ((9 << 16) | component_base_lco),
component_latch = ((8 << 16) | component_base_lco_with_value),
component_barrier = ((9 << 16) | component_base_lco),
component_flex_barrier = ((10 << 16) | component_base_lco),

// An LCO representing a value which may not have been computed yet.
component_promise = ((10 << 16) | component_base_lco_with_value),
component_promise = ((11 << 16) | component_base_lco_with_value),

// AGAS locality services.
component_agas_locality_namespace = 11,
component_agas_locality_namespace = 12,

// AGAS primary address resolution services.
component_agas_primary_namespace = 12,
component_agas_primary_namespace = 13,

// AGAS global type system.
component_agas_component_namespace = 13,
component_agas_component_namespace = 14,

// AGAS symbolic naming services.
component_agas_symbol_namespace = 14,
component_agas_symbol_namespace = 15,

component_last,
component_first_dynamic = component_last,

// Force this enum type to be at least 32 bits.
component_upper_bound = 0x7fffffffL //-V112
component_upper_bound = 0x7fffffffL //-V112
};

enum factory_state_enum
Expand Down
12 changes: 12 additions & 0 deletions hpx/runtime/components/server/component.hpp
View file
Expand Up @@ -18,12 +18,24 @@ namespace hpx { namespace components {
: public simple_component<Component>
{
public:
typedef component<Component> component_type;
typedef component_type derived_type;

/// \brief Construct a simple_component instance holding a new wrapped
/// instance
template <typename ...Ts>
component(Ts&&... vs)
: simple_component<Component>(std::forward<Ts>(vs)...)
{}

/// \brief The function \a create is used for allocation and
/// initialization of instances of the derived components.
static component_type* create(std::size_t count)
{
// simple components can be created individually only
HPX_ASSERT(1 == count);
return new component_type(); //-V572
}
};
}}

Expand Down
2 changes: 1 addition & 1 deletion hpx/runtime/components/server/simple_component_base.hpp
View file
Expand Up @@ -301,7 +301,7 @@ namespace hpx { namespace components
{
// simple components can be created individually only
HPX_ASSERT(1 == count);
return static_cast<component_type*>(new Component()); //-V572
return new component_type(); //-V572
}

/// \brief The function \a destroy is used for destruction and
Expand Down
68 changes: 66 additions & 2 deletions hpx/runtime/trigger_lco.hpp
View file
Expand Up @@ -110,6 +110,26 @@ namespace hpx
set_lco_value(id, std::move(addr), std::forward<Result>(t), move_credits);
}

/// \brief Set the result value for the (simple unmanaged) LCO referenced by the given id
///
/// \param id [in] This represents the id of the LCO which should
/// receive the given value.
/// \param t [in] This is the value which should be sent to the LCO.
/// \param move_credits [in] If this is set to \a true then it is ok to
/// send all credits in \a id along with the generated
/// message. The default value is \a true.
template <typename Result>
typename std::enable_if<!std::is_same<typename util::decay<Result>::type,
naming::address>::value>::type
set_lco_value_simple_unmanaged(
naming::id_type const& id, Result&& t, bool move_credits = true)
{
naming::address addr(nullptr,
components::component_base_lco_with_value_simple_unmanaged);
set_lco_value(
id, std::move(addr), std::forward<Result>(t), move_credits);
}

/// \brief Set the result value for the (unmanaged) LCO referenced by the given id
///
/// \param id [in] This represents the id of the LCO which should
Expand Down Expand Up @@ -167,6 +187,27 @@ namespace hpx
move_credits);
}

/// \brief Set the result value for the (simple unmanaged) LCO referenced by the given id
///
/// \param id [in] This represents the id of the LCO which should
/// receive the given value.
/// \param t [in] This is the value which should be sent to the LCO.
/// \param cont [in] This represents the LCO to trigger after completion.
/// \param move_credits [in] If this is set to \a true then it is ok to
/// send all credits in \a id along with the generated
/// message. The default value is \a true.
template <typename Result>
typename std::enable_if<!std::is_same<typename util::decay<Result>::type,
naming::address>::value>::type
set_lco_value_simple_unmanaged(naming::id_type const& id, Result&& t,
naming::id_type const& cont, bool move_credits = true)
{
naming::address addr(nullptr,
components::component_base_lco_with_value_simple_unmanaged);
set_lco_value(
id, std::move(addr), std::forward<Result>(t), cont, move_credits);
}

/// \brief Set the result value for the (unmanaged) LCO referenced by the given id
///
/// \param id [in] This represents the id of the LCO which should
Expand Down Expand Up @@ -467,7 +508,7 @@ namespace hpx
>::type local_result_type;

if (components::get_base_type(addr.type_) ==
components::component_base_lco_with_value_unmanaged)
components::component_base_lco_with_value_simple_unmanaged)
{
typedef typename lcos::base_lco_with_value<
local_result_type, remote_result_type,
Expand All @@ -477,6 +518,17 @@ namespace hpx
detail::set_lco_value<set_value_action>(id, std::move(addr),
std::forward<Result>(t), move_credits);
}
else if (components::get_base_type(addr.type_) ==
components::component_base_lco_with_value_unmanaged)
{
typedef typename lcos::base_lco_with_value<local_result_type,
remote_result_type,
traits::detail::component_tag>::set_value_action
set_value_action;

detail::set_lco_value<set_value_action>(
id, std::move(addr), std::forward<Result>(t), move_credits);
}
else
{
HPX_ASSERT(!addr || components::get_base_type(addr.type_) ==
Expand All @@ -502,7 +554,7 @@ namespace hpx
>::type local_result_type;

if (components::get_base_type(addr.type_) ==
components::component_base_lco_with_value_unmanaged)
components::component_base_lco_with_value_simple_unmanaged)
{
typedef typename lcos::base_lco_with_value<
local_result_type, remote_result_type,
Expand All @@ -513,6 +565,18 @@ namespace hpx
local_result_type, remote_result_type, set_value_action
>(id, std::move(addr), std::forward<Result>(t), cont, move_credits);
}
else if (components::get_base_type(addr.type_) ==
components::component_base_lco_with_value_unmanaged)
{
typedef typename lcos::base_lco_with_value<local_result_type,
remote_result_type,
traits::detail::component_tag>::set_value_action
set_value_action;

detail::set_lco_value<local_result_type, remote_result_type,
set_value_action>(id, std::move(addr), std::forward<Result>(t),
cont, move_credits);
}
else
{
HPX_ASSERT(!addr || components::get_base_type(addr.type_) ==
Expand Down

0 comments on commit 1839bda

Please sign in to comment.