From adee091491b009ce5f2b6ae5091e987cb48d2a58 Mon Sep 17 00:00:00 2001 From: Jeremy Rifkin <51220084+jeremy-rifkin@users.noreply.github.com> Date: Tue, 20 Aug 2024 20:53:29 -0500 Subject: [PATCH] Add zero-overhead variants, CPPTRACE_CATCH_ALT, and unprefixed aliases --- CMakeLists.txt | 4 + README.md | 118 ++++++++++++++++++++---- cmake/OptionVariables.cmake | 1 + include/cpptrace/from_current.hpp | 34 +++++++ src/from_current.cpp | 25 ++++-- test/CMakeLists.txt | 1 + test/unit/from_current_z.cpp | 143 ++++++++++++++++++++++++++++++ 7 files changed, 303 insertions(+), 23 deletions(-) create mode 100644 test/unit/from_current_z.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 8f0d225..17dab79 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -317,6 +317,10 @@ if(NOT CPPTRACE_STD_FORMAT) target_compile_definitions(${target_name} PUBLIC CPPTRACE_NO_STD_FORMAT) endif() +if(CPPTRACE_UNPREFIXED_TRY_CATCH) + target_compile_definitions(${target_name} PUBLIC CPPTRACE_UNPREFIXED_TRY_CATCH) +endif() + if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") SET(CMAKE_C_ARCHIVE_FINISH " -no_warning_for_no_symbols -c ") SET(CMAKE_CXX_ARCHIVE_FINISH " -no_warning_for_no_symbols -c ") diff --git a/README.md b/README.md index c64a08a..c5487f2 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ Cpptrace also has a C API, docs [here](docs/c-api.md). - [Utilities](#utilities) - [Configuration](#configuration) - [Traces From All Exceptions](#traces-from-all-exceptions) + - [Removing the `CPPTRACE_` prefix](#removing-the-cpptrace_-prefix) - [How it works](#how-it-works) - [Performance](#performance) - [Traced Exception Objects](#traced-exception-objects) @@ -371,14 +372,14 @@ namespace cpptrace { ### Traces From All Exceptions Cpptrace provides `CPPTRACE_TRY` and `CPPTRACE_CATCH` macros that allow a stack trace to be collected from the current -thrown exception object, with no overhead in the non-throwing path: +thrown exception object, with minimal or no overhead in the non-throwing path: ```cpp CPPTRACE_TRY { foo(); } CPPTRACE_CATCH(const std::exception& e) { - std::cout<<"Exception: "< [!TIP] +> The choice between the `Z` and non-`Z` (zero-overhead and non-zero-overhead) variants of the exception handlers should +> not matter 99% of the time, however, both are provided in the rare case that it does. +> +> `CPPTRACE_TRY`/`CPPTRACE_CATCH` could only hurt performance if used in a hot loop where the compiler can't optimize +> away the internal bookkeeping, otherwise the bookkeeping should be completely negligible. +> +> `CPPTRACE_TRYZ`/`CPPTRACE_CATCHZ` could only hurt performance when there is an exceptionally deep nesting of exception +> handlers in a call stack before a matching handler. + +More information on performance considerations with the zero-overhead variant: + +Tracing the stack multiple times in throwing paths should not matter for the vast majority applications given that: +1. Performance very rarely is critical in throwing paths and exceptions should be exceptionally rare +2. Exception handling is not usually used in such a way that you could have a deep nesting of handlers before finding a + matching handler +3. An that most call stacks are fairly shallow To put the scale of this performance consideration into perspective: In my benchmarking I have found generation of raw traces to take on the order of `100ns` per frame. Thus, even if there were 100 non-matching handlers before a matching handler in a 100-deep call stack the total time would stil be on the order of one millisecond. -It's possible to avoid this by adding some bookkeeping to the `CPPTRACE_TRY` block. With the tradeoff between -zero-overhead try-catch in the happy path and a little extra overhead in the unhappy throwing path I decided to keep -try-catch zero-overhead. Should this be a concern to anyone, I'm happy to facilitate both solutions. +Nonetheless, I chose a default bookkeeping behavior for `CPPTRACE_TRY`/`CPPTRACE_CATCH` since it is safer with better +performance guarantees for the most general possible set of users. ### Traced Exception Objects diff --git a/cmake/OptionVariables.cmake b/cmake/OptionVariables.cmake index 1a4aa79..90ee11d 100644 --- a/cmake/OptionVariables.cmake +++ b/cmake/OptionVariables.cmake @@ -174,6 +174,7 @@ option(CPPTRACE_WERROR_BUILD "" OFF) option(CPPTRACE_POSITION_INDEPENDENT_CODE "" ON) option(CPPTRACE_SKIP_UNIT "" OFF) option(CPPTRACE_STD_FORMAT "" ON) +option(CPPTRACE_UNPREFIXED_TRY_CATCH "" OFF) option(CPPTRACE_USE_EXTERNAL_GTEST "" OFF) set(CPPTRACE_ZSTD_REPO "https://github.com/facebook/zstd.git" CACHE STRING "") set(CPPTRACE_ZSTD_TAG "794ea1b0afca0f020f4e57b6732332231fb23c70" CACHE STRING "") # v1.5.6 diff --git a/include/cpptrace/from_current.hpp b/include/cpptrace/from_current.hpp index 59497b5..9157999 100644 --- a/include/cpptrace/from_current.hpp +++ b/include/cpptrace/from_current.hpp @@ -41,11 +41,19 @@ namespace cpptrace { exception_unwind_interceptor(1); return 0; // EXCEPTION_CONTINUE_SEARCH } + CPPTRACE_FORCE_NO_INLINE inline int unconditional_exception_filter() { + collect_current_trace(1); + return 0; // EXCEPTION_CONTINUE_SEARCH + } #else class CPPTRACE_EXPORT unwind_interceptor { public: virtual ~unwind_interceptor(); }; + class CPPTRACE_EXPORT unconditional_unwind_interceptor { + public: + virtual ~unconditional_unwind_interceptor(); + }; CPPTRACE_EXPORT void do_prepare_unwind_interceptor(char(*)(std::size_t)); @@ -81,6 +89,16 @@ namespace cpptrace { } __except(::cpptrace::detail::exception_filter()) {} \ }(); \ } catch(param) + #define CPPTRACE_TRYZ \ + try { \ + [&]() { \ + __try { \ + [&]() { + #define CPPTRACE_CATCHZ(param) \ + }(); \ + } __except(::cpptrace::detail::unconditional_exception_filter()) {} \ + }(); \ + } catch(param) #else #define CPPTRACE_TRY \ try { \ @@ -92,6 +110,22 @@ namespace cpptrace { #define CPPTRACE_CATCH(param) \ } catch(::cpptrace::detail::unwind_interceptor&) {} \ } catch(param) + #define CPPTRACE_TRYZ \ + try { \ + try { + #define CPPTRACE_CATCHZ(param) \ + } catch(::cpptrace::detail::unconditional_unwind_interceptor&) {} \ + } catch(param) +#endif + +#define CPPTRACE_CATCH_ALT(param) catch(param) + +#ifdef CPPTRACE_UNPREFIXED_TRY_CATCH + #define TRY CPPTRACE_TRY + #define CATCH(param) CPPTRACE_CATCH(param) + #define TRYZ CPPTRACE_TRYZ + #define CATCHZ(param) CPPTRACE_CATCHZ(param) + #define CATCH_ALT(param) CPPTRACE_CATCH_ALT(param) #endif #endif diff --git a/src/from_current.cpp b/src/from_current.cpp index f86017c..87a5bb2 100644 --- a/src/from_current.cpp +++ b/src/from_current.cpp @@ -48,7 +48,16 @@ namespace cpptrace { return false; } + CPPTRACE_FORCE_NO_INLINE + bool unconditional_exception_unwind_interceptor(const std::type_info*, const std::type_info*, void**, unsigned) { + collect_current_trace(1); + return false; + } + + using do_catch_fn = decltype(intercept_unwind); + unwind_interceptor::~unwind_interceptor() = default; + unconditional_unwind_interceptor::~unconditional_unwind_interceptor() = default; #if IS_LIBSTDCXX constexpr size_t vtable_size = 11; @@ -194,10 +203,7 @@ namespace cpptrace { } #endif - // allocated below, cleaned up by OS after exit - void* new_vtable_page = nullptr; - - void perform_typeinfo_surgery(const std::type_info& info) { + void perform_typeinfo_surgery(const std::type_info& info, do_catch_fn* do_catch_function) { if(vtable_size == 0) { // set to zero if we don't know what standard library we're working with return; } @@ -243,12 +249,13 @@ namespace cpptrace { } // allocate a page for the new vtable so it can be made read-only later - new_vtable_page = allocate_page(page_size); + // the OS cleans this up, no cleanup done here for it + void* new_vtable_page = allocate_page(page_size); // make our own copy of the vtable memcpy(new_vtable_page, type_info_vtable_pointer, vtable_size * sizeof(void*)); // ninja in the custom __do_catch interceptor auto new_vtable = static_cast(new_vtable_page); - new_vtable[6] = reinterpret_cast(intercept_unwind); + new_vtable[6] = reinterpret_cast(do_catch_function); // make the page read-only mprotect_page(new_vtable_page, page_size, memory_readonly); @@ -273,7 +280,11 @@ namespace cpptrace { if(!did_prepare) { cpptrace::detail::intercept_unwind_handler = intercept_unwind_handler; try { - perform_typeinfo_surgery(typeid(cpptrace::detail::unwind_interceptor)); + perform_typeinfo_surgery(typeid(cpptrace::detail::unwind_interceptor), intercept_unwind); + perform_typeinfo_surgery( + typeid(cpptrace::detail::unconditional_unwind_interceptor), + unconditional_exception_unwind_interceptor + ); } catch(std::exception& e) { std::fprintf( stderr, diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 114e807..26b7255 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -84,6 +84,7 @@ if(NOT CPPTRACE_SKIP_UNIT) unit/object_trace.cpp unit/stacktrace.cpp unit/from_current.cpp + unit/from_current_z.cpp unit/traced_exception.cpp ) target_compile_features(unittest PRIVATE cxx_std_20) diff --git a/test/unit/from_current_z.cpp b/test/unit/from_current_z.cpp new file mode 100644 index 0000000..4a26ede --- /dev/null +++ b/test/unit/from_current_z.cpp @@ -0,0 +1,143 @@ +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include +#include + +using namespace std::literals; + + +// NOTE: returning something and then return stacktrace_from_current_3(line_numbers) * 2; later helps prevent the call from +// being optimized to a jmp +CPPTRACE_FORCE_NO_INLINE int stacktrace_from_current_z_3(std::vector& line_numbers) { + line_numbers.insert(line_numbers.begin(), __LINE__ + 1); + throw std::runtime_error("foobar"); +} + +CPPTRACE_FORCE_NO_INLINE int stacktrace_from_current_z_2(std::vector& line_numbers) { + line_numbers.insert(line_numbers.begin(), __LINE__ + 1); + return stacktrace_from_current_z_3(line_numbers) * 2; +} + +CPPTRACE_FORCE_NO_INLINE int stacktrace_from_current_z_1(std::vector& line_numbers) { + line_numbers.insert(line_numbers.begin(), __LINE__ + 1); + return stacktrace_from_current_z_2(line_numbers) * 2; +} + +TEST(FromCurrentZ, Basic) { + std::vector line_numbers; + CPPTRACE_TRYZ { + line_numbers.insert(line_numbers.begin(), __LINE__ + 1); + stacktrace_from_current_z_1(line_numbers); + } CPPTRACE_CATCHZ(const std::runtime_error& e) { + EXPECT_EQ(e.what(), "foobar"sv); + const auto& trace = cpptrace::from_current_exception(); + ASSERT_GE(trace.frames.size(), 4); + auto it = std::find_if( + trace.frames.begin(), + trace.frames.end(), + [](const cpptrace::stacktrace_frame& frame) { + return frame.filename.find("from_current_z.cpp") != std::string::npos + && frame.symbol.find("lambda") == std::string::npos; // due to msvc + } + ); + ASSERT_NE(it, trace.frames.end()); + size_t i = static_cast(it - trace.frames.begin()); + int j = 0; + ASSERT_LT(i, trace.frames.size()); + ASSERT_LT(j, line_numbers.size()); + EXPECT_THAT(trace.frames[i].filename, testing::EndsWith("from_current_z.cpp")); + EXPECT_EQ(trace.frames[i].line.value(), line_numbers[j]); + EXPECT_THAT(trace.frames[i].symbol, testing::HasSubstr("stacktrace_from_current_z_3")); + i++; + j++; + ASSERT_LT(i, trace.frames.size()); + ASSERT_LT(j, line_numbers.size()); + EXPECT_THAT(trace.frames[i].filename, testing::EndsWith("from_current_z.cpp")); + EXPECT_EQ(trace.frames[i].line.value(), line_numbers[j]); + EXPECT_THAT(trace.frames[i].symbol, testing::HasSubstr("stacktrace_from_current_z_2")); + i++; + j++; + ASSERT_LT(i, trace.frames.size()); + ASSERT_LT(j, line_numbers.size()); + EXPECT_THAT(trace.frames[i].filename, testing::EndsWith("from_current_z.cpp")); + EXPECT_EQ(trace.frames[i].line.value(), line_numbers[j]); + EXPECT_THAT(trace.frames[i].symbol, testing::HasSubstr("stacktrace_from_current_z_1")); + i++; + j++; + ASSERT_LT(i, trace.frames.size()); + ASSERT_LT(j, line_numbers.size()); + EXPECT_THAT(trace.frames[i].filename, testing::EndsWith("from_current_z.cpp")); + EXPECT_EQ(trace.frames[i].line.value(), line_numbers[j]); + EXPECT_THAT(trace.frames[i].symbol, testing::HasSubstr("FromCurrentZ_Basic_Test::TestBody")); + } +} + +TEST(FromCurrentZ, CorrectHandler) { + std::vector line_numbers; + CPPTRACE_TRYZ { + CPPTRACE_TRYZ { + line_numbers.insert(line_numbers.begin(), __LINE__ + 1); + stacktrace_from_current_z_1(line_numbers); + } CPPTRACE_CATCHZ(const std::logic_error&) { + FAIL(); + } + } CPPTRACE_CATCHZ(const std::exception& e) { + EXPECT_EQ(e.what(), "foobar"sv); + const auto& trace = cpptrace::from_current_exception(); + auto it = std::find_if( + trace.frames.begin(), + trace.frames.end(), + [](const cpptrace::stacktrace_frame& frame) { + return frame.filename.find("from_current_z.cpp") != std::string::npos + && frame.symbol.find("lambda") == std::string::npos; + } + ); + EXPECT_NE(it, trace.frames.end()); + it = std::find_if( + trace.frames.begin(), + trace.frames.end(), + [](const cpptrace::stacktrace_frame& frame) { + return frame.symbol.find("FromCurrentZ_CorrectHandler_Test::TestBody") != std::string::npos; + } + ); + EXPECT_NE(it, trace.frames.end()); + } +} + +TEST(FromCurrentZ, RawTrace) { + std::vector line_numbers; + CPPTRACE_TRYZ { + line_numbers.insert(line_numbers.begin(), __LINE__ + 1); + stacktrace_from_current_z_1(line_numbers); + } CPPTRACE_CATCHZ(const std::exception& e) { + EXPECT_EQ(e.what(), "foobar"sv); + const auto& raw_trace = cpptrace::raw_trace_from_current_exception(); + auto trace = raw_trace.resolve(); + auto it = std::find_if( + trace.frames.begin(), + trace.frames.end(), + [](const cpptrace::stacktrace_frame& frame) { + return frame.filename.find("from_current_z.cpp") != std::string::npos + && frame.symbol.find("lambda") == std::string::npos; + } + ); + EXPECT_NE(it, trace.frames.end()); + it = std::find_if( + trace.frames.begin(), + trace.frames.end(), + [](const cpptrace::stacktrace_frame& frame) { + return frame.symbol.find("FromCurrentZ_RawTrace_Test::TestBody") != std::string::npos; + } + ); + EXPECT_NE(it, trace.frames.end()); + } +}