re PR libstdc++/62313 (Data race in debug iterators)

2014-09-29  François Dumont  <fdumont@gcc.gnu.org>

	PR libstdc++/62313
	* include/debug/safe_base.h
	(_Safe_iterator_base(const _Safe_iterator_base&)): Delete declaration.
	(_Safe_iterator_base& operator=(const _Safe_iterator_base&)): Likewise.
	* include/debug/safe_iterator.h (_Safe_iterator<>): Move normal iterator
	before _Safe_iterator_base in memory. Lock before modifying the iterator
	in numerous places.
	* include/debug/safe_local_iterator.h
	(_Safe_local_iterator_base(const _Safe_local_iterator_base&)): Delete
	declaration.
	(_Safe_local_iterator_base& operator=(const _Safe_local_iterator_base&)):
	Likewise.
	* include/debug/safe_unordered_base.h (_Safe_local_iterator<>):  Move
	normal iterator before _Safe_iterator_base in memory. Lock before
	modifying the iterator in numerous places.
	* include/debug/forward_list (_Safe_forward_list<>::_M_swap_aux): Adapt.
	* include/debug/safe_sequence.tcc
	(_Safe_sequence<>::_M_transfer_from_if): Adapt.

From-SVN: r215693

libstdc++-v3/ChangeLog

+2014-09-29  François Dumont  <fdumont@gcc.gnu.org>
+
+	PR libstdc++/62313
+	* include/debug/safe_base.h
+	(_Safe_iterator_base(const _Safe_iterator_base&)): Delete declaration.
+	(_Safe_iterator_base& operator=(const _Safe_iterator_base&)): Likewise.
+	* include/debug/safe_iterator.h (_Safe_iterator<>): Move normal iterator
+	before _Safe_iterator_base in memory. Lock before modifying the iterator
+	in numerous places.
+	* include/debug/safe_local_iterator.h
+	(_Safe_local_iterator_base(const _Safe_local_iterator_base&)): Delete
+	declaration.
+	(_Safe_local_iterator_base& operator=(const _Safe_local_iterator_base&)):
+	Likewise.
+	* include/debug/safe_unordered_base.h (_Safe_local_iterator<>):  Move
+	normal iterator before _Safe_iterator_base in memory. Lock before
+	modifying the iterator in numerous places.
+	* include/debug/forward_list (_Safe_forward_list<>::_M_swap_aux): Adapt.
+	* include/debug/safe_sequence.tcc
+	(_Safe_sequence<>::_M_transfer_from_if): Adapt.
+
 2014-09-25  Jonathan Wakely  <jwakely@redhat.com>
 
 	DR 1339

libstdc++-v3/include/debug/forward_list

@@ -86,24 +86,26 @@ namespace __gnu_debug
       for (_Safe_iterator_base* __iter = __lhs_iterators; __iter;)
 	{
 	  // Even iterator is cast to const_iterator, not a problem.
-	  const_iterator* __victim = static_cast<const_iterator*>(__iter);
+	  _Safe_iterator_base* __victim_base = __iter;
+	  const_iterator* __victim =
+	    static_cast<const_iterator*>(__victim_base);
 	  __iter = __iter->_M_next;
 	  if (__victim->base() == __rseq._M_base().cbefore_begin())
 	    {
 	      __victim->_M_unlink();
-	      if (__lhs_iterators == __victim)
-		__lhs_iterators = __victim->_M_next;
+	      if (__lhs_iterators == __victim_base)
+		__lhs_iterators = __victim_base->_M_next;
 	      if (__bbegin_its)
 		{
-		  __victim->_M_next = __bbegin_its;
-		  __bbegin_its->_M_prior = __victim;
+		  __victim_base->_M_next = __bbegin_its;
+		  __bbegin_its->_M_prior = __victim_base;
 		}
 	      else
-		__last_bbegin = __victim;
-	      __bbegin_its = __victim;
+		__last_bbegin = __victim_base;
+	      __bbegin_its = __victim_base;
 	    }
 	  else
-	    __victim->_M_sequence = &__lhs;
+	    __victim_base->_M_sequence = &__lhs;
 	}
 
       if (__bbegin_its)

libstdc++-v3/include/debug/safe_base.h

@@ -95,12 +95,6 @@ namespace __gnu_debug
     : _M_sequence(0), _M_version(0), _M_prior(0), _M_next(0)
     { this->_M_attach(__x._M_sequence, __constant); }
 
-    _Safe_iterator_base&
-    operator=(const _Safe_iterator_base&);
-
-    explicit
-    _Safe_iterator_base(const _Safe_iterator_base&);
-
     ~_Safe_iterator_base() { this->_M_detach(); }
 
     /** For use in _Safe_iterator. */

libstdc++-v3/include/debug/safe_local_iterator.h

@@ -49,15 +49,15 @@ namespace __gnu_debug
    *  the iterator's state.
    */
   template<typename _Iterator, typename _Sequence>
-    class _Safe_local_iterator : public _Safe_local_iterator_base
+    class _Safe_local_iterator
+    : private _Iterator
+    , public _Safe_local_iterator_base
     {
-      typedef _Safe_local_iterator _Self;
+      typedef _Iterator _Iter_base;
+      typedef _Safe_local_iterator_base _Safe_base;
       typedef typename _Sequence::const_local_iterator _Const_local_iterator;
       typedef typename _Sequence::size_type size_type;
 
-      /// The underlying iterator
-      _Iterator _M_current;
-
       /// Determine if this is a constant iterator.
       bool
       _M_constant() const
@@ -68,6 +68,14 @@ namespace __gnu_debug
 
       typedef std::iterator_traits<_Iterator> _Traits;
 
+      struct _Attach_single
+      { };
+
+      _Safe_local_iterator(const _Iterator& __i, _Safe_sequence_base* __cont,
+			   _Attach_single) noexcept
+      : _Iter_base(__i)
+      { _M_attach_single(__cont); }
+
     public:
       typedef _Iterator					iterator_type;
       typedef typename _Traits::iterator_category	iterator_category;
@@ -77,7 +85,7 @@ namespace __gnu_debug
       typedef typename _Traits::pointer			pointer;
 
       /// @post the iterator is singular and unattached
-      _Safe_local_iterator() : _M_current() { }
+      _Safe_local_iterator() noexcept : _Iter_base() { }
 
       /**
        * @brief Safe iterator construction from an unsafe iterator and
@@ -86,8 +94,9 @@ namespace __gnu_debug
        * @pre @p seq is not NULL
        * @post this is not singular
        */
-      _Safe_local_iterator(const _Iterator& __i, const _Sequence* __seq)
-      : _Safe_local_iterator_base(__seq, _M_constant()), _M_current(__i)
+      _Safe_local_iterator(const _Iterator& __i,
+			   const _Safe_sequence_base* __cont)
+      : _Iter_base(__i), _Safe_base(__cont, _M_constant())
       {
 	_GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
 			      _M_message(__msg_init_singular)
@@ -97,9 +106,8 @@ namespace __gnu_debug
       /**
        * @brief Copy construction.
        */
-      _Safe_local_iterator(const _Safe_local_iterator& __x)
-      : _Safe_local_iterator_base(__x, _M_constant()),
-	_M_current(__x._M_current)
+      _Safe_local_iterator(const _Safe_local_iterator& __x) noexcept
+      : _Iter_base(__x.base())
       {
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
@@ -108,6 +116,25 @@ namespace __gnu_debug
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
+	_M_attach(__x._M_sequence);
+      }
+
+      /**
+       * @brief Move construction.
+       * @post __x is singular and unattached
+       */
+      _Safe_local_iterator(_Safe_local_iterator&& __x) noexcept
+      : _Iter_base()
+      {
+	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
+			      || __x.base() == _Iterator(),
+			      _M_message(__msg_init_copy_singular)
+			      ._M_iterator(*this, "this")
+			      ._M_iterator(__x, "other"));
+	auto __cont = __x._M_sequence;
+	__x._M_detach();
+	std::swap(base(), __x.base());
+	_M_attach(__cont);
       }
 
       /**
@@ -121,8 +148,7 @@ namespace __gnu_debug
 	      _MutableIterator,
 	      typename _Sequence::local_iterator::iterator_type>::__value,
 					  _Sequence>::__type>& __x)
-	: _Safe_local_iterator_base(__x, _M_constant()),
-	  _M_current(__x.base())
+	: _Iter_base(__x.base())
 	{
 	  // _GLIBCXX_RESOLVE_LIB_DEFECTS
 	  // DR 408. Is vector<reverse_iterator<char*> > forbidden?
@@ -131,6 +157,7 @@ namespace __gnu_debug
 				_M_message(__msg_init_const_singular)
 				._M_iterator(*this, "this")
 				._M_iterator(__x, "other"));
+	  _M_attach(__x._M_sequence);
 	}
 
       /**
@@ -146,8 +173,54 @@ namespace __gnu_debug
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
-	_M_current = __x._M_current;
-	this->_M_attach(__x._M_sequence);
+
+	if (this->_M_sequence && this->_M_sequence == __x._M_sequence)
+	  {
+	    __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
+	    base() = __x.base();
+	    _M_version = __x._M_sequence->_M_version;
+	  }
+	else
+	  {
+	    _M_detach();
+	    base() = __x.base();
+	    _M_attach(__x._M_sequence);
+	  }
+
+	return *this;
+      }
+
+      /**
+       * @brief Move assignment.
+       * @post __x is singular and unattached
+       */
+      _Safe_local_iterator&
+      operator=(_Safe_local_iterator&& __x) noexcept
+      {
+	_GLIBCXX_DEBUG_VERIFY(this != &__x,
+			      _M_message(__msg_self_move_assign)
+			      ._M_iterator(*this, "this"));
+	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
+			      || __x.base() == _Iterator(),
+			      _M_message(__msg_copy_singular)
+			      ._M_iterator(*this, "this")
+			      ._M_iterator(__x, "other"));
+
+	if (this->_M_sequence && this->_M_sequence == __x._M_sequence)
+	  {
+	    __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
+	    base() = __x.base();
+	    _M_version = __x._M_sequence->_M_version;
+	  }
+	else
+	  {
+	    _M_detach();
+	    base() = __x.base();
+	    _M_attach(__x._M_sequence);
+	  }
+
+	__x._M_detach();
+	__x.base() = _Iterator();
 	return *this;
       }
 
@@ -161,7 +234,7 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_dereferenceable(),
 			      _M_message(__msg_bad_deref)
 			      ._M_iterator(*this, "this"));
-	return *_M_current;
+	return *base();
       }
 
       /**
@@ -175,7 +248,7 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_dereferenceable(),
 			      _M_message(__msg_bad_deref)
 			      ._M_iterator(*this, "this"));
-	return std::__addressof(*_M_current);
+	return std::__addressof(*base());
       }
 
       // ------ Input iterator requirements ------
@@ -189,7 +262,8 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_incrementable(),
 			      _M_message(__msg_bad_inc)
 			      ._M_iterator(*this, "this"));
-	++_M_current;
+	__gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
+	++base();
 	return *this;
       }
 
@@ -203,39 +277,42 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_incrementable(),
 			      _M_message(__msg_bad_inc)
 			      ._M_iterator(*this, "this"));
-	_Safe_local_iterator __tmp(*this);
-	++_M_current;
-	return __tmp;
+	__gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
+	return _Safe_local_iterator(base()++, this->_M_sequence,
+				    _Attach_single());
       }
 
       // ------ Utilities ------
       /**
        * @brief Return the underlying iterator
        */
-      _Iterator
-      base() const { return _M_current; }
+      _Iterator&
+      base() noexcept { return *this; }
+
+      const _Iterator&
+      base() const noexcept { return *this; }
 
       /**
        * @brief Return the bucket
        */
       size_type
-      bucket() const { return _M_current._M_get_bucket(); }
+      bucket() const { return base()._M_get_bucket(); }
 
       /**
        * @brief Conversion to underlying non-debug iterator to allow
        * better interaction with non-debug containers.
        */
-      operator _Iterator() const { return _M_current; }
+      operator _Iterator() const { return *this; }
 
       /** Attach iterator to the given sequence. */
       void
       _M_attach(_Safe_sequence_base* __seq)
-      { _Safe_iterator_base::_M_attach(__seq, _M_constant()); }
+      { _Safe_base::_M_attach(__seq, _M_constant()); }
 
       /** Likewise, but not thread-safe. */
       void
       _M_attach_single(_Safe_sequence_base* __seq)
-      { _Safe_iterator_base::_M_attach_single(__seq, _M_constant()); }
+      { _Safe_base::_M_attach_single(__seq, _M_constant()); }
 
       /// Is the iterator dereferenceable?
       bool

libstdc++-v3/include/debug/safe_sequence.tcc

@@ -81,42 +81,45 @@ namespace __gnu_debug
 
 	  for (_Safe_iterator_base* __iter = __from._M_iterators; __iter;)
 	    {
-	      iterator* __victim = static_cast<iterator*>(__iter);
+	      _Safe_iterator_base* __victim_base = __iter;
+	      iterator* __victim = static_cast<iterator*>(__victim_base);
 	      __iter = __iter->_M_next;
 	      if (!__victim->_M_singular() && __pred(__victim->base()))
 		{
 		  __victim->_M_detach_single();
 		  if (__transfered_iterators)
 		    {
-		      __victim->_M_next = __transfered_iterators;
-		      __transfered_iterators->_M_prior = __victim;
+		      __victim_base->_M_next = __transfered_iterators;
+		      __transfered_iterators->_M_prior = __victim_base;
 		    }
 		  else
-		    __last_iterator = __victim;
-		  __victim->_M_sequence = this;
-		  __victim->_M_version = this->_M_version;
-		  __transfered_iterators = __victim;
+		    __last_iterator = __victim_base;
+		  __victim_base->_M_sequence = this;
+		  __victim_base->_M_version = this->_M_version;
+		  __transfered_iterators = __victim_base;
 		}
 	    }
 
 	  for (_Safe_iterator_base* __iter2 = __from._M_const_iterators;
 		 __iter2;)
 	    {
-	      const_iterator* __victim = static_cast<const_iterator*>(__iter2);
+	      _Safe_iterator_base* __victim_base = __iter2;
+	      const_iterator* __victim =
+		static_cast<const_iterator*>(__victim_base);
 	      __iter2 = __iter2->_M_next;
 	      if (!__victim->_M_singular() && __pred(__victim->base()))
 		{
 		  __victim->_M_detach_single();
 		  if (__transfered_const_iterators)
 		    {
-		      __victim->_M_next = __transfered_const_iterators;
-		      __transfered_const_iterators->_M_prior = __victim;
+		      __victim_base->_M_next = __transfered_const_iterators;
+		      __transfered_const_iterators->_M_prior = __victim_base;
 		    }
 		  else
 		    __last_const_iterator = __victim;
-		  __victim->_M_sequence = this;
-		  __victim->_M_version = this->_M_version;
-		  __transfered_const_iterators = __victim;
+		  __victim_base->_M_sequence = this;
+		  __victim_base->_M_version = this->_M_version;
+		  __transfered_const_iterators = __victim_base;
 		}
 	    }
 	}

libstdc++-v3/include/debug/safe_unordered_base.h

@@ -71,12 +71,6 @@ namespace __gnu_debug
 			      bool __constant)
     { this->_M_attach(__x._M_sequence, __constant); }
 
-    _Safe_local_iterator_base&
-    operator=(const _Safe_local_iterator_base&);
-
-    explicit
-    _Safe_local_iterator_base(const _Safe_local_iterator_base&);
-
     ~_Safe_local_iterator_base() { this->_M_detach(); }
 
     _Safe_unordered_container_base*