diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index b4c1ec69339..3b9dafc5818 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -119,6 +119,7 @@ add_executable(test_bitcoin txvalidation_tests.cpp txvalidationcache_tests.cpp uint256_tests.cpp + util_check_tests.cpp util_string_tests.cpp util_tests.cpp util_threadnames_tests.cpp diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 1215f532b74..2e13d4226e1 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -251,9 +251,4 @@ FUZZ_TARGET(integer, .init = initialize_integer) } catch (const std::ios_base::failure&) { } } - - try { - CHECK_NONFATAL(b); - } catch (const NonFatalCheckError&) { - } } diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 580a6338a84..c8f5e87adfa 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -381,6 +381,12 @@ FUZZ_TARGET(rpc, .init = initialize_rpc) arguments.push_back(ConsumeRPCArgument(fuzzed_data_provider, good_data)); } try { + std::optional maybe_mock{}; + if (rpc_command == "echo") { + // Avoid aborting fuzzing for this specific test-only RPC with an + // intentional trigger_internal_bug + maybe_mock.emplace(); + } rpc_testing_setup->CallRPC(rpc_command, arguments); } catch (const UniValue& json_rpc_error) { const std::string error_msg{json_rpc_error.find_value("message").get_str()}; diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 87303d74174..5d81b4287dd 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -534,20 +534,23 @@ BOOST_AUTO_TEST_CASE(check_dup_param_names) make_rpc({{"p1", NAMED_ONLY}, {"p2", NAMED}}); make_rpc({{"p1", NAMED_ONLY}, {"p2", NAMED_ONLY}}); - // Error if parameters names are duplicates, unless one parameter is - // positional and the other is named and .also_positional is true. - BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", POSITIONAL}}), NonFatalCheckError); - make_rpc({{"p1", POSITIONAL}, {"p1", NAMED}}); - BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", NAMED_ONLY}}), NonFatalCheckError); - make_rpc({{"p1", NAMED}, {"p1", POSITIONAL}}); - BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED}}), NonFatalCheckError); - BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED_ONLY}}), NonFatalCheckError); - BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", POSITIONAL}}), NonFatalCheckError); - BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED}}), NonFatalCheckError); - BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + { + test_only_CheckFailuresAreExceptionsNotAborts mock_checks{}; + // Error if parameter names are duplicates, unless one parameter is + // positional and the other is named and .also_positional is true. + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", POSITIONAL}}), NonFatalCheckError); + make_rpc({{"p1", POSITIONAL}, {"p1", NAMED}}); + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + make_rpc({{"p1", NAMED}, {"p1", POSITIONAL}}); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", POSITIONAL}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED_ONLY}}), NonFatalCheckError); - // Make sure duplicate aliases are detected too. - BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p2|p1", NAMED_ONLY}}), NonFatalCheckError); + // Make sure duplicate aliases are detected too. + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p2|p1", NAMED_ONLY}}), NonFatalCheckError); + } } BOOST_AUTO_TEST_CASE(help_example) diff --git a/src/test/util_check_tests.cpp b/src/test/util_check_tests.cpp new file mode 100644 index 00000000000..93ac9194604 --- /dev/null +++ b/src/test/util_check_tests.cpp @@ -0,0 +1,33 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include + +BOOST_AUTO_TEST_SUITE(util_check_tests) + +BOOST_AUTO_TEST_CASE(check_pass) +{ + Assume(true); + Assert(true); + CHECK_NONFATAL(true); +} + +BOOST_AUTO_TEST_CASE(check_fail) +{ + // Disable aborts for easier testing here + test_only_CheckFailuresAreExceptionsNotAborts mock_checks{}; + + if constexpr (G_ABORT_ON_FAILED_ASSUME) { + BOOST_CHECK_EXCEPTION(Assume(false), NonFatalCheckError, HasReason{"Internal bug detected: false"}); + } else { + BOOST_CHECK_NO_THROW(Assume(false)); + } + BOOST_CHECK_EXCEPTION(Assert(false), NonFatalCheckError, HasReason{"Internal bug detected: false"}); + BOOST_CHECK_EXCEPTION(CHECK_NONFATAL(false), NonFatalCheckError, HasReason{"Internal bug detected: false"}); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/check.cpp b/src/util/check.cpp index 9f07fbe4780..77783c843d7 100644 --- a/src/util/check.cpp +++ b/src/util/check.cpp @@ -27,8 +27,13 @@ NonFatalCheckError::NonFatalCheckError(std::string_view msg, std::string_view fi { } +bool g_detail_test_only_CheckFailuresAreExceptionsNotAborts{false}; + void assertion_fail(std::string_view file, int line, std::string_view func, std::string_view assertion) { + if (g_detail_test_only_CheckFailuresAreExceptionsNotAborts) { + throw NonFatalCheckError{assertion, file, line, func}; + } auto str = strprintf("%s:%s %s: Assertion `%s' failed.\n", file, line, func, assertion); fwrite(str.data(), 1, str.size(), stderr); std::abort(); diff --git a/src/util/check.h b/src/util/check.h index 6548453a97e..1267d9b5826 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -21,7 +21,7 @@ constexpr bool G_FUZZING_BUILD{ false #endif }; -constexpr bool G_ABORT_ON_FAILED_ASSUME{ +constexpr bool G_ABORT_ON_FAILED_ASSUME{G_FUZZING_BUILD || #ifdef ABORT_ON_FAILED_ASSUME true #else @@ -46,6 +46,12 @@ inline bool EnableFuzzDeterminism() } } +extern bool g_detail_test_only_CheckFailuresAreExceptionsNotAborts; +struct test_only_CheckFailuresAreExceptionsNotAborts { + test_only_CheckFailuresAreExceptionsNotAborts() { g_detail_test_only_CheckFailuresAreExceptionsNotAborts = true; }; + ~test_only_CheckFailuresAreExceptionsNotAborts() { g_detail_test_only_CheckFailuresAreExceptionsNotAborts = false; }; +}; + std::string StrFormatInternalBug(std::string_view msg, std::string_view file, int line, std::string_view func); class NonFatalCheckError : public std::runtime_error @@ -54,11 +60,17 @@ public: NonFatalCheckError(std::string_view msg, std::string_view file, int line, std::string_view func); }; +/** Internal helper */ +void assertion_fail(std::string_view file, int line, std::string_view func, std::string_view assertion); + /** Helper for CHECK_NONFATAL() */ template T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, const char* func, const char* assertion) { if (!val) { + if constexpr (G_ABORT_ON_FAILED_ASSUME) { + assertion_fail(file, line, func, assertion); + } throw NonFatalCheckError{assertion, file, line, func}; } return std::forward(val); @@ -68,14 +80,11 @@ T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, co #error "Cannot compile without assertions!" #endif -/** Helper for Assert() */ -void assertion_fail(std::string_view file, int line, std::string_view func, std::string_view assertion); - /** Helper for Assert()/Assume() */ template constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) { - if (IS_ASSERT || std::is_constant_evaluated() || G_FUZZING_BUILD || G_ABORT_ON_FAILED_ASSUME) { + if (IS_ASSERT || std::is_constant_evaluated() || G_ABORT_ON_FAILED_ASSUME) { if (!val) { assertion_fail(file, line, func, assertion); } diff --git a/test/functional/rpc_misc.py b/test/functional/rpc_misc.py index f8384e5b413..139384b652e 100755 --- a/test/functional/rpc_misc.py +++ b/test/functional/rpc_misc.py @@ -15,6 +15,9 @@ from test_framework.util import ( from test_framework.authproxy import JSONRPCException +import http +import subprocess + class RpcMiscTest(BitcoinTestFramework): def set_test_params(self): @@ -24,11 +27,22 @@ class RpcMiscTest(BitcoinTestFramework): node = self.nodes[0] self.log.info("test CHECK_NONFATAL") - assert_raises_rpc_error( - -1, - 'Internal bug detected: request.params[9].get_str() != "trigger_internal_bug"', - lambda: node.echo(arg9='trigger_internal_bug'), - ) + msg_internal_bug = 'request.params[9].get_str() != "trigger_internal_bug"' + self.restart_node(0) # Required to flush the chainstate + try: + node.echo(arg9="trigger_internal_bug") + assert False # Must hit one of the exceptions below + except ( + subprocess.CalledProcessError, + http.client.CannotSendRequest, + http.client.RemoteDisconnected, + ): + self.log.info("Restart node after crash") + assert_equal(-6, node.process.wait(timeout=10)) + self.start_node(0) + except JSONRPCException as e: + assert_equal(e.error["code"], -1) + assert f"Internal bug detected: {msg_internal_bug}" in e.error["message"] self.log.info("test max arg size") ARG_SZ_COMMON = 131071 # Common limit, used previously in the test framework, serves as a regression test