From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id AA02D3855BB1 for ; Mon, 11 Apr 2022 17:01:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AA02D3855BB1 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-115-vt0wX8cvMhOHGHB9fHIZ0w-1; Mon, 11 Apr 2022 13:01:34 -0400 X-MC-Unique: vt0wX8cvMhOHGHB9fHIZ0w-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E288086B8AF; Mon, 11 Apr 2022 17:01:33 +0000 (UTC) Received: from localhost (unknown [10.33.36.45]) by smtp.corp.redhat.com (Postfix) with ESMTP id AC16B200D8FC; Mon, 11 Apr 2022 17:01:33 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Fix std::basic_stacktrace special members [PR105031] Date: Mon, 11 Apr 2022 18:01:33 +0100 Message-Id: <20220411170133.169581-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Apr 2022 17:01:39 -0000 Tested x86_64-linux, pushed to trunk. -- >8 -- 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. --- libstdc++-v3/include/std/stacktrace | 59 +++-- .../19_diagnostics/stacktrace/stacktrace.cc | 215 ++++++++++++++++++ 2 files changed, 252 insertions(+), 22 deletions(-) create mode 100644 libstdc++-v3/testsuite/19_diagnostics/stacktrace/stacktrace.cc 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 +#include "testsuite_allocator.h" + +static_assert( std::is_nothrow_default_constructible_v ); +static_assert( std::is_copy_constructible_v ); +static_assert( std::is_nothrow_move_constructible_v ); +static_assert( std::is_copy_assignable_v ); +static_assert( std::is_nothrow_move_assignable_v ); +static_assert( std::is_nothrow_swappable_v ); + +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; + using Stacktrace = std::basic_stacktrace; + + 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; + using Stacktrace = std::basic_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 ); + + 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; + using Stacktrace = std::basic_stacktrace; + + 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::basic_stacktrace s; + s = auto(s); +} + +int main() +{ + test_cons(); + test_assign(); + test_swap(); + test_pr105031(); +} -- 2.34.1