This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Prevent double free in basic_string


Actually, for number of threads >=3 this patch is better:
--- old/libstdc++-v3/include/bits/basic_string.h	2012-05-30 17:03:15.974035000 -0400
+++ new/libstdc++-v3/include/bits/basic_string.h	2012-05-31 13:06:39.903442000 -0400
@@ -224,14 +224,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_M_refdata() throw()
 	{ return reinterpret_cast<_CharT*>(this + 1); }
 
 	_CharT*
 	_M_grab(const _Alloc& __alloc1, const _Alloc& __alloc2)
 	{
-	  return (!_M_is_leaked() && __alloc1 == __alloc2)
-	          ? _M_refcopy() : _M_clone(__alloc1);
+          _CharT* res = (_CharT*)0;
+	  if(!_M_is_leaked() && __alloc1 == __alloc2) {
+            res = _M_refcopy();
+          }
+          if (res == (_CharT*)0) {
+            res = _M_clone(__alloc1);
+          }
+          return res;
 	}
 
 	// Create & Destroy
 	static _Rep*
 	_S_create(size_type, size_type, const _Alloc&);
 
@@ -259,13 +265,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_CharT*
 	_M_refcopy() throw()
 	{
 #if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
 	  if (__builtin_expect(this != &_S_empty_rep(), false))
 #endif
-            __gnu_cxx::__atomic_add_dispatch(&this->_M_refcount, 1);
+          {
+            // Here we try to detect and compensate an unexpected decrement of
+            // _M_refcount from another thread. In the properly designed code
+            // this should not happen ever! If we do nothing about this
+            // condition then the string is going to be freed twice.
+            // Unfortunatelly, some cases that lead to this condition are not
+            // obvious for the developers, e.g. non-constant
+            // methods operator [], at(), begin, end, etc.
+            // In many of these cases the string is not going to be
+            // changed from another thread and it's actually safe to
+            // clone the string.
+            if (__gnu_cxx::__exchange_and_add_dispatch(&this->_M_refcount, 1) < 0) {
+              __gnu_cxx::__atomic_add_dispatch(&this->_M_refcount, -1);
+              return (_CharT*)0;
+            }
+          }
 	  return _M_refdata();
 	}  // XXX MT
 
 	_CharT*
 	_M_clone(const _Alloc&, size_type __res = 0);
       };

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]