From patchwork Thu Aug 12 00:03:59 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Carlini X-Patchwork-Id: 61517 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 336E2B70DA for ; Thu, 12 Aug 2010 10:04:27 +1000 (EST) Received: (qmail 17589 invoked by alias); 12 Aug 2010 00:04:17 -0000 Received: (qmail 17490 invoked by uid 22791); 12 Aug 2010 00:04:14 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from vsmtp1.tin.it (HELO vsmtp1.tin.it) (212.216.176.141) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 12 Aug 2010 00:04:02 +0000 Received: from [192.168.0.4] (79.52.212.149) by vsmtp1.tin.it (8.0.022) id 4B9917F00E9BBE49; Thu, 12 Aug 2010 02:03:59 +0200 Message-ID: <4C633A6F.7080506@oracle.com> Date: Thu, 12 Aug 2010 02:03:59 +0200 From: Paolo Carlini User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.11) Gecko/20100714 SUSE/3.0.6 Thunderbird/3.0.6 MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" CC: libstdc++ Subject: [v3] Fix forward_list::remove vs LWG 526-type situations X-IsSubscribed: yes 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 Hi, noticed while adding one more use of std::__addressof to _Hashtable: fixed exactly like the list::remove case. Tested x86_64-linux, committed. Paolo. //////////////////// 2010-08-11 Paolo Carlini * include/bits/hashtable.h (_Hashtable<>::erase(const key_type&)): Use std::__addressof. * include/bits/forward_list.tcc (forward_list<>::remove): Deal correctly with &__tmp->_M_value == &__val. * testsuite/23_containers/forward_list/operations/remove_freed.cc: New. Index: include/bits/hashtable.h =================================================================== --- include/bits/hashtable.h (revision 163100) +++ include/bits/hashtable.h (working copy) @@ -1079,7 +1079,8 @@ // _GLIBCXX_RESOLVE_LIB_DEFECTS // 526. Is it undefined if a function in the standard changes // in parameters? - if (&this->_M_extract((*__slot)->_M_v) != &__k) + if (std::__addressof(this->_M_extract((*__slot)->_M_v)) + != std::__addressof(__k)) { _Node* __p = *__slot; *__slot = __p->_M_next; Index: include/bits/forward_list.tcc =================================================================== --- include/bits/forward_list.tcc (revision 163100) +++ include/bits/forward_list.tcc (working copy) @@ -286,13 +286,26 @@ remove(const _Tp& __val) { _Node* __curr = static_cast<_Node*>(&this->_M_impl._M_head); - while (_Node* __temp = static_cast<_Node*>(__curr->_M_next)) + _Node* __extra = 0; + + while (_Node* __tmp = static_cast<_Node*>(__curr->_M_next)) { - if (__temp->_M_value == __val) - this->_M_erase_after(__curr); - else - __curr = static_cast<_Node*>(__curr->_M_next); + if (__tmp->_M_value == __val) + { + if (std::__addressof(__tmp->_M_value) + != std::__addressof(__val)) + { + this->_M_erase_after(__curr); + continue; + } + else + __extra = __curr; + } + __curr = static_cast<_Node*>(__curr->_M_next); } + + if (__extra) + this->_M_erase_after(__extra); } template @@ -302,9 +315,9 @@ remove_if(_Pred __pred) { _Node* __curr = static_cast<_Node*>(&this->_M_impl._M_head); - while (_Node* __temp = static_cast<_Node*>(__curr->_M_next)) + while (_Node* __tmp = static_cast<_Node*>(__curr->_M_next)) { - if (__pred(__temp->_M_value)) + if (__pred(__tmp->_M_value)) this->_M_erase_after(__curr); else __curr = static_cast<_Node*>(__curr->_M_next); Index: testsuite/23_containers/forward_list/operations/remove_freed.cc =================================================================== --- testsuite/23_containers/forward_list/operations/remove_freed.cc (revision 0) +++ testsuite/23_containers/forward_list/operations/remove_freed.cc (revision 0) @@ -0,0 +1,94 @@ +// { dg-options "-std=gnu++0x" } + +// 2010-08-11 Paolo Carlini +// +// Copyright (C) 2010 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 + +// 23.3.3.5 forward_list operations [forwardlist.ops] + +// Used to cause many Valgrind errors: LWG 526-type situation. +int test01() +{ + bool test __attribute__((unused)) = true; + + std::forward_list fl1; + + fl1.push_front(1); + fl1.push_front(2); + fl1.push_front(3); + fl1.push_front(4); + fl1.push_front(1); + + fl1.remove(*fl1.begin()); + + VERIFY( std::distance(fl1.begin(), fl1.end()) == 3 ); + + auto it1 = fl1.begin(); + + VERIFY( *it1 == 4 ); + ++it1; + VERIFY( *it1 == 3 ); + ++it1; + VERIFY( *it1 == 2 ); + + std::forward_list fl2; + + fl2.push_front(3); + fl2.push_front(3); + fl2.push_front(3); + fl2.push_front(3); + fl2.push_front(3); + + auto it2 = fl2.begin(); + ++it2; + ++it2; + + fl2.remove(*it2); + + VERIFY( std::distance(fl2.begin(), fl2.end()) == 0 ); + + std::forward_list fl3; + + fl3.push_front(1); + fl3.push_front(2); + fl3.push_front(3); + fl3.push_front(3); + fl3.push_front(3); + + auto it3 = fl3.begin(); + ++it3; + ++it3; + + fl3.remove(*it3); + + VERIFY( std::distance(fl3.begin(), fl3.end()) == 2 ); + + it3 = fl3.begin(); + VERIFY( *it3 == 2 ); + ++it3; + VERIFY( *it3 == 1 ); +} + +int main() +{ + test01(); + return 0; +}