libstdc++: Improve behaviour of std::stacktrace::current
authorJonathan Wakely <jwakely@redhat.com>
Mon, 11 Apr 2022 09:20:40 +0000 (10:20 +0100)
committerJonathan Wakely <jwakely@redhat.com>
Mon, 11 Apr 2022 16:59:08 +0000 (17:59 +0100)
This prevents inlining the current() function to guarantee that it is
present in the stacktrace, then tells libbacktrace to skip that frame.

To avoid overflow in the int argument to __glibcxx_backtrace_simple, we
need to check if the skip parameter exceeds INT_MAX (which is possible
for 16-bit targets where short and int have the same width). We also
need to limit the size of the returned value to the max_depth parameter,
which was missing previously.

This also fixes basic_stacktrace::max_size() to not exceed the maximum
size supported by the allocator, which might be smaller than the maximum
value of size_type.

libstdc++-v3/ChangeLog:

* include/std/stacktrace (basic_stacktrace::current): Duplicate
implementation into each overload. Add noinline attribute and
skip current frame.
(basic_stacktrace::max_size()): Call _Impl::_S_max_size.
(basic_stacktrace::_S_curr_cb()): New function defining lambda.
(basic_stacktrace::_Impl::_S_max_size): New function defining
maximum size in terms of allocator and size_type.
(basic_stacktrace::_Impl::_M_allocate): Check against
max_size().
* testsuite/19_diagnostics/stacktrace/entry.cc: Call function
for non-constexpr checks. Check line number is correct.

libstdc++-v3/include/std/stacktrace
libstdc++-v3/testsuite/19_diagnostics/stacktrace/entry.cc

index 623f44b..4e271ce 100644 (file)
@@ -36,6 +36,7 @@
 #include <bits/stl_algo.h>
 #include <bits/stl_iterator.h>
 #include <bits/stl_uninitialized.h>
+#include <ext/numeric_traits.h>
 #include <cxxabi.h>
 
 struct __glibcxx_backtrace_state;
@@ -232,19 +233,42 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       // [stacktrace.basic.ctor], creation and assignment
 
+      [[__gnu__::__noinline__]]
       static basic_stacktrace
       current(const allocator_type& __alloc = allocator_type()) noexcept
       {
-       return current(0, size_type(-1), __alloc);
+       auto __state = stacktrace_entry::_S_init();
+       basic_stacktrace __ret(__alloc);
+       if (!__ret._M_reserve(64)) [[unlikely]]
+         return __ret;
+
+       if (__glibcxx_backtrace_simple(__state, 1, _S_curr_cb(),
+                                      nullptr, std::__addressof(__ret)))
+         __ret._M_clear();
+
+       return __ret;
       }
 
+      [[__gnu__::__noinline__]]
       static basic_stacktrace
       current(size_type __skip,
              const allocator_type& __alloc = allocator_type()) noexcept
       {
-       return current(__skip, size_type(-1), __alloc);
+       auto __state = stacktrace_entry::_S_init();
+       basic_stacktrace __ret(__alloc);
+       if (__skip >= __INT_MAX__) [[unlikely]]
+         return __ret;
+       if (!__ret._M_reserve(64)) [[unlikely]]
+         return __ret;
+
+       if (__glibcxx_backtrace_simple(__state, __skip + 1, _S_curr_cb(),
+                                      nullptr, std::__addressof(__ret)))
+         __ret._M_clear();
+
+       return __ret;
       }
 
+      [[__gnu__::__noinline__]]
       static basic_stacktrace
       current(size_type __skip, size_type __max_depth,
              const allocator_type& __alloc = allocator_type()) noexcept
@@ -253,23 +277,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
        auto __state = stacktrace_entry::_S_init();
        basic_stacktrace __ret(__alloc);
-       if (!__ret._M_reserve(std::min<int>(__max_depth, 64)))
+       if (__max_depth == 0 || __skip >= __INT_MAX__) [[unlikely]]
+         return __ret;
+       if (!__ret._M_reserve(std::min<int>(__max_depth, 64))) [[unlikely]]
          return __ret;
 
-       auto __cb = [](void* __data, uintptr_t __pc) {
-         auto& __s = *static_cast<basic_stacktrace*>(__data);
-         stacktrace_entry __f;
-         __f._M_pc = __pc;
-         if (__s._M_push_back(__f))
-           return 0;
-         return 1;
-       };
+       if (__glibcxx_backtrace_simple(__state, __skip + 1, _S_curr_cb(),
+                                      nullptr, std::__addressof(__ret)))
+         __ret._M_clear();
+       else if (__ret.size() > __max_depth)
+         __ret.resize(__max_depth);
 
-       if (__glibcxx_backtrace_simple(__state, __skip, +__cb, nullptr,
-                                      std::__addressof(__ret)))
-         {
-           __ret._M_clear();
-         }
        return __ret;
       }
 
@@ -443,7 +461,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       [[nodiscard]] bool empty() const noexcept { return size() == 0; }
       size_type size() const noexcept { return _M_impl._M_size; }
-      size_type max_size() const noexcept { return size_type(-1); }
+
+      size_type
+      max_size() const noexcept
+      { return _Impl::_S_max_size(_M_impl._M_alloc); }
 
       const_reference
       operator[](size_type __n) const noexcept
@@ -507,6 +528,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        _M_impl._M_deallocate(_M_alloc);
       }
 
+      static auto
+      _S_curr_cb() noexcept
+      -> int (*) (void*, uintptr_t)
+      {
+       return [](void* __data, uintptr_t __pc) {
+         auto& __s = *static_cast<basic_stacktrace*>(__data);
+         stacktrace_entry __f;
+         __f._M_pc = __pc;
+         if (__s._M_push_back(__f))
+           return 0;
+         return 1;
+       };
+      }
+
       struct _Impl
       {
        using pointer = typename _AllocTraits::pointer;
@@ -515,21 +550,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        size_type _M_size     = 0;
        size_type _M_capacity = 0;
 
+       static size_type
+       _S_max_size(const allocator_type& __alloc) noexcept
+       {
+         const size_t __size_max = __gnu_cxx::__int_traits<size_type>::__max;
+         const size_t __alloc_max = _AllocTraits::max_size(__alloc);
+         return std::min(__size_max, __alloc_max);
+       }
+
        // Precondition: _M_frames == nullptr
        pointer
        _M_allocate(allocator_type& __alloc, size_type __n) noexcept
        {
          __try
            {
-             _M_frames = __n ? __alloc.allocate(__n) : nullptr;
-             _M_capacity = __n;
+             if (0 < __n && __n <= _S_max_size(__alloc)) [[unlikely]]
+               {
+                 _M_frames = __alloc.allocate(__n);
+                 _M_capacity = __n;
+                 return _M_frames;
+               }
            }
          __catch (...)
            {
-             _M_frames = nullptr;
-             _M_capacity = 0;
            }
-         return _M_frames;
+         _M_frames = nullptr;
+         _M_capacity = 0;
+         return nullptr;;
        }
 
        void
index 0bbcabd..a222c42 100644 (file)
@@ -36,7 +36,8 @@ test_members()
   VERIFY( e1 != e2 );
   VERIFY( e1.description() == e2.description() );
   VERIFY( e1.source_file() == e2.source_file() );
-  VERIFY( e1.source_line() != e2.source_line() );
+  VERIFY( e1.source_line() == (__LINE__ - 5) );
+  VERIFY( e2.source_line() == (__LINE__ - 5) );
 
   std::stacktrace_entry e3 = []{
     return std::stacktrace::current().at(0);
@@ -44,10 +45,10 @@ test_members()
   VERIFY( e1 != e3 );
   VERIFY( e1.description() != e3.description() );
   VERIFY( e1.source_file() == e3.source_file() );
-  VERIFY( e1.source_line() != e3.source_line() );
+  VERIFY( e3.source_line() == (__LINE__ - 5) );
 }
 
 int main()
 {
-  test_constexpr();
+  test_members();
 }