From 1d84469f5b4ae3c24ecaa175c5be2771137ed0c4 Mon Sep 17 00:00:00 2001 From: Thorulf Neustrup Date: Thu, 23 Feb 2023 11:21:32 +0100 Subject: [PATCH 1/3] Remove constructor and destructor --- include/utap/document.h | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/include/utap/document.h b/include/utap/document.h index 8590d077..e6b591bd 100644 --- a/include/utap/document.h +++ b/include/utap/document.h @@ -248,20 +248,6 @@ struct simregion_t : stringify_t int get_loc() const; bool is_in_prechart() const; - simregion_t() - { - message = new message_t(); - condition = new condition_t(); - update = new update_t(); - } - - ~simregion_t() noexcept - { - delete message; - delete condition; - delete update; - } - std::ostream& print(std::ostream&) const; bool has_message() const { return message != nullptr && !message->empty(); } bool has_condition() const { return condition != nullptr && !condition->empty(); } From 5d587547ef048fdd336e57bdbcb195e8b50e97c7 Mon Sep 17 00:00:00 2001 From: Thorulf Neustrup Date: Thu, 13 Jul 2023 09:20:52 +0200 Subject: [PATCH 2/3] Added testing and default initialize to nullptr --- include/utap/document.h | 12 +-- src/document.cpp | 6 ++ test/models/lsc_example.xml | 183 ++++++++++++++++++++++++++++++++++++ test/test_parser.cpp | 12 +++ 4 files changed, 207 insertions(+), 6 deletions(-) create mode 100644 test/models/lsc_example.xml diff --git a/include/utap/document.h b/include/utap/document.h index 869cab96..05978d79 100644 --- a/include/utap/document.h +++ b/include/utap/document.h @@ -241,17 +241,17 @@ struct update_t : LSC_element_t struct simregion_t : stringify_t { int nr{}; - message_t* message; /** May be empty */ - condition_t* condition; /** May be empty */ - update_t* update; /** May be empty */ + message_t* message{nullptr}; /** May be empty */ + condition_t* condition{nullptr}; /** May be empty */ + update_t* update{nullptr}; /** May be empty */ int get_loc() const; bool is_in_prechart() const; std::ostream& print(std::ostream&) const; - bool has_message() const { return message != nullptr && !message->empty(); } - bool has_condition() const { return condition != nullptr && !condition->empty(); } - bool has_update() const { return update != nullptr && !update->empty(); } + bool has_message() const { return message != nullptr; } + bool has_condition() const { return condition != nullptr; } + bool has_update() const { return update != nullptr; } void set_message(std::deque& messages, int nr); void set_condition(std::deque& conditions, int nr); void set_update(std::deque& updates, int nr); diff --git a/src/document.cpp b/src/document.cpp index e964e6fe..7c0dc5ea 100644 --- a/src/document.cpp +++ b/src/document.cpp @@ -621,6 +621,8 @@ bool simregion_t::is_in_prechart() const void simregion_t::set_message(std::deque& messages, int nr) { + assert(nr != -1); + for (auto& message : messages) { if (message.nr == nr) { this->message = &message; @@ -631,6 +633,8 @@ void simregion_t::set_message(std::deque& messages, int nr) void simregion_t::set_condition(std::deque& conditions, int nr) { + assert(nr != -1); + for (auto& condition : conditions) { if (condition.nr == nr) { this->condition = &condition; @@ -641,6 +645,8 @@ void simregion_t::set_condition(std::deque& conditions, int nr) void simregion_t::set_update(std::deque& updates, int nr) { + assert(nr != -1); + for (auto& update : updates) { if (update.nr == nr) { this->update = &update; diff --git a/test/models/lsc_example.xml b/test/models/lsc_example.xml new file mode 100644 index 00000000..f953ec7a --- /dev/null +++ b/test/models/lsc_example.xml @@ -0,0 +1,183 @@ + + + + // Place global declarations here. +chan m1, m3, m4, m2; +clock x; + + + + + + LscTemplate + int a, int b + Universal + Invariant + // Place local declarations here. + + + + + + + + + D + + + C + + + B + + + A + + + 2 + + + + + 3 + + + + + + 4 + + + + + + 1 + + + + + 1 + cold + + + + + 3 + hot + + + + // Place template instantiations here. +Scenario = LscTemplate(2,3); + +// List one or more processes to be composed into a system. +system A, B, C, D; + + + + sat: Scenario + + + + + + diff --git a/test/test_parser.cpp b/test/test_parser.cpp index 799647b2..f3b529c5 100644 --- a/test/test_parser.cpp +++ b/test/test_parser.cpp @@ -253,4 +253,16 @@ TEST_CASE("Test leads to token is parsed correctly") { auto f = document_fixture{}.add_default_process().build_query_fixture(); CHECK_NOTHROW(f.parse_query("true --> true")); +} + +TEST_CASE("Sim region cleanup causes memory errors (run with ASAN)") +{ + auto doc = read_document("lsc_example.xml"); + REQUIRE(doc); + auto& errors = doc->get_errors(); + CHECK(errors.size() == 0); + + auto& templ = doc->get_templates().back(); + auto sims = templ.get_simregions(); + CHECK(sims.size() == 3); } \ No newline at end of file From 90553e515cbdc5e483b3e1af9ee5293033beb2f7 Mon Sep 17 00:00:00 2001 From: Thorulf Neustrup Date: Wed, 9 Aug 2023 12:37:38 +0200 Subject: [PATCH 3/3] Strengthen non empty gurantees with unsigned integer types --- include/utap/document.h | 17 ++++++++++------- src/document.cpp | 38 +++++++++++++------------------------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/include/utap/document.h b/include/utap/document.h index 05978d79..62f1c0dd 100644 --- a/include/utap/document.h +++ b/include/utap/document.h @@ -202,11 +202,11 @@ struct instance_line_t; // to be defined later /** Common members among LSC elements */ struct LSC_element_t { - int nr{-1}; /**< Placement in input file */ + uint32_t nr; /**< Placement in input file */ int location{-1}; bool is_in_prechart{false}; + explicit LSC_element_t(uint32_t nr): nr{nr} {} int get_nr() const { return nr; } - bool empty() const { return nr == -1; } }; /** Information about a message. Messages have a source (src) and a @@ -218,6 +218,7 @@ struct message_t : LSC_element_t instance_line_t* src{nullptr}; /**< Pointer to source instance line */ instance_line_t* dst{nullptr}; /**< Pointer to destination instance line */ expression_t label; /**< The label */ + explicit message_t(uint32_t nr): LSC_element_t{nr} {} }; /** Information about a condition. Conditions have an anchor instance lines. * The label is stored as an expression. @@ -227,6 +228,7 @@ struct condition_t : LSC_element_t std::vector anchors{}; /**< Pointer to anchor instance lines */ expression_t label; /**< The label */ bool isHot{false}; + explicit condition_t(uint32_t nr): LSC_element_t{nr} {} }; /** Information about an update. Update have an anchor instance line. @@ -236,11 +238,12 @@ struct update_t : LSC_element_t { instance_line_t* anchor{nullptr}; /**< Pointer to anchor instance line */ expression_t label; /**< The label */ + explicit update_t(uint32_t nr): LSC_element_t{nr} {} }; struct simregion_t : stringify_t { - int nr{}; + uint32_t nr{}; message_t* message{nullptr}; /** May be empty */ condition_t* condition{nullptr}; /** May be empty */ update_t* update{nullptr}; /** May be empty */ @@ -252,9 +255,9 @@ struct simregion_t : stringify_t bool has_message() const { return message != nullptr; } bool has_condition() const { return condition != nullptr; } bool has_update() const { return update != nullptr; } - void set_message(std::deque& messages, int nr); - void set_condition(std::deque& conditions, int nr); - void set_update(std::deque& updates, int nr); + void set_message(std::deque& messages, uint32_t nr); + void set_condition(std::deque& conditions, uint32_t nr); + void set_update(std::deque& updates, uint32_t nr); }; struct compare_simregion @@ -339,7 +342,7 @@ struct instance_t */ struct instance_line_t : public instance_t { - int32_t instance_nr; /**< InstanceLine number in template */ + uint32_t instance_nr; /**< InstanceLine number in template */ std::vector getSimregions(const std::vector& simregions); void add_parameters(instance_t& inst, frame_t params, const std::vector& arguments); }; diff --git a/src/document.cpp b/src/document.cpp index 7c0dc5ea..2057be91 100644 --- a/src/document.cpp +++ b/src/document.cpp @@ -340,37 +340,34 @@ instance_line_t& template_t::add_instance_line() message_t& template_t::add_message(symbol_t src, symbol_t dst, int loc, bool pch) { int32_t nr = messages.empty() ? 0 : messages.back().nr + 1; - auto& message = messages.emplace_back(); + auto& message = messages.emplace_back(nr); message.src = static_cast(src.get_data()); message.dst = static_cast(dst.get_data()); message.location = loc; message.is_in_prechart = pch; - message.nr = nr; return message; } update_t& template_t::add_update(symbol_t anchor, int loc, bool pch) { int32_t nr = updates.empty() ? 0 : updates.back().nr + 1; - auto& update = updates.emplace_back(); + auto& update = updates.emplace_back(nr); update.anchor = static_cast(anchor.get_data()); update.location = loc; update.is_in_prechart = pch; - update.nr = nr; return update; } condition_t& template_t::add_condition(vector anchors, int loc, bool pch, bool isHot) { int32_t nr = conditions.empty() ? 0 : conditions.back().nr + 1; - auto& condition = conditions.emplace_back(); + auto& condition = conditions.emplace_back(nr); for (auto& anchor : anchors) { condition.anchors.push_back(static_cast(anchor.get_data())); // TODO } condition.location = loc; condition.is_in_prechart = pch; - condition.nr = nr; condition.isHot = isHot; return condition; } @@ -532,7 +529,6 @@ bool template_t::get_update(instance_line_t& instance, int y, update_t*& simUpda */ bool template_t::get_update(vector& instances, int y, update_t*& simUpdate) { - update_t update; for (auto& instance : instances) { if (get_update(*instance, y, simUpdate)) return true; @@ -563,24 +559,22 @@ vector instance_line_t::getSimregions(const vector& si // get the simregions anchored to this instance for (const auto& reg : simregions) { const message_t* m = reg.message; - if (!m->empty() && (m->src->instance_nr == this->instance_nr || m->dst->instance_nr == this->instance_nr)) { + if ((m->src->instance_nr == this->instance_nr || m->dst->instance_nr == this->instance_nr)) { i_simregions.push_back(reg); continue; } const update_t* u = reg.update; - if (!u->empty() && u->anchor->instance_nr == this->instance_nr) { + if (u->anchor->instance_nr == this->instance_nr) { i_simregions.push_back(reg); continue; } const condition_t* c = reg.condition; - if (!c->empty()) { - for (auto* instance : c->anchors) { - if (instance->instance_nr == this->instance_nr) { - i_simregions.push_back(reg); - break; - } + for (auto* instance : c->anchors) { + if (instance->instance_nr == this->instance_nr) { + i_simregions.push_back(reg); + break; } } } @@ -619,10 +613,8 @@ bool simregion_t::is_in_prechart() const return false; // should not happen } -void simregion_t::set_message(std::deque& messages, int nr) +void simregion_t::set_message(std::deque& messages, uint32_t nr) { - assert(nr != -1); - for (auto& message : messages) { if (message.nr == nr) { this->message = &message; @@ -631,10 +623,8 @@ void simregion_t::set_message(std::deque& messages, int nr) } } -void simregion_t::set_condition(std::deque& conditions, int nr) +void simregion_t::set_condition(std::deque& conditions, uint32_t nr) { - assert(nr != -1); - for (auto& condition : conditions) { if (condition.nr == nr) { this->condition = &condition; @@ -643,10 +633,8 @@ void simregion_t::set_condition(std::deque& conditions, int nr) } } -void simregion_t::set_update(std::deque& updates, int nr) +void simregion_t::set_update(std::deque& updates, uint32_t nr) { - assert(nr != -1); - for (auto& update : updates) { if (update.nr == nr) { this->update = &update; @@ -681,7 +669,7 @@ std::ostream& simregion_t::print(std::ostream& os) const return os << ")"; } -inline auto find_simregion_by_nr(int nr) +inline auto find_simregion_by_nr(uint32_t nr) { return [nr](const simregion_t& reg) { return reg.nr == nr; }; }