CORE-cer / CORE

Implementation of CORE in cpp
GNU General Public License v3.0
8 stars 2 forks source link

Crashing in RemoveStates when doing RemoveUnreachableStates #37

Closed nicobuzeta closed 1 year ago

nicobuzeta commented 1 year ago

Appears to be a bug in that causes segmentation fault with long query with OR.

No crash:

std::string string_query =
  "SELECT * FROM Stock\n"
  "WHERE SELL: (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY)";

Crash:

std::string string_query =
  "SELECT * FROM Stock\n"
  "WHERE SELL: (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY)";
Valgrind ``` ==1068091== Invalid read of size 8 ==1068091== at 0x173FF5: CORE::Internal::CEA::RemoveStates::map_states(__gmp_expr<__mpz_struct [1], __mpz_struct [1]>, std::vector >) (remove_states.hpp:93) ==1068091== by 0x173936: CORE::Internal::CEA::RemoveStates::eval(CORE::Internal::CEA::LogicalCEA&&) (remove_states.hpp:41) ==1068091== by 0x188CA2: CORE::Internal::CEA::LogicalCEATransformer::operator()(CORE::Internal::CEA::LogicalCEA&&) (logical_cea_transformer.hpp:16) ==1068091== by 0x174643: CORE::Internal::CEA::RemoveUnreachableStates::eval(CORE::Internal::CEA::LogicalCEA&&) (remove_unreachable_states.hpp:58) ==1068091== by 0x188EFE: CORE::Internal::CEA::LogicalCEATransformer::operator()(CORE::Internal::CEA::LogicalCEA&&) (logical_cea_transformer.hpp:16) ==1068091== by 0x1755A1: CORE::Internal::CEA::CEA::CEA(CORE::Internal::CEA::LogicalCEA&&) (cea.hpp:32) ==1068091== by 0x2B717F: CORE::Internal::Evaluation::UnitTests::CATCH2_INTERNAL_TEST_100() (evaluation_algorithm.cpp:739) ==1068091== by 0x3D1E39: Catch::TestInvokerAsFunction::invoke() const (catch_test_case_registry_impl.cpp:149) ==1068091== by 0x3C7444: Catch::TestCaseHandle::invoke() const (catch_test_case_info.hpp:115) ==1068091== by 0x3C64F7: Catch::RunContext::invokeActiveTestCase() (catch_run_context.cpp:538) ==1068091== by 0x3C6213: Catch::RunContext::runCurrentTest(std::__cxx11::basic_string, std::allocator >&, std::__cxx11::basic_string, std::allocator >&) (catch_run_context.cpp:501) ==1068091== by 0x3C4845: Catch::RunContext::runTest(Catch::TestCaseHandle const&) (catch_run_context.cpp:232) ==1068091== Address 0x505c1d8 is 0 bytes after a block of size 280 alloc'd ==1068091== at 0x4841FC3: operator new(unsigned long) (vg_replace_malloc.c:472) ==1068091== by 0x1A8685: std::__new_allocator::allocate(unsigned long, void const*) (new_allocator.h:147) ==1068091== by 0x19C0DB: allocate (allocator.h:198) ==1068091== by 0x19C0DB: allocate (alloc_traits.h:482) ==1068091== by 0x19C0DB: std::_Vector_base >::_M_allocate(unsigned long) (stl_vector.h:378) ==1068091== by 0x19AC3E: std::_Vector_base >::_M_create_storage(unsigned long) (stl_vector.h:395) ==1068091== by 0x192502: std::_Vector_base >::_Vector_base(unsigned long, std::allocator const&) (stl_vector.h:332) ==1068091== by 0x1889AA: std::vector >::vector(std::vector > const&) (stl_vector.h:600) ==1068091== by 0x1738FE: CORE::Internal::CEA::RemoveStates::eval(CORE::Internal::CEA::LogicalCEA&&) (remove_states.hpp:41) ==1068091== by 0x188CA2: CORE::Internal::CEA::LogicalCEATransformer::operator()(CORE::Internal::CEA::LogicalCEA&&) (logical_cea_transformer.hpp:16) ==1068091== by 0x174643: CORE::Internal::CEA::RemoveUnreachableStates::eval(CORE::Internal::CEA::LogicalCEA&&) (remove_unreachable_states.hpp:58) ==1068091== by 0x188EFE: CORE::Internal::CEA::LogicalCEATransformer::operator()(CORE::Internal::CEA::LogicalCEA&&) (logical_cea_transformer.hpp:16) ==1068091== by 0x1755A1: CORE::Internal::CEA::CEA::CEA(CORE::Internal::CEA::LogicalCEA&&) (cea.hpp:32) ==1068091== by 0x2B717F: CORE::Internal::Evaluation::UnitTests::CATCH2_INTERNAL_TEST_100() (evaluation_algorithm.cpp:739) ```

Crashes when removing first SELL but not if remove SELL and (SELL OR BUY). Must depend on amount of ORs.

std::string string_query =
  "SELECT * FROM Stock\n"
  "WHERE (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY): (SELL OR BUY)";

Replacing all continuous : with contiguous ; also crashes.

nicobuzeta commented 1 year ago

image

current_node in map_states is wrong.

nicobuzeta commented 1 year ago

Issue was that 1 << number sometimes overflowed. This was an oversight as the result was being saved to an mpz_class but the bit shift itself was being performed the 1 not on an mpz_class. All 1 << number hace been changed to mpz_class(1).

Might need to revisit in the future to change the mpz_class to be saved beforehand and reused, so we don't allocate a new mpz_class on every shift. For now works and doubtful of performance impact as all bit shifts inside of fors occur before automata evaluation code.