From eb9fcf8050c1cfec7582709d5035bc3595597b8e Mon Sep 17 00:00:00 2001 From: Antony Polukhin Date: Tue, 7 Feb 2017 22:55:19 +0300 Subject: [PATCH] Removed internals from stacktraces, added more tests on safe dumping --- example/terminate_handler.cpp | 46 ++++++++++++++++++- .../boost/stacktrace/detail/frame_msvc.ipp | 4 +- .../boost/stacktrace/detail/frame_noop.ipp | 6 +-- .../boost/stacktrace/detail/frame_unwind.ipp | 10 +++- include/boost/stacktrace/frame.hpp | 2 +- include/boost/stacktrace/safe_dump_to.hpp | 43 ++++++++++------- include/boost/stacktrace/stacktrace.hpp | 23 +++++----- 7 files changed, 94 insertions(+), 40 deletions(-) diff --git a/example/terminate_handler.cpp b/example/terminate_handler.cpp index 508bb42..80f331e 100644 --- a/example/terminate_handler.cpp +++ b/example/terminate_handler.cpp @@ -192,6 +192,49 @@ int run_4(const char* argv[]) { return 0; } +#include + +int test_inplace() { + { + boost::stacktrace::safe_dump_to("./backtrace2.dump"); + boost::stacktrace::stacktrace ss2; + std::ifstream ifs("./backtrace2.dump"); + boost::stacktrace::stacktrace ss1 = boost::stacktrace::stacktrace::from_dump(ifs); + ifs.close(); + boost::filesystem::remove("./backtrace2.dump"); + + if (ss1.size() != ss2.size()) { + std::cerr << "Stacktraces differ:\n" << ss1 << "\n vs \n" << ss2 << '\n'; + return 51; + } + + if (ss1 && ss1[0].name() != ss2[0].name()) { + std::cerr << "Stacktraces differ:\n" << ss1 << "\n vs \n" << ss2 << '\n'; + return 52; + } + } + + { + void* data[1024]; + boost::stacktrace::safe_dump_to(data, sizeof(data)); + boost::stacktrace::stacktrace ss2; + boost::stacktrace::stacktrace ss1 = boost::stacktrace::stacktrace::from_dump(data, sizeof(data)); + + if (ss1.size() != ss2.size()) { + std::cerr << "Stacktraces differ:\n" << ss1 << "\n vs \n" << ss2 << '\n'; + return 53; + } + + if (ss1 && ss1[0].name() != ss2[0].name()) { + std::cerr << "Stacktraces differ:\n" << ss1 << "\n vs \n" << ss2 << '\n'; + return 54; + } + } + + return 0; +} + + int main(int argc, const char* argv[]) { if (argc < 2) { // We are copying files to make sure that stacktrace printing works independently from executable name @@ -199,7 +242,8 @@ int main(int argc, const char* argv[]) { copy_and_run(argv[0], '2', false); copy_and_run(argv[0], '3', true); copy_and_run(argv[0], '4', false); - return 0; + + return test_inplace(); } switch (argv[1][0]) { diff --git a/include/boost/stacktrace/detail/frame_msvc.ipp b/include/boost/stacktrace/detail/frame_msvc.ipp index ff2d24f..31bbbb4 100644 --- a/include/boost/stacktrace/detail/frame_msvc.ipp +++ b/include/boost/stacktrace/detail/frame_msvc.ipp @@ -33,9 +33,9 @@ namespace boost { namespace stacktrace { namespace detail { -std::size_t this_thread_frames::collect(void** memory, std::size_t size) BOOST_NOEXCEPT { +std::size_t this_thread_frames::collect(void** memory, std::size_t size, std::size_t skip) BOOST_NOEXCEPT { return ::CaptureStackBackTrace( - 0, + skip + 1, static_cast(size), memory, 0 diff --git a/include/boost/stacktrace/detail/frame_noop.ipp b/include/boost/stacktrace/detail/frame_noop.ipp index c35d304..0d4f4e6 100644 --- a/include/boost/stacktrace/detail/frame_noop.ipp +++ b/include/boost/stacktrace/detail/frame_noop.ipp @@ -20,11 +20,7 @@ std::string to_string(const frame* /*frames*/, std::size_t /*count*/) { return std::string(); } -std::size_t from_dump(const char* /*filename*/, void** /*frames*/) { - return 0; -} - -std::size_t this_thread_frames::collect(void** /*memory*/, std::size_t /*size*/) BOOST_NOEXCEPT { +std::size_t this_thread_frames::collect(void** /*memory*/, std::size_t /*size*/, std::size_t /*skip*/) BOOST_NOEXCEPT { return 0; } diff --git a/include/boost/stacktrace/detail/frame_unwind.ipp b/include/boost/stacktrace/detail/frame_unwind.ipp index ba7f4c1..7777b20 100644 --- a/include/boost/stacktrace/detail/frame_unwind.ipp +++ b/include/boost/stacktrace/detail/frame_unwind.ipp @@ -41,12 +41,18 @@ namespace boost { namespace stacktrace { namespace detail { struct unwind_state { + std::size_t frames_to_skip; void** current; void** end; }; inline _Unwind_Reason_Code unwind_callback(::_Unwind_Context* context, void* arg) { unwind_state* state = static_cast(arg); + if (state->frames_to_skip) { + --state->frames_to_skip; + return ::_URC_NO_REASON; + } + *state->current = reinterpret_cast( ::_Unwind_GetIP(context) ); @@ -58,13 +64,13 @@ inline _Unwind_Reason_Code unwind_callback(::_Unwind_Context* context, void* arg return ::_URC_NO_REASON; } -std::size_t this_thread_frames::collect(void** memory, std::size_t size) BOOST_NOEXCEPT { +std::size_t this_thread_frames::collect(void** memory, std::size_t size, std::size_t skip) BOOST_NOEXCEPT { std::size_t frames_count = 0; if (!size) { return frames_count; } - boost::stacktrace::detail::unwind_state state = { memory, memory + size }; + boost::stacktrace::detail::unwind_state state = { skip + 1, memory, memory + size }; ::_Unwind_Backtrace(&boost::stacktrace::detail::unwind_callback, &state); frames_count = state.current - memory; diff --git a/include/boost/stacktrace/frame.hpp b/include/boost/stacktrace/frame.hpp index 0a491d9..f49d219 100644 --- a/include/boost/stacktrace/frame.hpp +++ b/include/boost/stacktrace/frame.hpp @@ -60,7 +60,7 @@ namespace detail { struct this_thread_frames { // struct is required to avoid warning about usage of inline+BOOST_NOINLINE - BOOST_NOINLINE BOOST_STACKTRACE_FUNCTION static std::size_t collect(void** memory, std::size_t size) BOOST_NOEXCEPT; + BOOST_NOINLINE BOOST_STACKTRACE_FUNCTION static std::size_t collect(void** memory, std::size_t size, std::size_t skip) BOOST_NOEXCEPT; }; } // namespace detail diff --git a/include/boost/stacktrace/safe_dump_to.hpp b/include/boost/stacktrace/safe_dump_to.hpp index 057c0d8..59add2b 100644 --- a/include/boost/stacktrace/safe_dump_to.hpp +++ b/include/boost/stacktrace/safe_dump_to.hpp @@ -20,6 +20,28 @@ namespace boost { namespace stacktrace { + +/// @cond +namespace detail { + struct suppress_noinline_warnings { + BOOST_NOINLINE static std::size_t safe_dump_to_impl(void* memory, std::size_t size) BOOST_NOEXCEPT { + void** mem = static_cast(memory); + const std::size_t frames_count = boost::stacktrace::detail::this_thread_frames::collect(mem, size / sizeof(void*) - 1, 1); + mem[frames_count] = 0; + return frames_count + 1; + } + + template + BOOST_NOINLINE static std::size_t safe_dump_to_impl(T file) BOOST_NOEXCEPT { + void* buffer[boost::stacktrace::detail::max_frames_dump + 1]; + const std::size_t frames_count = boost::stacktrace::detail::this_thread_frames::collect(buffer, boost::stacktrace::detail::max_frames_dump, 1); + buffer[frames_count] = 0; + return boost::stacktrace::detail::dump(file, buffer, frames_count + 1); + } + }; +} +/// @endcond + /// @brief Stores current function call sequence into the memory. /// /// @b Complexity: O(N) where N is call sequence length, O(1) if BOOST_STACKTRACE_USE_NOOP is defined. @@ -32,22 +54,9 @@ namespace boost { namespace stacktrace { /// /// @param size Size of the preallocated buffer. BOOST_FORCEINLINE std::size_t safe_dump_to(void* memory, std::size_t size) BOOST_NOEXCEPT { - void** mem = static_cast(memory); - return boost::stacktrace::detail::this_thread_frames::collect(mem, size / sizeof(void*)); + return boost::stacktrace::detail::suppress_noinline_warnings::safe_dump_to_impl(memory, size); } -/// @cond -namespace detail { - template - BOOST_FORCEINLINE std::size_t safe_dump_to_impl(T file) BOOST_NOEXCEPT { - void* buffer[boost::stacktrace::detail::max_frames_dump + 1]; - const std::size_t frames_count = boost::stacktrace::detail::this_thread_frames::collect(buffer, boost::stacktrace::detail::max_frames_dump); - buffer[frames_count] = 0; - return boost::stacktrace::detail::dump(file, buffer, frames_count + 1); - } -} -/// @endcond - /// @brief Opens a file and rewrites its content with current function call sequence. /// /// @b Complexity: O(N) where N is call sequence length, O(1) if BOOST_STACKTRACE_USE_NOOP is defined. @@ -58,7 +67,7 @@ namespace detail { /// /// @param file File to store current function call sequence. BOOST_FORCEINLINE std::size_t safe_dump_to(const char* file) BOOST_NOEXCEPT { - return boost::stacktrace::detail::safe_dump_to_impl(file); + return boost::stacktrace::detail::suppress_noinline_warnings::safe_dump_to_impl(file); } #ifdef BOOST_STACKTRACE_DOXYGEN_INVOKED @@ -77,14 +86,14 @@ BOOST_FORCEINLINE std::size_t safe_dump_to(platform_specific_descriptor fd) BOOS #elif defined(BOOST_WINDOWS) BOOST_FORCEINLINE std::size_t safe_dump_to(void* fd) BOOST_NOEXCEPT { - return boost::stacktrace::detail::safe_dump_to_impl(fd); + return boost::stacktrace::detail::suppress_noinline_warnings::safe_dump_to_impl(fd); } #else // POSIX BOOST_FORCEINLINE std::size_t safe_dump_to(int fd) BOOST_NOEXCEPT { - return boost::stacktrace::detail::safe_dump_to_impl(fd); + return boost::stacktrace::detail::suppress_noinline_warnings::safe_dump_to_impl(fd); } #endif diff --git a/include/boost/stacktrace/stacktrace.hpp b/include/boost/stacktrace/stacktrace.hpp index 7d75c1e..eb2de1b 100644 --- a/include/boost/stacktrace/stacktrace.hpp +++ b/include/boost/stacktrace/stacktrace.hpp @@ -37,15 +37,15 @@ class basic_stacktrace { boost::container::vector impl_; /// @cond - static const std::size_t frames_to_skip = 2; + static const std::size_t frames_to_skip = 1; void fill(void** begin, std::size_t size) { - if (size < frames_to_skip) { + if (!size) { return; } - impl_.reserve(static_cast(size - frames_to_skip)); - for (std::size_t i = frames_to_skip; i < size; ++i) { + impl_.reserve(static_cast(size)); + for (std::size_t i = 0; i < size; ++i) { impl_.push_back( frame(begin[i]) ); @@ -72,6 +72,9 @@ public: typedef typename boost::container::vector::const_reverse_iterator reverse_iterator; typedef typename boost::container::vector::const_reverse_iterator const_reverse_iterator; + // @param skip How many latest calls to skip and do not store in *this. + + /// @brief Stores the current function call sequence inside the class. /// /// @b Complexity: O(N) where N is call sequence length, O(1) if BOOST_STACKTRACE_USE_NOOP is defined. @@ -85,19 +88,15 @@ public: BOOST_NOINLINE explicit basic_stacktrace(std::size_t max_depth = static_cast(-1), const allocator_type& a = allocator_type()) BOOST_NOEXCEPT : impl_(a) { - BOOST_CONSTEXPR_OR_CONST size_t buffer_size = 128; + BOOST_CONSTEXPR_OR_CONST std::size_t buffer_size = 128; if (!max_depth) { return; } - if (static_cast(-1) - frames_to_skip >= max_depth) { - max_depth += frames_to_skip; - } - try { { // Fast path without additional allocations void* buffer[buffer_size]; - const std::size_t frames_count = boost::stacktrace::detail::this_thread_frames::collect(buffer, buffer_size); + const std::size_t frames_count = boost::stacktrace::detail::this_thread_frames::collect(buffer, buffer_size, frames_to_skip); if (buffer_size > frames_count || frames_count >= max_depth) { const std::size_t size = (max_depth < frames_count ? max_depth : frames_count); fill(buffer, size); @@ -109,7 +108,7 @@ public: typedef typename Allocator::template rebind::other allocator_void_t; boost::container::vector buf(buffer_size * 2, 0, impl_.get_allocator()); do { - const std::size_t frames_count = boost::stacktrace::detail::this_thread_frames::collect(buf.data(), buf.size()); + const std::size_t frames_count = boost::stacktrace::detail::this_thread_frames::collect(buf.data(), buf.size(), frames_to_skip); if (buf.size() > frames_count || frames_count >= max_depth) { const std::size_t size = (max_depth < frames_count ? max_depth : frames_count); fill(buf.data(), size); @@ -274,7 +273,7 @@ public: static basic_stacktrace from_dump(const void* begin, std::size_t size, const allocator_type& a = allocator_type()) { basic_stacktrace ret(0, a); const void* const* first = static_cast(begin); - const std::size_t frames_count = frames_count_from_buffer_size(size); + const std::size_t frames_count = frames_count_from_buffer_size(size); if (!frames_count) { return ret; }