Re: libstdc++/51866 too, sorry

2012-01-18 Thread François Dumont

Attached patch applied.

2012-01-18  François Dumont fdum...@gcc.gnu.org
Roman Kononov ro...@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.




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_multimapnum, int, num_hash, num_equal mmap;
+  

Re: libstdc++/51866 too, sorry

2012-01-18 Thread Paolo Carlini

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.