From patchwork Wed Jan 18 20:27:42 2012 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: 136693 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]) by ozlabs.org (Postfix) with SMTP id C943FB6EE7 for ; Thu, 19 Jan 2012 07:28:08 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1327523289; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC: Subject:References:In-Reply-To:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=zWLz/TpVr5LIOBiA2qCmGqdObC0=; b=f0YfIyaPxohCRg5wpUapIN7sMfuRUblynLRtd2b+hMS3yrukSEBGRHXfbqrJaW NE29XWh6KhWx292NyjBZ6OBG7wSmc9Tky2lfqPf+r+2zKJPZXNG9AYXUgxk/H198 qQirTVlsHj8zbwNrTet0xKSBNyb51OfTkU4WTXWco7oeM= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=SuXb+052bW57PSMwIQK4kjwsh4aj0Z5k6hPl2HOPC7Jz/7L48LfSCTxSENzmyr 2eDeHKpJ37igxwvR+FdRchlBFW6QiW0FUVtj1vK7ecpflNbmRvAf2Qyd7bcQwVXU HE37l911oE9jAdr9cmG1g5BTLgrRNous5BmlmxKiTor6Q=; Received: (qmail 11579 invoked by alias); 18 Jan 2012 20:28:04 -0000 Received: (qmail 11564 invoked by uid 22791); 18 Jan 2012 20:28:02 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-bk0-f47.google.com (HELO mail-bk0-f47.google.com) (209.85.214.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 18 Jan 2012 20:27:47 +0000 Received: by bkar19 with SMTP id r19so1675063bka.20 for ; Wed, 18 Jan 2012 12:27:46 -0800 (PST) Received: by 10.204.150.7 with SMTP id w7mr6140966bkv.107.1326918466035; Wed, 18 Jan 2012 12:27:46 -0800 (PST) Received: from localhost.localdomain (arf62-1-82-237-250-248.fbx.proxad.net. [82.237.250.248]) by mx.google.com with ESMTPS id ga13sm20692139bkc.5.2012.01.18.12.27.43 (version=SSLv3 cipher=OTHER); Wed, 18 Jan 2012 12:27:44 -0800 (PST) Message-ID: <4F172B3E.4030600@gmail.com> Date: Wed, 18 Jan 2012 21:27:42 +0100 From: =?ISO-8859-1?Q?Fran=E7ois_Dumont?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111109 Thunderbird/8.0 MIME-Version: 1.0 To: Paolo Carlini CC: Jonathan Wakely , "libstdc++@gcc.gnu.org" , gcc-patches@gcc.gnu.org Subject: Re: libstdc++/51866 too, sorry References: <4F1381E8.50909@oracle.com> <4F1489C4.3030807@gmail.com> <4F15E4BC.2070409@gmail.com> <4F1605B5.9000801@oracle.com> In-Reply-To: <4F1605B5.9000801@oracle.com> 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 Attached patch applied. 2012-01-18 François Dumont Roman Kononov PR libstdc++/51866 * include/bits/hashtable.h (_Hashtable<>::_M_insert(_Arg, false_type)): Do not keep a reference to a potentially moved instance. * testsuite/23_containers/unordered_multiset/insert/51866.cc: New. * testsuite/23_containers/unordered_multimap/insert/51866.cc: New. François On 01/18/2012 12:35 AM, Paolo Carlini wrote: > On 01/17/2012 11:31 PM, Jonathan Wakely wrote: >> On 17 January 2012 21:14, François Dumont wrote: >>> Ok to commit ? >> OK, thanks. > Great. Please also double check with the submitters of libstdc++/51845 > (ask on the audit trail?) that it's actually fixed by the same patch, > and in case resolve it as duplicate. > > Thanks again, > Paolo. > > Index: include/bits/hashtable.h =================================================================== --- include/bits/hashtable.h (revision 183164) +++ include/bits/hashtable.h (working copy) @@ -1227,7 +1227,7 @@ = this->_M_hash_code(__k); this->_M_store_code(__new_node, __code); - // Second, do rehash if necessary. + // Second, do rehash if necessary. if (__do_rehash.first) _M_rehash(__do_rehash.second, __saved_state); @@ -1347,21 +1347,24 @@ = _M_rehash_policy._M_need_rehash(_M_bucket_count, _M_element_count, 1); - const key_type& __k = this->_M_extract()(__v); - typename _Hashtable::_Hash_code_type __code = this->_M_hash_code(__k); + // First compute the hash code so that we don't do anything if it throws. + typename _Hashtable::_Hash_code_type __code + = this->_M_hash_code(this->_M_extract()(__v)); _Node* __new_node = nullptr; __try { - // First allocate new node so that we don't rehash if it throws. + // Second allocate new node so that we don't rehash if it throws. __new_node = _M_allocate_node(std::forward<_Arg>(__v)); this->_M_store_code(__new_node, __code); if (__do_rehash.first) _M_rehash(__do_rehash.second, __saved_state); - // Second, find the node before an equivalent one. - size_type __n = _M_bucket_index(__k, __code); - _BaseNode* __prev = _M_find_before_node(__n, __k, __code); + // Third, find the node before an equivalent one. + size_type __bkt = _M_bucket_index(__new_node); + _BaseNode* __prev + = _M_find_before_node(__bkt, this->_M_extract()(__new_node->_M_v), + __code); if (__prev) { // Insert after the node before the equivalent one. @@ -1372,7 +1375,7 @@ // The inserted node has no equivalent in the hashtable. We must // insert the new node at the beginning of the bucket to preserve // equivalent elements relative positions. - _M_insert_bucket_begin(__n, __new_node); + _M_insert_bucket_begin(__bkt, __new_node); ++_M_element_count; return iterator(__new_node); } Index: testsuite/23_containers/unordered_multimap/insert/51866.cc =================================================================== --- testsuite/23_containers/unordered_multimap/insert/51866.cc (revision 0) +++ testsuite/23_containers/unordered_multimap/insert/51866.cc (revision 0) @@ -0,0 +1,87 @@ +// { dg-options "-std=gnu++0x" } +// +// Copyright (C) 2012 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 +// . + +#include +#include + +struct num +{ + int value; + num(int n) : value(n) {} + num(num const&) = default; + num& operator=(num const&) = default; + num(num&& o) : value(o.value) + { o.value = -1; } + num& operator=(num&& o) + { + if (this != &o) + { + value = o.value; + o.value = -1; + } + return *this; + } +}; + +struct num_hash +{ + size_t operator()(num const& a) const + { return a.value; } +}; + +struct num_equal +{ + static bool _S_invalid_equal_call; + bool operator()(num const& a, num const& b) const + { + if (a.value == -1 || b.value == -1) + _S_invalid_equal_call = true; + return a.value == b.value; + } +}; + +bool num_equal::_S_invalid_equal_call = false; + +// libstdc++/51866 +void test01() +{ + bool test __attribute__((unused)) = true; + + std::unordered_multimap mmap; + mmap.insert(std::make_pair(num(1), 1)); + mmap.insert(std::make_pair(num(2), 2)); + mmap.insert(std::make_pair(num(1), 3)); + mmap.insert(std::make_pair(num(2), 4)); + VERIFY( mmap.size() == 4 ); + auto iter = mmap.cbegin(); + auto x0 = (iter++)->first.value; + auto x1 = (iter++)->first.value; + auto x2 = (iter++)->first.value; + auto x3 = (iter++)->first.value; + VERIFY( iter == mmap.cend() ); + VERIFY( (x0 == 1 && x1 == 1 && x2 == 2 && x3 == 2) + || (x0 == 2 && x1 == 2 && x2 == 1 && x3 == 1) ); + VERIFY( !num_equal::_S_invalid_equal_call ); +} + +int main() +{ + test01(); + return 0; +} Index: testsuite/23_containers/unordered_multiset/insert/51866.cc =================================================================== --- testsuite/23_containers/unordered_multiset/insert/51866.cc (revision 0) +++ testsuite/23_containers/unordered_multiset/insert/51866.cc (revision 0) @@ -0,0 +1,87 @@ +// { dg-options "-std=gnu++0x" } +// +// Copyright (C) 2012 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 +// . + +#include +#include + +struct num +{ + int value; + num(int n) : value(n) {} + num(num const&) = default; + num& operator=(num const&) = default; + num(num&& o) : value(o.value) + { o.value = -1; } + num& operator=(num&& o) + { + if (this != &o) + { + value = o.value; + o.value = -1; + } + return *this; + } +}; + +struct num_hash +{ + size_t operator()(num const& a) const + { return a.value; } +}; + +struct num_equal +{ + static bool _S_invalid_equal_call; + bool operator()(num const& a, num const& b) const + { + if (a.value == -1 || b.value == -1) + _S_invalid_equal_call = true; + return a.value == b.value; + } +}; + +bool num_equal::_S_invalid_equal_call = false; + +// libstdc++/51866 +void test01() +{ + bool test __attribute__((unused)) = true; + + std::unordered_multiset mset; + mset.insert(num(1)); + mset.insert(num(2)); + mset.insert(num(1)); + mset.insert(num(2)); + VERIFY( mset.size() == 4 ); + auto iter = mset.cbegin(); + int x0 = (iter++)->value; + int x1 = (iter++)->value; + int x2 = (iter++)->value; + int x3 = (iter++)->value; + VERIFY( iter == mset.cend() ); + VERIFY( (x0 == 1 && x1 == 1 && x2 == 2 && x3 == 2) + || (x0 == 2 && x1 == 2 && x2 == 1 && x3 == 1) ); + VERIFY( !num_equal::_S_invalid_equal_call ); +} + +int main() +{ + test01(); + return 0; +}