From e798527df818fe495341c54642f224d0359ea505 Mon Sep 17 00:00:00 2001 From: Antony Polukhin Date: Thu, 8 Sep 2016 21:23:43 +0300 Subject: [PATCH] Fixed libunwind exception safety in assignments and construction, changed tests to use boost's lightweight test infrastructure, code cleanup, include guards added --- build/Jamfile.v2 | 3 -- include/boost/stacktrace.hpp | 20 +++++++- .../boost/stacktrace/detail/stacktrace.ipp | 41 ++++++++++------- .../stacktrace/detail/stacktrace_helpers.hpp | 6 ++- .../detail/stacktrace_libunwind.hpp | 46 ++++++++++++++++--- .../stacktrace/detail/stacktrace_linux.hpp | 7 ++- .../stacktrace/detail/stacktrace_windows.hpp | 23 ++++++---- src/backtrace.cpp | 2 +- src/libunwind.cpp | 2 +- src/windbg.cpp | 2 +- test/test.cpp | 24 +++++++++- 11 files changed, 132 insertions(+), 44 deletions(-) diff --git a/build/Jamfile.v2 b/build/Jamfile.v2 index 1aa3d19..08a2745 100644 --- a/build/Jamfile.v2 +++ b/build/Jamfile.v2 @@ -38,7 +38,6 @@ lib boost_stacktrace_libunwind ../src/libunwind.cpp : # requirements all - BOOST_STACKTRACE_USE_LIBUNWIND=1 unwind [ check-target-builds ../build//libunwind : : no ] : # default build @@ -53,7 +52,6 @@ lib boost_stacktrace_backtrace ../src/backtrace.cpp : # requirements all - BOOST_STACKTRACE_USE_BACKTRACE=1 dl [ check-target-builds ../build//backtrace : : no ] : # default build @@ -69,7 +67,6 @@ lib boost_stacktrace_windbg : # requirements all Dbghelp - BOOST_STACKTRACE_USE_WINDBG=1 [ check-target-builds ../build//WinDbg : : no ] : # default build : # usage-requirements diff --git a/include/boost/stacktrace.hpp b/include/boost/stacktrace.hpp index b5e8597..3271587 100644 --- a/include/boost/stacktrace.hpp +++ b/include/boost/stacktrace.hpp @@ -4,6 +4,8 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) +#ifndef BOOST_STACKTRACE_STACKTRACE_HPP +#define BOOST_STACKTRACE_STACKTRACE_HPP #include #ifdef BOOST_HAS_PRAGMA_ONCE @@ -27,22 +29,34 @@ namespace boost { namespace stacktrace { class stacktrace { BOOST_STATIC_CONSTEXPR std::size_t max_implementation_size = sizeof(void*) * 110u; boost::aligned_storage::type impl_; + public: - /// Stores the current function call sequence inside the class + /// @brief Stores the current function call sequence inside the class. + /// + /// @b Complexity: O(N) where N is call seaquence length BOOST_STACKTRACE_FUNCTION stacktrace() BOOST_NOEXCEPT; + /// @b Complexity: O(1) BOOST_STACKTRACE_FUNCTION stacktrace(const stacktrace& bt) BOOST_NOEXCEPT; + + /// @b Complexity: O(1) BOOST_STACKTRACE_FUNCTION stacktrace& operator=(const stacktrace& bt) BOOST_NOEXCEPT; + + /// @b Complexity: O(N) for libunwind, O(1) for other backends. BOOST_STACKTRACE_FUNCTION ~stacktrace() BOOST_NOEXCEPT; /// @returns Number of function names stored inside the class. + /// + /// @b Complexity: O(1) BOOST_STACKTRACE_FUNCTION std::size_t size() const BOOST_NOEXCEPT; /// @param frame Zero based index of function to return. 0 /// is the function index where stacktrace was constructed and /// index close to this->size() contains function `main()`. /// @returns Function name in a human readable form. - /// @throws std::bad_alloc if not enough memory. + /// @throws std::bad_alloc if not enough memory to construct resulting string. + /// + /// @b Complexity: Amortized O(1) BOOST_STACKTRACE_FUNCTION std::string operator[](std::size_t frame) const; }; @@ -78,3 +92,5 @@ std::basic_ostream& operator<<(std::basic_ostream #endif /// @endcond + +#endif // BOOST_STACKTRACE_STACKTRACE_HPP diff --git a/include/boost/stacktrace/detail/stacktrace.ipp b/include/boost/stacktrace/detail/stacktrace.ipp index ba5bd76..8332e57 100644 --- a/include/boost/stacktrace/detail/stacktrace.ipp +++ b/include/boost/stacktrace/detail/stacktrace.ipp @@ -4,15 +4,18 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) +#ifndef BOOST_STACKTRACE_DETAIL_STACKTRACE_IPP +#define BOOST_STACKTRACE_DETAIL_STACKTRACE_IPP + #include #ifdef BOOST_HAS_PRAGMA_ONCE # pragma once #endif +#include #include -#include -#if BOOST_OS_WINDOWS +#if defined(BOOST_WINDOWS) || defined(BOOST_STACKTRACE_USE_WINDBG) # include #elif defined(BOOST_STACKTRACE_USE_LIBUNWIND) # include @@ -30,15 +33,17 @@ namespace boost { namespace stacktrace { -template -inline boost::stacktrace::detail::backtrace_holder& to_bt(T& data) BOOST_NOEXCEPT { - return *reinterpret_cast(&data); -} +namespace detail { + template + inline boost::stacktrace::detail::backtrace_holder& to_bt(T& data) BOOST_NOEXCEPT { + return *reinterpret_cast(&data); + } -template -inline const boost::stacktrace::detail::backtrace_holder& to_bt(const T& data) BOOST_NOEXCEPT { - return *reinterpret_cast(&data); -} + template + inline const boost::stacktrace::detail::backtrace_holder& to_bt(const T& data) BOOST_NOEXCEPT { + return *reinterpret_cast(&data); + } +} // namespace detail stacktrace::stacktrace() BOOST_NOEXCEPT { @@ -46,26 +51,30 @@ stacktrace::stacktrace() BOOST_NOEXCEPT { } stacktrace::stacktrace(const stacktrace& bt) BOOST_NOEXCEPT { - new (&impl_) boost::stacktrace::detail::backtrace_holder(to_bt(bt.impl_)); + new (&impl_) boost::stacktrace::detail::backtrace_holder( + boost::stacktrace::detail::to_bt(bt.impl_) + ); } stacktrace& stacktrace::operator=(const stacktrace& bt) BOOST_NOEXCEPT { - to_bt(impl_) = to_bt(bt.impl_); + boost::stacktrace::detail::to_bt(impl_) = boost::stacktrace::detail::to_bt(bt.impl_); return *this; } stacktrace::~stacktrace() BOOST_NOEXCEPT { BOOST_STATIC_ASSERT_MSG(sizeof(impl_) >= sizeof(boost::stacktrace::detail::backtrace_holder), "Too small storage for holding backtrace"); - to_bt(impl_).~backtrace_holder(); + boost::stacktrace::detail::to_bt(impl_).~backtrace_holder(); } std::size_t stacktrace::size() const BOOST_NOEXCEPT { - return to_bt(impl_).size(); + return boost::stacktrace::detail::to_bt(impl_).size(); } -std::string stacktrace::operator[](std::size_t frame) const BOOST_NOEXCEPT { - return to_bt(impl_).get_frame(frame); +std::string stacktrace::operator[](std::size_t frame) const { + return boost::stacktrace::detail::to_bt(impl_).get_frame(frame); } }} + +#endif // BOOST_STACKTRACE_DETAIL_STACKTRACE_IPP diff --git a/include/boost/stacktrace/detail/stacktrace_helpers.hpp b/include/boost/stacktrace/detail/stacktrace_helpers.hpp index 29bf3ce..bd190ea 100644 --- a/include/boost/stacktrace/detail/stacktrace_helpers.hpp +++ b/include/boost/stacktrace/detail/stacktrace_helpers.hpp @@ -4,12 +4,14 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) +#ifndef BOOST_STACKTRACE_DETAIL_STACKTRACE_HELPERS_HPP +#define BOOST_STACKTRACE_DETAIL_STACKTRACE_HELPERS_HPP + #include #ifdef BOOST_HAS_PRAGMA_ONCE # pragma once #endif -#include #include #include @@ -42,3 +44,5 @@ inline boost::array to_hex(T addr) BOOST_NOEXCE }*/ }}} // namespace boost::stacktrace::detail + +#endif // BOOST_STACKTRACE_DETAIL_STACKTRACE_HELPERS_HPP diff --git a/include/boost/stacktrace/detail/stacktrace_libunwind.hpp b/include/boost/stacktrace/detail/stacktrace_libunwind.hpp index 9adc80b..547c4c1 100644 --- a/include/boost/stacktrace/detail/stacktrace_libunwind.hpp +++ b/include/boost/stacktrace/detail/stacktrace_libunwind.hpp @@ -4,6 +4,9 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) +#ifndef BOOST_STACKTRACE_DETAIL_STACKTRACE_LIBUNWIND_HPP +#define BOOST_STACKTRACE_DETAIL_STACKTRACE_LIBUNWIND_HPP + #include #ifdef BOOST_HAS_PRAGMA_ONCE # pragma once @@ -13,7 +16,7 @@ #include #include #include -#include +#include #define UNW_LOCAL_ONLY #include @@ -22,45 +25,72 @@ namespace boost { namespace stacktrace { namespace detail { struct backtrace_holder { BOOST_STATIC_CONSTEXPR std::size_t skip_frames = 1u; - std::vector frames; + std::size_t frames_count; + boost::shared_ptr frames; - backtrace_holder() BOOST_NOEXCEPT { + backtrace_holder() BOOST_NOEXCEPT + : frames_count(0) + { unw_context_t uc; if (unw_getcontext(&uc) != 0) { return; } + { // Counting frames_count + unw_cursor_t cursor; + if (unw_init_local(&cursor, &uc) != 0) { + return; + } + while (unw_step(&cursor) > 0) { + ++ frames_count; + } + if (frames_count <= skip_frames) { + frames_count = 0; + return; + } + frames_count -= skip_frames; + } + unw_cursor_t cursor; if (unw_init_local(&cursor, &uc) != 0) { + frames_count = 0; return; } for (std::size_t i = 0; i < skip_frames; ++i) { if (unw_step(&cursor) <= 0) { + frames_count = 0; return; } } BOOST_TRY { + frames = boost::make_shared(frames_count); + std::size_t i = 0; while (unw_step(&cursor) > 0){ - frames.push_back(get_frame_impl(cursor)); + frames[i] = get_frame_impl(cursor); + ++ i; } } BOOST_CATCH(...) {} BOOST_CATCH_END } std::size_t size() const BOOST_NOEXCEPT { - return frames.size(); + return frames_count; } std::string get_frame(std::size_t frame) const { - return frames[frame]; + if (frame < frames_count) { + return frames[frame]; + } else { + return std::string(); + } } static std::string get_frame_impl(unw_cursor_t& cursor) { std::string res; unw_word_t offp; - char data[128]; + char data[256]; const int ret = unw_get_proc_name (&cursor, data, sizeof(data) / sizeof(char), &offp); if (ret == -UNW_ENOMEM) { @@ -94,3 +124,5 @@ struct backtrace_holder { }; }}} // namespace boost::stacktrace::detail + +#endif // BOOST_STACKTRACE_DETAIL_STACKTRACE_LIBUNWIND_HPP diff --git a/include/boost/stacktrace/detail/stacktrace_linux.hpp b/include/boost/stacktrace/detail/stacktrace_linux.hpp index 75e1d50..0a048d9 100644 --- a/include/boost/stacktrace/detail/stacktrace_linux.hpp +++ b/include/boost/stacktrace/detail/stacktrace_linux.hpp @@ -4,6 +4,9 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) +#ifndef BOOST_STACKTRACE_DETAIL_STACKTRACE_LINUX_HPP +#define BOOST_STACKTRACE_DETAIL_STACKTRACE_LINUX_HPP + #include #ifdef BOOST_HAS_PRAGMA_ONCE # pragma once @@ -26,7 +29,7 @@ struct backtrace_holder { #ifdef BOOST_STACKTRACE_HEADER_ONLY BOOST_STATIC_CONSTEXPR std::size_t skip_frames = 0u; #else - BOOST_STATIC_CONSTEXPR std::size_t skip_frames = 1u; + BOOST_STATIC_CONSTEXPR std::size_t skip_frames = 2u; #endif inline backtrace_holder() BOOST_NOEXCEPT { @@ -64,3 +67,5 @@ struct backtrace_holder { }}} // namespace boost::stacktrace::detail + +#endif // BOOST_STACKTRACE_DETAIL_STACKTRACE_LINUX_HPP diff --git a/include/boost/stacktrace/detail/stacktrace_windows.hpp b/include/boost/stacktrace/detail/stacktrace_windows.hpp index 3961b39..0cde56d 100644 --- a/include/boost/stacktrace/detail/stacktrace_windows.hpp +++ b/include/boost/stacktrace/detail/stacktrace_windows.hpp @@ -4,12 +4,17 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) +#ifndef BOOST_STACKTRACE_DETAIL_STACKTRACE_WINDOWS_HPP +#define BOOST_STACKTRACE_DETAIL_STACKTRACE_WINDOWS_HPP + +#include +#ifdef BOOST_HAS_PRAGMA_ONCE +# pragma once +#endif -#include #include #include -#include #include #include "DbgHelp.h" #include @@ -17,8 +22,9 @@ namespace boost { namespace stacktrace { namespace detail { struct symbol_info_with_stack { + BOOST_STATIC_CONSTEXPR std::size_t max_name_length = MAX_SYM_NAME * sizeof(char); SYMBOL_INFO symbol; - char name_part[MAX_SYM_NAME * sizeof(TCHAR)]; + char name_part[max_name_length]; }; struct backtrace_holder { @@ -44,7 +50,7 @@ struct backtrace_holder { return frames_count; } - inline std::string get_frame(std::size_t frame) const BOOST_NOEXCEPT { + inline std::string get_frame(std::size_t frame) const { std::string res; if (frame >= frames_count || !process) { @@ -52,14 +58,10 @@ struct backtrace_holder { } symbol_info_with_stack s; - s.symbol.MaxNameLen = f.name.size() - 1; + s.symbol.MaxNameLen = symbol_info_with_stack::max_name_length; s.symbol.SizeOfStruct = sizeof(SYMBOL_INFO); - SymFromAddr(process, reinterpret_cast(buffer[frame]), 0, &s.symbol); - BOOST_TRY { - res = s.symbol.Name; - } BOOST_CATCH(...) {} - BOOST_CATCH_END + res = s.symbol.Name; return res; } @@ -67,3 +69,4 @@ struct backtrace_holder { }}} // namespace boost::stacktrace::detail +#endif // BOOST_STACKTRACE_DETAIL_STACKTRACE_WINDOWS_HPP diff --git a/src/backtrace.cpp b/src/backtrace.cpp index b44ae7e..608c295 100644 --- a/src/backtrace.cpp +++ b/src/backtrace.cpp @@ -4,5 +4,5 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) - +#define BOOST_STACKTRACE_USE_BACKTRACE #include diff --git a/src/libunwind.cpp b/src/libunwind.cpp index b44ae7e..dbf68cb 100644 --- a/src/libunwind.cpp +++ b/src/libunwind.cpp @@ -4,5 +4,5 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) - +#define BOOST_STACKTRACE_USE_LIBUNWIND #include diff --git a/src/windbg.cpp b/src/windbg.cpp index b44ae7e..aaf02f4 100644 --- a/src/windbg.cpp +++ b/src/windbg.cpp @@ -4,5 +4,5 @@ // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) - +#define BOOST_STACKTRACE_USE_WINDBG #include diff --git a/test/test.cpp b/test/test.cpp index d224e39..db9bdf1 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -35,10 +35,32 @@ void test_nested() { assert(ss1.str().find("main") != std::string::npos); assert(ss2.str().find("main") != std::string::npos); + + + assert(ss2.str().find("stacktrace") == std::string::npos); + +#if !defined(BOOST_STACKTRACE_HEADER_ONLY) || !defined(BOOST_STACKTRACE_USE_BACKTRACE) + assert(ss1.str().find("stacktrace") != std::string::npos); + assert(ss1.str().find("pair") != std::string::npos); + + assert(ss1.str().find("foo1") != std::string::npos); + assert(ss1.str().find("foo2") != std::string::npos); + assert(ss2.str().find("foo1") != std::string::npos); + assert(ss2.str().find("foo2") != std::string::npos); +#endif } int main() { - std::cout << return_from_nested_namespaces(); + std::stringstream ss; + ss << return_from_nested_namespaces(); + std::cout << ss.str() << '\n'; + assert(ss.str().find("main") != std::string::npos); + assert(ss.str().find("stacktrace") == std::string::npos); + +#if !defined(BOOST_STACKTRACE_HEADER_ONLY) || !defined(BOOST_STACKTRACE_USE_BACKTRACE) + assert(ss.str().find("get_backtrace_from_nested_namespaces") != std::string::npos); +#endif + test_nested(); }