From 18f43fa00e80cd1eb762a77f309c5a581a86cd0a Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Wed, 13 Apr 2022 10:33:19 +0200 Subject: [PATCH] utils/exceptions: add try_catch Introduces a utility function which allows obtaining a pointer to the exception data held behind an std::exception_ptr if the data matches the requested type. It can be used to implement manual but concise try..catch chains. The `try_catch` has the best performance when used with libstdc++ as it uses the stdlib specific functions for simulating a try..catch without having to actually throw. For other stdlibs, the implementation falls back to a throw surrounded by an actual try..catch. --- configure.py | 4 + test/boost/exceptions_fallback_test.cc | 11 ++ test/boost/exceptions_optimized_test.cc | 30 ++++++ test/boost/exceptions_test.inc.cc | 137 ++++++++++++++++++++++++ utils/exceptions.cc | 23 ++++ utils/exceptions.hh | 48 +++++++++ 6 files changed, 253 insertions(+) create mode 100644 test/boost/exceptions_fallback_test.cc create mode 100644 test/boost/exceptions_optimized_test.cc create mode 100644 test/boost/exceptions_test.inc.cc diff --git a/configure.py b/configure.py index da90966366..901e58c92e 100755 --- a/configure.py +++ b/configure.py @@ -508,6 +508,8 @@ scylla_tests = set([ 'test/boost/rate_limiter_test', 'test/boost/per_partition_rate_limit_test', 'test/boost/expr_test', + 'test/boost/exceptions_optimized_test', + 'test/boost/exceptions_fallback_test', 'test/manual/ec2_snitch_test', 'test/manual/enormous_table_scan_test', 'test/manual/gce_snitch_test', @@ -1290,6 +1292,8 @@ deps['test/boost/linearizing_input_stream_test'] = [ ] deps['test/boost/expr_test'] = ['test/boost/expr_test.cc'] + scylla_core deps['test/boost/rate_limiter_test'] = ['test/boost/rate_limiter_test.cc', 'db/rate_limiter.cc'] +deps['test/boost/exceptions_optimized_test'] = ['test/boost/exceptions_optimized_test.cc', 'utils/exceptions.cc'] +deps['test/boost/exceptions_fallback_test'] = ['test/boost/exceptions_fallback_test.cc', 'utils/exceptions.cc'] deps['test/boost/duration_test'] += ['test/lib/exception_utils.cc'] deps['test/boost/schema_loader_test'] += ['tools/schema_loader.cc'] diff --git a/test/boost/exceptions_fallback_test.cc b/test/boost/exceptions_fallback_test.cc new file mode 100644 index 0000000000..9f4627a4a4 --- /dev/null +++ b/test/boost/exceptions_fallback_test.cc @@ -0,0 +1,11 @@ +/* + * Copyright (C) 2022-present ScyllaDB + */ + +/* + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +#define NO_OPTIMIZED_EXCEPTION_HANDLING + +#include "exceptions_test.inc.cc" diff --git a/test/boost/exceptions_optimized_test.cc b/test/boost/exceptions_optimized_test.cc new file mode 100644 index 0000000000..6db2b59148 --- /dev/null +++ b/test/boost/exceptions_optimized_test.cc @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2022-present ScyllaDB + */ + +/* + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +#if defined(NO_OPTIMIZED_EXCEPTION_HANDLING) +#undef NO_OPTIMIZED_EXCEPTION_HANDLING +#endif + +#include "utils/exceptions.hh" + +#if defined(OPTIMIZED_EXCEPTION_HANDLING_AVAILABLE) + +#include "exceptions_test.inc.cc" + +#else + +#include +#include + +SEASTAR_TEST_CASE(test_noop) { + BOOST_TEST_MESSAGE("Optimized implementation of handling exceptions " + "without throwing is not available. Skipping tests in this file."); + return seastar::make_ready_future<>(); +} + +#endif diff --git a/test/boost/exceptions_test.inc.cc b/test/boost/exceptions_test.inc.cc new file mode 100644 index 0000000000..a6bc1cfc5d --- /dev/null +++ b/test/boost/exceptions_test.inc.cc @@ -0,0 +1,137 @@ +/* + * Copyright (C) 2022-present ScyllaDB + */ + +/* + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +// Common definitions of test cases used in +// handle_exception_optimized_test.cc +// handle_exception_fallback_test.cc + +#include +#include +#include +#include +#include +#include +#include + +#include "seastarx.hh" +#include +#include +#include + +#include "utils/exceptions.hh" + +class base_exception : public std::exception {}; +class derived_exception : public base_exception {}; + +static void dummy_fn(void*) { + // +} + +template +static std::exception_ptr maybe_wrap_eptr(T&& t) { + if constexpr (std::is_same_v) { + return std::move(t); + } else { + return std::make_exception_ptr(std::move(t)); + } +} + +static const std::type_info& eptr_typeid(std::exception_ptr eptr) { + try { + std::rethrow_exception(eptr); + } catch (...) { + return *abi::__cxa_current_exception_type(); + } +} + +template +static void check_catch(Throw&& ex) { + auto eptr = maybe_wrap_eptr(std::move(ex)); + BOOST_TEST_MESSAGE("Checking if " << seastar::pretty_type_name(eptr_typeid(eptr)) + << " is caught as " << seastar::pretty_type_name(typeid(Capture))); + + auto typed_eptr = try_catch(eptr); + BOOST_CHECK_NE(typed_eptr, nullptr); + + // Verify that it's the same as what the usual throwing gives + // TODO: Does this check make sense? Does the standard guarantee + // that this will give the same pointer? + try { + std::rethrow_exception(eptr); + } catch (Capture& t) { + BOOST_CHECK_EQUAL(typed_eptr, &t); + } catch (...) { + // Can happen if the first check fails, just skip + assert(typed_eptr == nullptr); + } +} + +template +static void check_no_catch(Throw&& ex) { + auto eptr = maybe_wrap_eptr(std::move(ex)); + BOOST_TEST_MESSAGE("Checking if " << seastar::pretty_type_name(eptr_typeid(eptr)) + << " is NOT caught as " << seastar::pretty_type_name(typeid(Capture))); + + auto typed_eptr = try_catch(eptr); + BOOST_CHECK_EQUAL(typed_eptr, nullptr); +} + +template +static std::exception_ptr make_nested_eptr(A&& a, B&& b) { + try { + throw std::move(b); + } catch (...) { + try { + std::throw_with_nested(std::move(a)); + } catch (...) { + return std::current_exception(); + } + } +} + +SEASTAR_TEST_CASE(test_try_catch) { + // Some standard examples, throwing exceptions derived from std::exception + // and catching them through their base types + + check_catch(derived_exception()); + check_catch(derived_exception()); + check_catch(derived_exception()); + check_no_catch(derived_exception()); + + check_no_catch(base_exception()); + check_catch(base_exception()); + check_catch(base_exception()); + check_no_catch(base_exception()); + + // Catching nested exceptions + check_catch(make_nested_eptr(base_exception(), derived_exception())); + check_catch(make_nested_eptr(base_exception(), derived_exception())); + check_no_catch(make_nested_eptr(base_exception(), derived_exception())); + + // Check that everything works if we throw some crazy stuff + check_catch(int(1)); + check_no_catch(int(1)); + + check_no_catch(nullptr); + check_no_catch(nullptr); + + // Catching pointers is not supported, but nothing should break if they are + // being thrown + derived_exception exc; + check_no_catch(&exc); + check_no_catch(&exc); + + check_no_catch(&dummy_fn); + check_no_catch(&dummy_fn); + + check_no_catch(&std::exception::what); + check_no_catch(&std::exception::what); + + return make_ready_future<>(); +} + diff --git a/utils/exceptions.cc b/utils/exceptions.cc index 0e8006cb1b..4b5a3d7c47 100644 --- a/utils/exceptions.cc +++ b/utils/exceptions.cc @@ -15,6 +15,7 @@ #include #include #include "exceptions.hh" +#include "utils/abi/eh_ia64.hh" #include @@ -73,3 +74,25 @@ bool is_timeout_exception(std::exception_ptr e) { } return false; } + +#if defined(OPTIMIZED_EXCEPTION_HANDLING_AVAILABLE) + +#include +#include "utils/abi/eh_ia64.hh" + +void* utils::internal::try_catch_dynamic(std::exception_ptr& eptr, const std::type_info* catch_type) noexcept { + // In both libstdc++ and libc++, exception_ptr has just one field + // which is a pointer to the exception data + void* raw_ptr = reinterpret_cast(eptr); + const std::type_info* ex_type = utils::abi::get_cxa_exception(raw_ptr)->exceptionType; + + // __do_catch can return true and set raw_ptr to nullptr, but only in the case + // when catch_type is a pointer and a nullptr is thrown. try_catch_dynamic + // doesn't work with catching pointers. + if (catch_type->__do_catch(ex_type, &raw_ptr, 1)) { + return raw_ptr; + } + return nullptr; +} + +#endif // __GLIBCXX__ diff --git a/utils/exceptions.hh b/utils/exceptions.hh index 83714e5af6..8b98f20b49 100644 --- a/utils/exceptions.hh +++ b/utils/exceptions.hh @@ -8,11 +8,26 @@ #pragma once +#include + +#if defined(__GLIBCXX__) && (defined(__x86_64__) || defined(__aarch64__)) + #define OPTIMIZED_EXCEPTION_HANDLING_AVAILABLE +#endif + +#if !defined(NO_OPTIMIZED_EXCEPTION_HANDLING) + #if defined(OPTIMIZED_EXCEPTION_HANDLING_AVAILABLE) + #define USE_OPTIMIZED_EXCEPTION_HANDLING + #else + #warn "Fast implementation of some of the exception handling routines is not available for this platform. Expect poor exception handling performance." + #endif +#endif + #include #include #include #include +#include namespace seastar { class logger; } @@ -60,3 +75,36 @@ inline void maybe_rethrow_exception(std::exception_ptr ex) { std::rethrow_exception(std::move(ex)); } } + +namespace utils::internal { + +#if defined(OPTIMIZED_EXCEPTION_HANDLING_AVAILABLE) +void* try_catch_dynamic(std::exception_ptr& eptr, const std::type_info* catch_type) noexcept; +#endif + +} // utils::internal + +/// If the exception_ptr holds an exception which would match on a `catch (T&)` +/// clause, returns a pointer to it. Otherwise, returns `nullptr`. +/// +/// The exception behind the pointer is valid as long as the exception +/// behind the exception_ptr is valid. +template +inline T* try_catch(std::exception_ptr& eptr) noexcept { + static_assert(!std::is_pointer_v, "catching pointers is not supported"); + static_assert(!std::is_lvalue_reference_v && !std::is_rvalue_reference_v, + "T must not be a reference"); + +#if defined(USE_OPTIMIZED_EXCEPTION_HANDLING) + void* opt_ptr = utils::internal::try_catch_dynamic(eptr, &typeid(std::remove_const_t)); + return reinterpret_cast(opt_ptr); +#else + try { + std::rethrow_exception(eptr); + } catch (T& t) { + return &t; + } catch (...) { + } + return nullptr; +#endif +}