From patchwork Thu Oct 17 14:21:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 1178670 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-511227-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="RF3AGyM3"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46vBDb4PQKz9sPF for ; Fri, 18 Oct 2019 01:21:33 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=i+8nOEgwbt34hvWPMUAH0rh/+axi1w2N5odML37c4YrxieXlVAIZ9 lhp9AdHzxcwsGEua4b4yxfmKX41uPpb31eFSKfbQ/RDH4HQQjsK4tuqNSwCp61i8 rlg9wnzA2ViBeu/fhIkFYXAYx0xwrvf4sSM/RfGh8wTaztcUUElIJY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=zVq06RflFf+tODDKamGExscxRyA=; b=RF3AGyM3vcuPWUxmJtt0 pX+KWNBnMCtSX0Uv59bAHZ2lssE0aVwNushPrSxnoBb/bEw3TQVFxSXSLmjzA4+K lRT9ukDtgD+rFlJJHxW5VEPz9xClDg3V2XZqheCmxiT/QrlT9/MYcYctEiV/lFSs JMgqFGcf5jWrWqGKMtGYMxs= Received: (qmail 108934 invoked by alias); 17 Oct 2019 14:21:25 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 108914 invoked by uid 89); 17 Oct 2019 14:21:25 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 17 Oct 2019 14:21:22 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B0DF72A09C5; Thu, 17 Oct 2019 14:21:21 +0000 (UTC) Received: from localhost (unknown [10.33.36.149]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3AF59600C8; Thu, 17 Oct 2019 14:21:21 +0000 (UTC) Date: Thu, 17 Oct 2019 15:21:20 +0100 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH] PR libstdc++/92124 fix incorrect container move assignment Message-ID: <20191017142120.GA32212@redhat.com> MIME-Version: 1.0 Content-Disposition: inline X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.12.1 (2019-06-15) The container requirements say that for move assignment "All existing elements of [the target] are either move assigned or destroyed". Some of our containers currently use __make_move_if_noexcept which makes the move depend on whether the element type is nothrow move constructible. This is incorrect, because the standard says we must move assign, not move or copy depending on the move constructor. Use make_move_iterator instead so that we move unconditionally. This ensures existing elements won't be copy assigned. PR libstdc++/92124 * include/bits/forward_list.h (_M_move_assign(forward_list&&, false_type)): Do not use __make_move_if_noexcept, instead move unconditionally. * include/bits/stl_deque.h (_M_move_assign2(deque&&, false_type)): Likewise. * include/bits/stl_list.h (_M_move_assign(list&&, false_type)): Likewise. * include/bits/stl_vector.h (_M_move_assign(vector&&, false_type)): Likewise. * testsuite/23_containers/vector/92124.cc: New test. Tested x86_64-linux, committed to trunk. commit 66f4aa35299bb8e967aa54930b27815cf8161693 Author: Jonathan Wakely Date: Thu Oct 17 14:27:53 2019 +0100 PR libstdc++/92124 fix incorrect container move assignment The container requirements say that for move assignment "All existing elements of [the target] are either move assigned or destroyed". Some of our containers currently use __make_move_if_noexcept which makes the move depend on whether the element type is nothrow move constructible. This is incorrect, because the standard says we must move assign, not move or copy depending on the move constructor. Use make_move_iterator instead so that we move unconditionally. This ensures existing elements won't be copy assigned. PR libstdc++/92124 * include/bits/forward_list.h (_M_move_assign(forward_list&&, false_type)): Do not use __make_move_if_noexcept, instead move unconditionally. * include/bits/stl_deque.h (_M_move_assign2(deque&&, false_type)): Likewise. * include/bits/stl_list.h (_M_move_assign(list&&, false_type)): Likewise. * include/bits/stl_vector.h (_M_move_assign(vector&&, false_type)): Likewise. * testsuite/23_containers/vector/92124.cc: New test. diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h index e686283a432..cab2ae788a7 100644 --- a/libstdc++-v3/include/bits/forward_list.h +++ b/libstdc++-v3/include/bits/forward_list.h @@ -1336,8 +1336,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER else // The rvalue's allocator cannot be moved, or is not equal, // so we need to individually move each element. - this->assign(std::__make_move_if_noexcept_iterator(__list.begin()), - std::__make_move_if_noexcept_iterator(__list.end())); + this->assign(std::make_move_iterator(__list.begin()), + std::make_move_iterator(__list.end())); } // Called by assign(_InputIterator, _InputIterator) if _Tp is diff --git a/libstdc++-v3/include/bits/stl_deque.h b/libstdc++-v3/include/bits/stl_deque.h index ac76d681ff0..50491e76ff5 100644 --- a/libstdc++-v3/include/bits/stl_deque.h +++ b/libstdc++-v3/include/bits/stl_deque.h @@ -2256,8 +2256,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { // The rvalue's allocator cannot be moved and is not equal, // so we need to individually move each element. - _M_assign_aux(std::__make_move_if_noexcept_iterator(__x.begin()), - std::__make_move_if_noexcept_iterator(__x.end()), + _M_assign_aux(std::make_move_iterator(__x.begin()), + std::make_move_iterator(__x.end()), std::random_access_iterator_tag()); __x.clear(); } diff --git a/libstdc++-v3/include/bits/stl_list.h b/libstdc++-v3/include/bits/stl_list.h index 701982538df..328a79851a8 100644 --- a/libstdc++-v3/include/bits/stl_list.h +++ b/libstdc++-v3/include/bits/stl_list.h @@ -1957,8 +1957,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 else // The rvalue's allocator cannot be moved, or is not equal, // so we need to individually move each element. - _M_assign_dispatch(std::__make_move_if_noexcept_iterator(__x.begin()), - std::__make_move_if_noexcept_iterator(__x.end()), + _M_assign_dispatch(std::make_move_iterator(__x.begin()), + std::make_move_iterator(__x.end()), __false_type{}); } #endif diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h index d33e589498a..ff08b266692 100644 --- a/libstdc++-v3/include/bits/stl_vector.h +++ b/libstdc++-v3/include/bits/stl_vector.h @@ -1828,8 +1828,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { // The rvalue's allocator cannot be moved and is not equal, // so we need to individually move each element. - this->assign(std::__make_move_if_noexcept_iterator(__x.begin()), - std::__make_move_if_noexcept_iterator(__x.end())); + this->_M_assign_aux(std::make_move_iterator(__x.begin()), + std::make_move_iterator(__x.end()), + std::random_access_iterator_tag()); __x.clear(); } } diff --git a/libstdc++-v3/testsuite/23_containers/deque/92124.cc b/libstdc++-v3/testsuite/23_containers/deque/92124.cc new file mode 100644 index 00000000000..6f8cf5560c1 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/deque/92124.cc @@ -0,0 +1,49 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-do run { target c++11 } } + +#include +#include + +struct X { + X() = default; + X(const X&) = default; + + // Move constructor might throw + X(X&&) noexcept(false) {} + + // Tracking calls to assignment functions + X& operator=(const X&) { throw 1; } + + X& operator=(X&&) noexcept(true) { return *this; } +}; + +void +test01() +{ + using A = __gnu_test::propagating_allocator; + A a1(1), a2(2); + std::deque v1(1, a1), v2(1, a2); + v1 = std::move(v2); +} + +int +main() +{ + test01(); +} diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/92124.cc b/libstdc++-v3/testsuite/23_containers/forward_list/92124.cc new file mode 100644 index 00000000000..52a28073daf --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/forward_list/92124.cc @@ -0,0 +1,49 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-do run { target c++11 } } + +#include +#include + +struct X { + X() = default; + X(const X&) = default; + + // Move constructor might throw + X(X&&) noexcept(false) {} + + // Tracking calls to assignment functions + X& operator=(const X&) { throw 1; } + + X& operator=(X&&) noexcept(true) { return *this; } +}; + +void +test01() +{ + using A = __gnu_test::propagating_allocator; + A a1(1), a2(2); + std::forward_list v1(1, a1), v2(1, a2); + v1 = std::move(v2); +} + +int +main() +{ + test01(); +} diff --git a/libstdc++-v3/testsuite/23_containers/list/92124.cc b/libstdc++-v3/testsuite/23_containers/list/92124.cc new file mode 100644 index 00000000000..117cb71201b --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/list/92124.cc @@ -0,0 +1,49 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-do run { target c++11 } } + +#include +#include + +struct X { + X() = default; + X(const X&) = default; + + // Move constructor might throw + X(X&&) noexcept(false) {} + + // Tracking calls to assignment functions + X& operator=(const X&) { throw 1; } + + X& operator=(X&&) noexcept(true) { return *this; } +}; + +void +test01() +{ + using A = __gnu_test::propagating_allocator; + A a1(1), a2(2); + std::list v1(1, a1), v2(1, a2); + v1 = std::move(v2); +} + +int +main() +{ + test01(); +} diff --git a/libstdc++-v3/testsuite/23_containers/vector/92124.cc b/libstdc++-v3/testsuite/23_containers/vector/92124.cc new file mode 100644 index 00000000000..3cb487d39f4 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/vector/92124.cc @@ -0,0 +1,49 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-do run { target c++11 } } + +#include +#include + +struct X { + X() = default; + X(const X&) = default; + + // Move constructor might throw + X(X&&) noexcept(false) {} + + // Tracking calls to assignment functions + X& operator=(const X&) { throw 1; } + + X& operator=(X&&) noexcept(true) { return *this; } +}; + +void +test01() +{ + using A = __gnu_test::propagating_allocator; + A a1(1), a2(2); + std::vector v1(1, a1), v2(1, a2); + v1 = std::move(v2); +} + +int +main() +{ + test01(); +}