re PR libstdc++/51866 ([c++0x][4.7 Regression] unordered_multiset compares moved-out values)

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.

Co-Authored-By: Roman Kononov <roman@binarylife.net>

From-SVN: r183285

libstdc++-v3/ChangeLog

@@ -1,3 +1,12 @@
+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.
+
 2012-01-17  Benjamin Kosnik  <bkoz@redhat.com>
 
 	* doc/html/*: Regenerate.

libstdc++-v3/include/bits/hashtable.h

@@ -1227,7 +1227,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      = 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 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  = _M_rehash_policy._M_need_rehash(_M_bucket_count,
 					    _M_element_count, 1);
 
-	const key_type& __k = this->_M_extract()(__v);
+	// 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(__k);
+	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.
+	    // Third, find the node before an equivalent one.
-	    size_type __n = _M_bucket_index(__k, __code);
+	    size_type __bkt = _M_bucket_index(__new_node);
-	    _BaseNode* __prev = _M_find_before_node(__n, __k, __code);
+	    _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 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      // 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);
 	  }

libstdc++-v3/testsuite/23_containers/unordered_multimap/insert/51866.cc

@@ -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_called_on_moved_instance;
+  bool operator()(num const& a, num const& b) const
+  {
+    if (a.value == -1 || b.value == -1)
+      _S_called_on_moved_instance = true;
+    return a.value == b.value;
+  }
+};
+
+bool num_equal::_S_called_on_moved_instance = 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_called_on_moved_instance );
+}
+  
+int main()
+{
+  test01();
+  return 0;
+}

libstdc++-v3/testsuite/23_containers/unordered_multiset/insert/51866.cc

@@ -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_called_on_moved_instance;
+  bool operator()(num const& a, num const& b) const
+  {
+    if (a.value == -1 || b.value == -1)
+      _S_called_on_moved_instance = true;
+    return a.value == b.value;
+  }
+};
+
+bool num_equal::_S_called_on_moved_instance = 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_called_on_moved_instance );
+}
+  
+int main()
+{
+  test01();
+  return 0;
+}