diff --git a/extra/fuzzer/CMakeLists.txt b/extra/fuzzer/CMakeLists.txt index 20ed6433..2b81bceb 100644 --- a/extra/fuzzer/CMakeLists.txt +++ b/extra/fuzzer/CMakeLists.txt @@ -25,6 +25,7 @@ foreach(_file IN LISTS immer_fuzzers) add_executable(${_target} EXCLUDE_FROM_ALL "${_file}") set_target_properties(${_target} PROPERTIES OUTPUT_NAME ${_output}) target_compile_options(${_target} PUBLIC ${immer_fuzzing_engine}) + target_compile_definitions(${_target} PUBLIC IMMER_THROW_ON_INVALID_STATE=1) target_link_libraries(${_target} PUBLIC ${immer_fuzzing_engine} immer-dev) add_dependencies(fuzzers ${_target}) if(CHECK_FUZZERS) diff --git a/extra/fuzzer/flex-vector-st.cpp b/extra/fuzzer/flex-vector-st.cpp index 4c2893e3..30f48ebd 100644 --- a/extra/fuzzer/flex-vector-st.cpp +++ b/extra/fuzzer/flex-vector-st.cpp @@ -42,8 +42,25 @@ extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, auto is_valid_size = [](auto& v) { return [&](auto idx) { return idx >= 0 && idx <= v.size(); }; }; - auto can_concat = [](auto&& v1, auto&& v2) { - return v1.size() + v2.size() < vector_t::max_size() / 4; + auto can_concat = [](const auto& v1, const auto& v2) { + // First, check max_size + if (v1.size() + v2.size() > vector_t::max_size()) { + return false; + } + + // But just checking max_size is not sufficient, because there are other + // conditions for the validity of the tree, like shift constraints, for + // example. + try { + // Try to concat and catch an exception if it fails + const auto v3 = v1 + v2; + if (v3.size()) { + return true; + } + } catch (const immer::detail::rbts::invalid_tree&) { + return false; + } + return true; }; auto can_compare = [](auto&& v) { // avoid comparing vectors that are too big, and hence, slow to compare diff --git a/immer/config.hpp b/immer/config.hpp index af34e5f1..9942c134 100644 --- a/immer/config.hpp +++ b/immer/config.hpp @@ -121,6 +121,10 @@ #endif #endif +#ifndef IMMER_THROW_ON_INVALID_STATE +#define IMMER_THROW_ON_INVALID_STATE 0 +#endif + namespace immer { const auto default_bits = 5; diff --git a/immer/detail/rbts/rrbtree.hpp b/immer/detail/rbts/rrbtree.hpp index 843921ba..ab02a20a 100644 --- a/immer/detail/rbts/rrbtree.hpp +++ b/immer/detail/rbts/rrbtree.hpp @@ -25,6 +25,16 @@ namespace immer { namespace detail { namespace rbts { +#if IMMER_THROW_ON_INVALID_STATE +struct invalid_tree : std::exception +{}; +#define IMMER_INVALID_STATE_ASSERT(expr) \ + if (!(expr)) \ + IMMER_THROW(invalid_tree{}) +#else +#define IMMER_INVALID_STATE_ASSERT(expr) assert(expr) +#endif + template struct rrbtree_iterator; @@ -107,13 +117,32 @@ struct rrbtree assert(check_tree()); } - rrbtree(size_t sz, shift_t sh, node_t* r, node_t* t) noexcept + rrbtree(size_t sz, shift_t sh, node_t* r, node_t* t) +#if IMMER_THROW_ON_INVALID_STATE +#else + noexcept +#endif : size{sz} , shift{sh} , root{r} , tail{t} { +#if IMMER_THROW_ON_INVALID_STATE + // assert only happens in the Debug build, but when + // IMMER_THROW_ON_INVALID_STATE is activated, we want to just check_tree + // even in Release. + try { + check_tree(); + } catch (...) { + // This not fully constructed rrbtree owns the nodes already, have + // to dec them. + if (r && t) + dec(); + throw; + } +#else assert(check_tree()); +#endif } rrbtree(const rrbtree& other) noexcept @@ -1339,13 +1368,13 @@ struct rrbtree bool check_tree() const { - assert(shift <= sizeof(size_t) * 8 - BL); - assert(shift >= BL); - assert(tail_offset() <= size); - assert(tail_size() <= branches); + IMMER_INVALID_STATE_ASSERT(shift <= sizeof(size_t) * 8 - BL); + IMMER_INVALID_STATE_ASSERT(shift >= BL); + IMMER_INVALID_STATE_ASSERT(tail_offset() <= size); + IMMER_INVALID_STATE_ASSERT(tail_size() <= branches); #if IMMER_DEBUG_DEEP_CHECK - assert(check_root()); - assert(check_tail()); + IMMER_INVALID_STATE_ASSERT(check_root()); + IMMER_INVALID_STATE_ASSERT(check_tail()); #endif return true; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index a6c1d499..5473f4a8 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -27,8 +27,8 @@ foreach(_file IN LISTS immer_unit_tests) add_dependencies(tests ${_target}) target_compile_definitions( ${_target} - PUBLIC - -DIMMER_OSS_FUZZ_DATA_PATH="${CMAKE_CURRENT_SOURCE_DIR}/oss-fuzz/data") + PUBLIC IMMER_OSS_FUZZ_DATA_PATH="${CMAKE_CURRENT_SOURCE_DIR}/oss-fuzz/data" + IMMER_THROW_ON_INVALID_STATE=1) target_link_libraries(${_target} PUBLIC immer-dev Catch2::Catch2WithMain) set_target_properties(${_target} PROPERTIES CXX_VISIBILITY_PRESET hidden) add_test("test/${_output}" ${_output}) diff --git a/test/oss-fuzz/flex-vector-st-0.cpp b/test/oss-fuzz/flex-vector-st-0.cpp index 6379c1dd..bd706a8a 100644 --- a/test/oss-fuzz/flex-vector-st-0.cpp +++ b/test/oss-fuzz/flex-vector-st-0.cpp @@ -63,16 +63,25 @@ int run_input(const std::uint8_t* data, std::size_t size) auto is_valid_size = [](auto& v) { return [&](auto idx) { return idx >= 0 && idx <= v.size(); }; }; - auto can_concat = [](auto&& v1, auto&& v2) { - const bool result = v1.size() + v2.size() < vector_t::max_size() / 4; - if (result) { - IMMER_FUZZED_TRACE("can_concat {} and {} (total is {}), max is {}", - v1.size(), - v2.size(), - v1.size() + v2.size(), - vector_t::max_size() / 4); - } - return result; + auto can_concat = [](const auto& v1, const auto& v2) { + // First, check max_size + if (v1.size() + v2.size() > vector_t::max_size()) { + return false; + } + + // But just checking max_size is not sufficient, because there are other + // conditions for the validity of the tree, like shift constraints, for + // example. + try { + // Try to concat and catch an exception if it fails + const auto v3 = v1 + v2; + if (v3.size()) { + return true; + } + } catch (const immer::detail::rbts::invalid_tree&) { + return false; + } + return true; }; auto can_compare = [](auto&& v) { // avoid comparing vectors that are too big, and hence, slow to compare @@ -241,3 +250,22 @@ TEST_CASE("some fuzzer crash I don't have a link for") CHECK(run_input(input.data(), input.size()) == 0); } } + +TEST_CASE("Creating rrbtree with invalid parameters shouldn't terminate but " + "should throw") +{ + SECTION("Invalid shift") + { + // assert(shift <= sizeof(size_t) * 8 - BL); + // assert(shift >= BL); + const auto create = [] { + const auto shift = immer::detail::rbts::shift_t{999}; + return immer::detail::rbts:: + rrbtree{ + 0, shift, nullptr, nullptr}; + }; + + REQUIRE(IMMER_THROW_ON_INVALID_STATE == 1); + REQUIRE_THROWS_AS(create(), immer::detail::rbts::invalid_tree); + } +}