Skip to content

Commit

Permalink
[Reproducer] Reproduce entering multi region substate segfault (#169)
Browse files Browse the repository at this point in the history
* fix: Use const current region variable for region loop
  • Loading branch information
erikzenker committed Nov 3, 2021
1 parent 88ba6e1 commit 597b27e
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 26 deletions.
9 changes: 6 additions & 3 deletions include/hsm/details/sm.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ template <class RootState, class... OptionalParameters> class sm {
auto status() -> std::string
{
std::stringstream statusStream;
for (Region region = 0; region < current_regions(); region++) {
const auto currentRegions = current_regions();
for (Region region = 0; region < currentRegions; region++) {
statusStream << "[" << region << "] "
<< "combined: " << m_currentCombinedState[region] << " "
<< "parent: " << currentParentState() << " "
Expand All @@ -134,7 +135,8 @@ template <class RootState, class... OptionalParameters> class sm {
bool allGuardsFailed = true;
bool allTransitionsInvalid = true;

for (Region region = 0; region < current_regions(); region++) {
const auto currentRegions = current_regions();
for (Region region = 0; region < currentRegions; region++) {

// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
auto& results = get_dispatch_table_entry(event, region);
Expand Down Expand Up @@ -191,7 +193,8 @@ template <class RootState, class... OptionalParameters> class sm {
while (true) {
auto allGuardsFailed = true;

for (Region region = 0; region < current_regions(); region++) {
const auto currentRegions = current_regions();
for (Region region = 0; region < currentRegions; region++) {

auto event = noneEvent {};
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
Expand Down
76 changes: 53 additions & 23 deletions include/hsm/gen/hsm.h
Original file line number Diff line number Diff line change
Expand Up @@ -678,21 +678,41 @@ template <class Tuple> constexpr auto to_pairs(const Tuple& tuple)
#include <boost/hana/size.hpp>
#include <boost/hana/zip.hpp>

//#include <boost/mp11.hpp>
namespace {
template <class X, class Tuple> class GetIdx;

template <class X, class... T> class GetIdx<X, boost::hana::tuple<T...>> {
template <std::size_t... idx>
static constexpr auto find_idx(std::index_sequence<idx...> seq) -> std::size_t
{
return seq.size() + ((std::is_same<X, T>::value ? idx - seq.size() : 0) + ...);
}

public:
static constexpr std::size_t value = find_idx(std::index_sequence_for<T...> {});
};

template <class X, class... T> class GetIdx<X, boost::hana::basic_tuple<T...>> {
template <std::size_t... idx>
static constexpr auto find_idx(std::index_sequence<idx...> seq) -> std::size_t
{
return seq.size() + ((std::is_same<X, T>::value ? idx - seq.size() : 0) + ...);
}

public:
static constexpr std::size_t value = find_idx(std::index_sequence_for<T...> {});
};
}

namespace hsm {

namespace bh {
using namespace boost::hana;
}

template <class Iterable, class Element>
constexpr auto index_of(Iterable const& iterable, Element const& element)
template <class Iterable, class Element> constexpr auto index_of(Iterable const&, Element const&)
{
return bh::apply(
[](auto size, auto dropped) { return size - dropped; },
bh::size(iterable),
bh::size(bh::drop_while(iterable, bh::not_equal.to(element))));
return GetIdx<Element, Iterable>::value;
}

template <class Typeids> constexpr auto make_index_map(Typeids typeids)
Expand Down Expand Up @@ -782,8 +802,8 @@ constexpr auto calcParentStateIdx
constexpr auto calcStateIdx
= [](std::size_t nStates, Idx combinedState) -> Idx { return combinedState % nStates; };

template <class Event>
constexpr decltype(auto) resolveEvent(Event event) {
template <class Event> constexpr decltype(auto) resolveEvent(Event event)
{
if constexpr (is_event(event)) {
return event.typeid_;
} else {
Expand Down Expand Up @@ -1770,6 +1790,8 @@ constexpr auto addDispatchTableEntry(
[=](auto& dispatchTable, auto&& transition2, bool internal) -> void {
const auto defer = false;
const auto valid = true;

// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
dispatchTable[fromIdx].push_front(
{ toIdx, history, defer, valid, internal, std::move(transition2) });
},
Expand Down Expand Up @@ -2309,13 +2331,16 @@ template <class RootState, class... OptionalParameters> class sm {
"Transition table needs to have at least one initial state");

auto optionalDependency = bh::make_basic_tuple(std::ref(optionalParameters)...);
// std::cout << (bh::at_c<0>(optionalDependency).get().callCount) << std::endl;
fill_unexpected_event_handler_tables(
rootState,
m_statesMap,
m_unexpectedEventHandlerTables,
get_unexpected_event_handler(rootState),
optionalDependency);

if constexpr (has_unexpected_event_handler(rootState)) {
fill_unexpected_event_handler_tables(
rootState,
m_statesMap,
m_unexpectedEventHandlerTables,
get_unexpected_event_handler(rootState),
optionalDependency);
}

fill_dispatch_table(optionalDependency);
fill_initial_state_table(rootState, m_initial_states);
fill_initial_state_table(rootState, m_history);
Expand Down Expand Up @@ -2359,7 +2384,8 @@ template <class RootState, class... OptionalParameters> class sm {
auto status() -> std::string
{
std::stringstream statusStream;
for (Region region = 0; region < current_regions(); region++) {
const auto currentRegions = current_regions();
for (Region region = 0; region < currentRegions; region++) {
statusStream << "[" << region << "] "
<< "combined: " << m_currentCombinedState[region] << " "
<< "parent: " << currentParentState() << " "
Expand All @@ -2380,7 +2406,8 @@ template <class RootState, class... OptionalParameters> class sm {
bool allGuardsFailed = true;
bool allTransitionsInvalid = true;

for (Region region = 0; region < current_regions(); region++) {
const auto currentRegions = current_regions();
for (Region region = 0; region < currentRegions; region++) {

// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
auto& results = get_dispatch_table_entry(event, region);
Expand Down Expand Up @@ -2437,7 +2464,8 @@ template <class RootState, class... OptionalParameters> class sm {
while (true) {
auto allGuardsFailed = true;

for (Region region = 0; region < current_regions(); region++) {
const auto currentRegions = current_regions();
for (Region region = 0; region < currentRegions; region++) {

auto event = noneEvent {};
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
Expand Down Expand Up @@ -2521,10 +2549,12 @@ template <class RootState, class... OptionalParameters> class sm {

template <class Event> auto call_unexpected_event_handler(Event& event)
{
// TODO: What todo in a multi region state machine?
m_unexpectedEventHandlerTables[bh::typeid_(event)]
.at(m_currentCombinedState.at(0))
->executeHandler(event);
if constexpr (has_unexpected_event_handler(rootState)) {
// TODO: What todo in a multi region state machine?
m_unexpectedEventHandlerTables[bh::typeid_(event)]
.at(m_currentCombinedState.at(0))
->executeHandler(event);
}
}

auto currentState(Region region)
Expand Down
2 changes: 2 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ add_executable(
integration/custom_targets.cpp
integration/chain_actions.cpp
integration/reproducer/should_execute_anonymous_transition_once.cpp
integration/reproducer/should_enter_substate_with_multiple_regions.cpp
)

target_compile_features(hsmIntegrationTests PRIVATE cxx_std_17)
target_link_libraries(hsmIntegrationTests PRIVATE hsm::hsm GTest::gtest_main)
gtest_discover_tests(hsmIntegrationTests TEST_PREFIX integration.)
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#include "hsm/hsm.h"

#include <gtest/gtest.h>

namespace {

using namespace ::testing;

// Events
struct e2 {
};
struct e1 {
};

// States
struct S1 {
};
struct S2 {
};

struct SubState {
static constexpr auto make_transition_table()
{
// clang-format off
return hsm::transition_table(
* hsm::state<S1> + hsm::event<e2> = hsm::state<S1>
, * hsm::state<S2> + hsm::event<e2> = hsm::state<S2>
);
// clang-format on
}
};

struct MainState {
static constexpr auto make_transition_table()
{
// clang-format off
return hsm::transition_table(
* hsm::state<S1> + hsm::event<e1> = hsm::state<SubState>
);
// clang-format on
}
};
}

TEST(ReproducerTests2, shouldEnterSubStateWithMultipleRegions)
{
hsm::sm<MainState> sm;
sm.process_event(e1 {});
}

0 comments on commit 597b27e

Please sign in to comment.