public inbox for libstdc++-cvs@sourceware.org help / color / mirror / Atom feed
From: Jonathan Wakely <redi@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r12-8089] libstdc++: Fix std::basic_stacktrace special members [PR105031] Date: Mon, 11 Apr 2022 17:01:11 +0000 (GMT) [thread overview] Message-ID: <20220411170111.67E83385CFE9@sourceware.org> (raw) https://gcc.gnu.org/g:73da9ec55609efa532b9ac85ac2dbe121c134e18 commit r12-8089-g73da9ec55609efa532b9ac85ac2dbe121c134e18 Author: Jonathan Wakely <jwakely@redhat.com> Date: Mon Apr 11 17:44:14 2022 +0100 libstdc++: Fix std::basic_stacktrace special members [PR105031] The PR points out that there is a non-constant condition used for an if-constexpr statement, but there are several other problems with the copy, move and swap members of std::basic_stacktrace. libstdc++-v3/ChangeLog: PR libstdc++/105031 * include/std/stacktrace (basic_stacktrace::basic_stacktrace): Fix allocator usage in constructors. (basic_stacktrace::operator=(const basic_stacktrace&)): Do not try to reallocate using const allocator. (basic_stacktrace::operator=(basic_stacktrace&&)): Fix if-constexpr with non-constant condition. Do not allocate new storage if allocator propagates. Do not set _M_size if allocation fails. (basic_stacktrace::swap(basic_stacktrace&)): Fix typo. Add assertion that non-propagating allocators are equal. * testsuite/19_diagnostics/stacktrace/stacktrace.cc: New test. Diff: --- libstdc++-v3/include/std/stacktrace | 59 +++--- .../19_diagnostics/stacktrace/stacktrace.cc | 215 +++++++++++++++++++++ 2 files changed, 252 insertions(+), 22 deletions(-) diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace index 4e271cef3f3..dd78c71c5dc 100644 --- a/libstdc++-v3/include/std/stacktrace +++ b/libstdc++-v3/include/std/stacktrace @@ -301,7 +301,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { } basic_stacktrace(const basic_stacktrace& __other) noexcept - : basic_stacktrace(__other, __other._M_alloc) + : basic_stacktrace(__other, + _AllocTraits::select_on_container_copy_construction(__other._M_alloc)) { } basic_stacktrace(basic_stacktrace&& __other) noexcept @@ -326,13 +327,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_alloc(__alloc) { if constexpr (_Allocator::is_always_equal::value) - { - _M_impl = std::__exchange(__other._M_impl, {}); - } + _M_impl = std::__exchange(__other._M_impl, {}); else if (_M_alloc == __other._M_alloc) - { - _M_impl = std::__exchange(__other._M_impl, {}); - } + _M_impl = std::__exchange(__other._M_impl, {}); + else if (const auto __s = __other._M_impl._M_size) + if (auto __f = _M_impl._M_allocate(_M_alloc, __s)) + { + std::uninitialized_copy_n(__other.begin(), __s, __f); + _M_impl._M_size = __s; + } } basic_stacktrace& @@ -361,9 +364,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Need to allocate new storage. _M_clear(); - // Use the allocator we will have after this function returns. - auto& __alloc = __pocca ? __other._M_alloc : _M_alloc; - if (auto __f = _M_impl._M_allocate(__alloc, __s)) + if constexpr (__pocca) + _M_alloc = __other._M_alloc; + + if (auto __f = _M_impl._M_allocate(_M_alloc, __s)) { std::uninitialized_copy_n(__other.begin(), __s, __f); _M_impl._M_size = __s; @@ -376,10 +380,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION auto __to = std::copy_n(__other.begin(), __s, begin()); std::destroy(__to, end()); _M_impl._M_size = __s; - } - if constexpr (__pocca) - _M_alloc = __other._M_alloc; + if constexpr (__pocca) + _M_alloc = __other._M_alloc; + } return *this; } @@ -397,19 +401,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION std::swap(_M_impl, __other._M_impl); else if (_M_alloc == __other._M_alloc) std::swap(_M_impl, __other._M_impl); - else + else if constexpr (__pocma) { - const auto __s = __other.size(); + // Free current storage and take ownership of __other's storage. + _M_clear(); + _M_impl = std::__exchange(__other._M_impl, {}); + } + else // Allocators are unequal and don't propagate. + { + const size_type __s = __other.size(); - if constexpr (__pocma || _M_impl._M_capacity < __s) + if (_M_impl._M_capacity < __s) { // Need to allocate new storage. _M_clear(); - // Use the allocator we will have after this function returns. - auto& __alloc = __pocma ? __other._M_alloc : _M_alloc; - if (auto __f = _M_impl._M_allocate(__alloc, __s)) - std::uninitialized_copy_n(__other.begin(), __s, __f); + if (auto __f = _M_impl._M_allocate(_M_alloc, __s)) + { + std::uninitialized_copy_n(__other.begin(), __s, __f); + _M_impl._M_size = __s; + } } else { @@ -420,8 +431,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION auto __to = std::copy(__first, __mid, begin()); __to = std::uninitialized_copy(__mid, __last, __to); std::destroy(__to, end()); + _M_impl._M_size = __s; } - _M_impl._M_size = __s; } if constexpr (__pocma) @@ -503,9 +514,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void swap(basic_stacktrace& __other) noexcept { - std::swap(_M_impl. __other._M_impl); + std::swap(_M_impl, __other._M_impl); if constexpr (_AllocTraits::propagate_on_container_swap::value) std::swap(_M_alloc, __other._M_alloc); + else if constexpr (!_AllocTraits::is_always_equal::value) + { + __glibcxx_assert(_M_alloc == __other._M_alloc); + } } private: diff --git a/libstdc++-v3/testsuite/19_diagnostics/stacktrace/stacktrace.cc b/libstdc++-v3/testsuite/19_diagnostics/stacktrace/stacktrace.cc new file mode 100644 index 00000000000..8dfdf4739be --- /dev/null +++ b/libstdc++-v3/testsuite/19_diagnostics/stacktrace/stacktrace.cc @@ -0,0 +1,215 @@ +// { dg-options "-std=gnu++23 -lstdc++_libbacktrace" } +// { dg-do run { target c++23 } } +// { dg-require-effective-target stacktrace } + +#include <stacktrace> +#include "testsuite_allocator.h" + +static_assert( std::is_nothrow_default_constructible_v<std::stacktrace> ); +static_assert( std::is_copy_constructible_v<std::stacktrace> ); +static_assert( std::is_nothrow_move_constructible_v<std::stacktrace> ); +static_assert( std::is_copy_assignable_v<std::stacktrace> ); +static_assert( std::is_nothrow_move_assignable_v<std::stacktrace> ); +static_assert( std::is_nothrow_swappable_v<std::stacktrace> ); + +void +test_cons() +{ + { + using Stacktrace = std::stacktrace; + using Alloc = Stacktrace::allocator_type; + + Stacktrace s0; + VERIFY( s0.empty() ); + VERIFY( s0.size() == 0 ); + VERIFY( s0.begin() == s0.end() ); + + Stacktrace s1(Alloc{}); + VERIFY( s1.empty() ); + VERIFY( s1.size() == 0 ); + VERIFY( s1.begin() == s1.end() ); + + VERIFY( s0 == s1 ); + + Stacktrace s2(s0); + VERIFY( s2 == s0 ); + + const Stacktrace curr = Stacktrace::current(); + + Stacktrace s3(curr); + VERIFY( ! s3.empty() ); + VERIFY( s3.size() != 0 ); + VERIFY( s3.begin() != s3.end() ); + VERIFY( s3 != s0 ); + + Stacktrace s4(s3); + VERIFY( ! s4.empty() ); + VERIFY( s4.size() != 0 ); + VERIFY( s4.begin() != s4.end() ); + VERIFY( s4 == s3 ); + VERIFY( s4 != s0 ); + + Stacktrace s5(std::move(s3)); + VERIFY( ! s5.empty() ); + VERIFY( s5.size() != 0 ); + VERIFY( s5.begin() != s5.end() ); + VERIFY( s5 == s4 ); + VERIFY( s5 != s0 ); + VERIFY( s3 == s0 ); + + Stacktrace s6(s4, Alloc{}); + VERIFY( s6 == s4 ); + + Stacktrace s7(std::move(s6), Alloc{}); + VERIFY( s7 == s4 ); + } + + { + using Alloc = __gnu_test::uneq_allocator<std::stacktrace_entry>; + using Stacktrace = std::basic_stacktrace<Alloc>; + + Stacktrace s0; + VERIFY( s0.empty() ); + VERIFY( s0.size() == 0 ); + VERIFY( s0.begin() == s0.end() ); + + Stacktrace s1(Alloc{}); + VERIFY( s1.empty() ); + VERIFY( s1.size() == 0 ); + VERIFY( s1.begin() == s1.end() ); + + VERIFY( s0 == s1 ); + + Stacktrace s2(s0); + VERIFY( s2 == s0 ); + + const Stacktrace curr = Stacktrace::current(); + + Stacktrace s3(curr); + VERIFY( ! s3.empty() ); + VERIFY( s3.size() != 0 ); + VERIFY( s3.begin() != s3.end() ); + VERIFY( s3 != s0 ); + + Stacktrace s4(s3); + VERIFY( ! s4.empty() ); + VERIFY( s4.size() != 0 ); + VERIFY( s4.begin() != s4.end() ); + VERIFY( s4 == s3 ); + VERIFY( s4 != s0 ); + + Stacktrace s5(std::move(s3)); + VERIFY( ! s5.empty() ); + VERIFY( s5.size() != 0 ); + VERIFY( s5.begin() != s5.end() ); + VERIFY( s5 == s4 ); + VERIFY( s5 != s0 ); + VERIFY( s3 == s0 ); + + // TODO test allocator-extended copy/move + + // TODO test allocator propagation + } +} + + +void +test_assign() +{ + { + using Stacktrace = std::stacktrace; + + Stacktrace s0; + s0 = s0; + VERIFY( s0.empty() ); + s0 = std::move(s0); + VERIFY( s0.empty() ); + + Stacktrace s1 = Stacktrace::current(); + VERIFY( s1 != s0 ); + s0 = s1; + VERIFY( s0 == s1 ); + VERIFY( s0.at(0).source_line() == (__LINE__ - 4) ); + + s1 = Stacktrace::current(); + VERIFY( s1 != s0 ); + Stacktrace s2 = s0; + Stacktrace s3 = s1; + s0 = std::move(s1); + VERIFY( s0 == s3 ); + VERIFY( s1 == s2 ); // ISO C++: valid but unspecified; GCC: swapped. + } + + { + using Alloc = __gnu_test::uneq_allocator<std::stacktrace_entry>; + using Stacktrace = std::basic_stacktrace<Alloc>; + + Stacktrace s0; + s0 = s0; + VERIFY( s0.empty() ); + s0 = std::move(s0); + VERIFY( s0.empty() ); + + Stacktrace s1 = Stacktrace::current(); + VERIFY( s1 != s0 ); + s0 = s1; + VERIFY( s0 == s1 ); + + s1 = Stacktrace::current(Alloc(__LINE__)); + VERIFY( s1 != s0 ); + s0 = std::move(s1); + VERIFY( s0.at(0).source_line() == s0.get_allocator().get_personality() ); + VERIFY( s1.empty() ); // ISO C++: valid but unspecified; GCC: empty. + + Stacktrace s2 = Stacktrace::current(s0.get_allocator()); + Stacktrace s3 = s2; + s2 = std::move(s0); + VERIFY( s2.at(0).source_line() == s2.get_allocator().get_personality() ); + VERIFY( s0 == s3 ); // ISO C++: valid but unspecified, GCC: swapped. + } +} + +void +test_swap() +{ + { + using Stacktrace = std::stacktrace; + + Stacktrace s0; + Stacktrace s1 = Stacktrace::current(); + swap(s0, s1); + VERIFY( s1.empty() ); + VERIFY( ! s0.empty() ); + } + + { + using Alloc = __gnu_test::uneq_allocator<std::stacktrace_entry>; + using Stacktrace = std::basic_stacktrace<Alloc>; + + Stacktrace s0; + Stacktrace s1 = Stacktrace::current(); + swap(s0, s1); + VERIFY( s1.empty() ); + VERIFY( ! s0.empty() ); + + // TODO test allocator propagation + } +} + +void +test_pr105031() +{ + // PR libstdc++/105031 + // wrong constexpr if statement in operator=(basic_stacktrace&&) + using Alloc = __gnu_test::uneq_allocator<std::stacktrace_entry>; + std::basic_stacktrace<Alloc> s; + s = auto(s); +} + +int main() +{ + test_cons(); + test_assign(); + test_swap(); + test_pr105031(); +}
reply other threads:[~2022-04-11 17:01 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220411170111.67E83385CFE9@sourceware.org \ --to=redi@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ --cc=libstdc++-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).