public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-8088] libstdc++: Improve behaviour of std::stacktrace::current
@ 2022-04-11 17:01 Jonathan Wakely
  0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2022-04-11 17:01 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:bdb9639ee99b68a8c7541e78057e96fe6a2c62ed

commit r12-8088-gbdb9639ee99b68a8c7541e78057e96fe6a2c62ed
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Apr 11 10:20:40 2022 +0100

    libstdc++: Improve behaviour of std::stacktrace::current
    
    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.

Diff:
---
 libstdc++-v3/include/std/stacktrace                | 91 ++++++++++++++++------
 .../testsuite/19_diagnostics/stacktrace/entry.cc   |  7 +-
 2 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index 623f44bdca4..4e271cef3f3 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -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
diff --git a/libstdc++-v3/testsuite/19_diagnostics/stacktrace/entry.cc b/libstdc++-v3/testsuite/19_diagnostics/stacktrace/entry.cc
index 0bbcabd8fae..a222c425b20 100644
--- a/libstdc++-v3/testsuite/19_diagnostics/stacktrace/entry.cc
+++ b/libstdc++-v3/testsuite/19_diagnostics/stacktrace/entry.cc
@@ -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();
 }


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-04-11 17:01 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 17:01 [gcc r12-8088] libstdc++: Improve behaviour of std::stacktrace::current Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).