diff mbox

libstdc++/51866 too, sorry

Message ID 4F172B3E.4030600@gmail.com
State New
Headers show

Commit Message

François Dumont Jan. 18, 2012, 8:27 p.m. UTC
Attached patch applied.

2012-01-18  François Dumont <fdumont@gcc.gnu.org>
             Roman Kononov <roman@binarylife.net>

         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.
>
>

Comments

Paolo Carlini Jan. 18, 2012, 8:28 p.m. UTC | #1
On 01/18/2012 09:27 PM, François Dumont wrote:
> Attached patch applied.
Thanks. Apparently 51845 is an unrelated issue, Jakub has an analysis 
and a candidate fix. Can you please look into that?

Thanks again,
Paolo.
diff mbox

Patch

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
+// <http://www.gnu.org/licenses/>.
+
+#include <unordered_map>
+#include <testsuite_hooks.h>
+
+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<num, int, num_hash, num_equal> 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
+// <http://www.gnu.org/licenses/>.
+
+#include <unordered_set>
+#include <testsuite_hooks.h>
+
+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<num, num_hash, num_equal> 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;
+}