* [committed] libstdc++: Improve behaviour of std::stacktrace::current
@ 2022-04-11 17:01 Jonathan Wakely
2022-04-11 21:27 ` Jonathan Wakely
0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Wakely @ 2022-04-11 17:01 UTC (permalink / raw)
To: libstdc++, gcc-patches
Tested x86_64-linux, pushed to trunk.
-- >8 --
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 | 91 ++++++++++++++-----
.../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();
}
--
2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [committed] libstdc++: Improve behaviour of std::stacktrace::current
2022-04-11 17:01 [committed] libstdc++: Improve behaviour of std::stacktrace::current Jonathan Wakely
@ 2022-04-11 21:27 ` Jonathan Wakely
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2022-04-11 21:27 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc Patches
[-- Attachment #1: Type: text/plain, Size: 822 bytes --]
On Mon, 11 Apr 2022 at 18:03, Jonathan Wakely via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
> // 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]]
I originally changed this to return early if the size isn't OK:
if (unlikely condition)
return nullptr;
but in the version I pushed it's:
if (likely condition)
// do allocation
but forgot to change the attribute to match.
Fixed by the attached. Tested x86_64-linux and pushed to trunk.
I have further improvements to stacktrace::current coming tomorrow.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 852 bytes --]
commit b1124648ff8f655307f264d7b353fd68e3b03e9c
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Apr 11 20:13:44 2022
libstdc++: Fix incorrect branch prediction hint in std::stacktrace
libstdc++-v3/ChangeLog:
* include/std/stacktrace (basic_stacktrace::_Impl::_M_allocate):
Change [[unlikely]] attribute to [[likely]].
diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index dd78c71c5dc..79038e803f2 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -579,7 +579,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
__try
{
- if (0 < __n && __n <= _S_max_size(__alloc)) [[unlikely]]
+ if (0 < __n && __n <= _S_max_size(__alloc)) [[likely]]
{
_M_frames = __alloc.allocate(__n);
_M_capacity = __n;
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-04-11 21:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 17:01 [committed] libstdc++: Improve behaviour of std::stacktrace::current Jonathan Wakely
2022-04-11 21:27 ` 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).