From patchwork Fri Dec 27 18:27:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Fran=C3=A7ois_Dumont?= X-Patchwork-Id: 305461 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 67C722C009B for ; Sat, 28 Dec 2013 05:28:15 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=Xd+1gxBovWah5j0Rr9t8lRThc2jEI2IrtqTmFWDijhKYP4 OR4qrvx/s5ak2/kXk65OLbpAhn7i4Uw9r2Jy8IOTjd5MZouIhTkd5E4vdImRwZDD NV6XTLr3Y5wEfY+Zs0Cbk/DFj9Tce6RIM5Y/4dSCS4LhG86T0s3nQEuU7zHgg= 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 :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=OsKyOHOmg/fgxoyFk2xJwamL6Eo=; b=JpSkhI61To8Sp0ymshj7 7g6VB93QB0Be3Kl1DD44F4o6lodt9K10mbtE6ZHWbeefCzcIvoRJqarb9WfuoZjz YBS90jtqjGM8uuRDFKO6RXPrkw2KcWaSdWncnQlHzJETcqd7wtOVi8Qtl09Eb62A THes3cYGs+6NTjVWG324EBM= Received: (qmail 22456 invoked by alias); 27 Dec 2013 18:28:07 -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 22437 invoked by uid 89); 27 Dec 2013 18:28:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-we0-f179.google.com Received: from mail-we0-f179.google.com (HELO mail-we0-f179.google.com) (74.125.82.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 27 Dec 2013 18:28:03 +0000 Received: by mail-we0-f179.google.com with SMTP id q59so8265100wes.24 for ; Fri, 27 Dec 2013 10:28:00 -0800 (PST) X-Received: by 10.194.108.36 with SMTP id hh4mr9896537wjb.14.1388168880203; Fri, 27 Dec 2013 10:28:00 -0800 (PST) Received: from localhost.localdomain (arf62-1-82-237-250-248.fbx.proxad.net. [82.237.250.248]) by mx.google.com with ESMTPSA id y13sm10755345wjr.8.2013.12.27.10.27.58 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 27 Dec 2013 10:27:58 -0800 (PST) Message-ID: <52BDC6AD.5030207@gmail.com> Date: Fri, 27 Dec 2013 19:27:57 +0100 From: =?ISO-8859-1?Q?Fran=E7ois_Dumont?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120829 Thunderbird/15.0 MIME-Version: 1.0 To: "libstdc++@gcc.gnu.org" , gcc-patches Subject: std::vector move assign patch Hi Here is a patch to fix an issue in normal mode during the move assignment. The destination vector allocator instance is moved too during the assignment which is wrong. As I discover this problem while working on issues with management of safe iterators during move operations this patch also fix those issues in the debug mode for the vector container. Fixes for other containers in debug mode will come later. 2013-12-27 François Dumont * include/bits/stl_vector.h (std::vector<>::_M_move_assign): Pass *this allocator instance when building temporary vector instance so that *this allocator do not get moved. * include/debug/safe_base.h (_Safe_sequence_base(_Safe_sequence_base&&)): New. * include/debug/vector (__gnu_debug::vector<>(vector&&)): Use latter. (__gnu_debug::vector<>(vector&&, const allocator_type&)): Swap safe iterators if the instance is moved. (__gnu_debug::vector<>::operator=(vector&&)): Likewise. * testsuite/23_containers/vector/allocator/move.cc (test01): Add check on a vector iterator. * testsuite/23_containers/vector/allocator/move_assign.cc (test02): Likewise. (test03): New, test with a non-propagating allocator. * testsuite/23_containers/vector/debug/move_assign_neg.cc: New. Tested under Linux x86_64 normal and debug modes. I will be in vacation for a week starting today so if you want to apply it quickly do not hesitate to do it yourself. François Index: include/bits/stl_vector.h =================================================================== --- include/bits/stl_vector.h (revision 206214) +++ include/bits/stl_vector.h (working copy) @@ -1433,7 +1433,7 @@ void _M_move_assign(vector&& __x, std::true_type) noexcept { - const vector __tmp(std::move(*this)); + const vector __tmp(std::move(*this), get_allocator()); this->_M_impl._M_swap_data(__x._M_impl); if (_Alloc_traits::_S_propagate_on_move_assign()) std::__alloc_on_move(_M_get_Tp_allocator(), Index: include/debug/safe_base.h =================================================================== --- include/debug/safe_base.h (revision 206214) +++ include/debug/safe_base.h (working copy) @@ -192,6 +192,12 @@ : _M_iterators(0), _M_const_iterators(0), _M_version(1) { } +#if __cplusplus >= 201103L + _Safe_sequence_base(_Safe_sequence_base&& __x) noexcept + : _Safe_sequence_base() + { _M_swap(__x); } +#endif + /** Notify all iterators that reference this sequence that the sequence is being destroyed. */ ~_Safe_sequence_base() Index: include/debug/vector =================================================================== --- include/debug/vector (revision 206214) +++ include/debug/vector (working copy) @@ -52,6 +52,7 @@ typedef __gnu_debug::_Equal_to<_Base_const_iterator> _Equal; #if __cplusplus >= 201103L + typedef __gnu_debug::_Safe_sequence > _Safe_base; typedef __gnu_cxx::__alloc_traits<_Allocator> _Alloc_traits; #endif @@ -111,18 +112,16 @@ vector(const vector& __x) : _Base(__x), _M_guaranteed_capacity(__x.size()) { } - /// Construction from a release-mode vector + /// Construction from a normal-mode vector vector(const _Base& __x) : _Base(__x), _M_guaranteed_capacity(__x.size()) { } #if __cplusplus >= 201103L vector(vector&& __x) noexcept : _Base(std::move(__x)), + _Safe_base(std::move(__x)), _M_guaranteed_capacity(this->size()) - { - this->_M_swap(__x); - __x._M_guaranteed_capacity = 0; - } + { __x._M_guaranteed_capacity = 0; } vector(const vector& __x, const allocator_type& __a) : _Base(__x, __a), _M_guaranteed_capacity(__x.size()) { } @@ -131,7 +130,10 @@ : _Base(std::move(__x), __a), _M_guaranteed_capacity(this->size()) { - __x._M_invalidate_all(); + if (__x.get_allocator() == __a) + this->_M_swap(__x); + else + __x._M_invalidate_all(); __x._M_guaranteed_capacity = 0; } @@ -146,7 +148,7 @@ vector& operator=(const vector& __x) { - static_cast<_Base&>(*this) = __x; + _M_base() = __x; this->_M_invalidate_all(); _M_update_guaranteed_capacity(); return *this; @@ -157,8 +159,13 @@ operator=(vector&& __x) noexcept(_Alloc_traits::_S_nothrow_move()) { __glibcxx_check_self_move_assign(__x); - _Base::operator=(std::move(__x)); - this->_M_invalidate_all(); + bool xfer_memory = _Alloc_traits::_S_propagate_on_move_assign() + || __x.get_allocator() == this->get_allocator(); + _M_base() = std::move(__x._M_base()); + if (xfer_memory) + this->_M_swap(__x); + else + this->_M_invalidate_all(); _M_update_guaranteed_capacity(); __x._M_invalidate_all(); __x._M_guaranteed_capacity = 0; @@ -168,7 +175,7 @@ vector& operator=(initializer_list __l) { - static_cast<_Base&>(*this) = __l; + _M_base() = __l; this->_M_invalidate_all(); _M_update_guaranteed_capacity(); return *this; Index: testsuite/23_containers/vector/allocator/move.cc =================================================================== --- testsuite/23_containers/vector/allocator/move.cc (revision 206214) +++ testsuite/23_containers/vector/allocator/move.cc (working copy) @@ -32,9 +32,11 @@ typedef std::vector test_type; test_type v1(alloc_type(1)); v1 = { T() }; + auto it = v1.begin(); test_type v2(std::move(v1)); VERIFY(1 == v1.get_allocator().get_personality()); VERIFY(1 == v2.get_allocator().get_personality()); + VERIFY( it == v2.begin() ); } void test02() Index: testsuite/23_containers/vector/allocator/move_assign.cc =================================================================== --- testsuite/23_containers/vector/allocator/move_assign.cc (revision 206214) +++ testsuite/23_containers/vector/allocator/move_assign.cc (working copy) @@ -46,16 +46,35 @@ typedef std::vector test_type; test_type v1(alloc_type(1)); v1.push_back(T()); + auto it = v1.begin(); test_type v2(alloc_type(2)); + v2.push_back(T()); v2 = std::move(v1); - v2.push_back(T()); + VERIFY( it == v2.begin() ); VERIFY(0 == v1.get_allocator().get_personality()); VERIFY(1 == v2.get_allocator().get_personality()); } +void test03() +{ + bool test __attribute__((unused)) = true; + typedef propagating_allocator alloc_type; + typedef std::vector test_type; + test_type v1(alloc_type(1)); + v1.push_back(T()); + auto it = v1.begin(); + test_type v2(alloc_type(1)); + v2.push_back(T()); + v2 = std::move(v1); + VERIFY( it == v2.begin() ); + VERIFY(1 == v1.get_allocator().get_personality()); + VERIFY(1 == v2.get_allocator().get_personality()); +} + int main() { test01(); test02(); + test03(); return 0; } Index: testsuite/23_containers/vector/debug/move_assign_neg.cc =================================================================== --- testsuite/23_containers/vector/debug/move_assign_neg.cc (revision 0) +++ testsuite/23_containers/vector/debug/move_assign_neg.cc (revision 0) @@ -0,0 +1,50 @@ +// Copyright (C) 2013 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-options "-std=gnu++11" } +// { dg-do run { xfail *-*-* } } + +#include + +#include +#include + +void test01() +{ + bool test __attribute__((unused)) = true; + + typedef __gnu_test::uneq_allocator alloc_type; + typedef __gnu_debug::vector test_type; + + test_type v1(alloc_type(1)); + v1 = { 0, 1, 2, 3 }; + + test_type v2(alloc_type(2)); + v2 = { 4, 5, 6, 7 }; + + auto it = v2.begin(); + + v1 = std::move(v2); + + VERIFY( it == v1.begin() ); // Error, it singular +} + +int main() +{ + test01(); + return 0; +}