Patchwork [google-4.6] Backport r183875 to fix incorrect increment/decrement of atomic pointers (issue6428056)

login
register
mail settings
Submitter Jing Yu
Date July 19, 2012, 10:11 p.m.
Message ID <20120719221133.3CE31600E1@jingyu.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/172093/
State New
Headers show

Comments

Jing Yu - July 19, 2012, 10:11 p.m.
Backport r183875 from trunk and gcc-4.7 to fix PR51811 ([C++0x] Incorrect increment/decrement of atomic pointers).

Tested:
1) --testers=crosstool.
2) unit test in Google ref b/6702865

OK for google-4_6 branch?

Thanks,
Jing


2012-07-19  Jing Yu  <jingyu@google.com>

	Backport r183875 to fix wrong atomic<_Tp*> add_fetch.
	PR libstdc++/51811, Google ref b/6702865
	* include/bits/atomic_0.h (atomic<_Tp*>): Fix offsets.
	* include/bits/atomic_2.h: Likewise.
	* testsuite/29_atomics/atomic/operators/51811.cc: New.
	* testsuite/29_atomics/atomic/operators/pointer_partial_void.cc: New.


--
This patch is available for review at http://codereview.appspot.com/6428056

Patch

Index: include/bits/atomic_0.h
===================================================================
--- include/bits/atomic_0.h	(revision 189589)
+++ include/bits/atomic_0.h	(working copy)
@@ -620,7 +620,7 @@  namespace __atomic0
       __return_pointer_type
       fetch_add(ptrdiff_t __d, memory_order __m = memory_order_seq_cst)
       {
-	void* __v = _ATOMIC_MODIFY_(this, +=, __d, __m);
+	void* __v = _ATOMIC_MODIFY_(this, +=, __d * sizeof(_PTp), __m);
 	return reinterpret_cast<__return_pointer_type>(__v);
       }
 
@@ -628,14 +628,14 @@  namespace __atomic0
       fetch_add(ptrdiff_t __d,
 		memory_order __m = memory_order_seq_cst) volatile
       {
-	void* __v = _ATOMIC_MODIFY_(this, +=, __d, __m);
+	void* __v = _ATOMIC_MODIFY_(this, +=, __d * sizeof(_PTp), __m);
 	return reinterpret_cast<__return_pointer_type>(__v);
       }
 
       __return_pointer_type
       fetch_sub(ptrdiff_t __d, memory_order __m = memory_order_seq_cst)
       {
-	void* __v = _ATOMIC_MODIFY_(this, -=, __d, __m);
+	void* __v = _ATOMIC_MODIFY_(this, -=, __d * sizeof(_PTp), __m);
 	return reinterpret_cast<__return_pointer_type>(__v);
       }
 
@@ -643,7 +643,7 @@  namespace __atomic0
       fetch_sub(ptrdiff_t __d,
 		memory_order __m = memory_order_seq_cst) volatile
       {
-	void* __v = _ATOMIC_MODIFY_(this, -=, __d, __m);
+	void* __v = _ATOMIC_MODIFY_(this, -=, __d * sizeof(_PTp), __m);
 	return reinterpret_cast<__return_pointer_type>(__v);
       }
     };
Index: include/bits/atomic_2.h
===================================================================
--- include/bits/atomic_2.h	(revision 189589)
+++ include/bits/atomic_2.h	(working copy)
@@ -644,21 +644,21 @@  namespace __atomic2
 
       __pointer_type
       fetch_add(ptrdiff_t __d, memory_order __m = memory_order_seq_cst)
-      { return __sync_fetch_and_add(&_M_p, __d); }
+      { return __sync_fetch_and_add(&_M_p, __d * sizeof(_PTp)); }
 
       __pointer_type
       fetch_add(ptrdiff_t __d,
 		memory_order __m = memory_order_seq_cst) volatile
-      { return __sync_fetch_and_add(&_M_p, __d); }
+      { return __sync_fetch_and_add(&_M_p, __d * sizeof(_PTp)); }
 
       __pointer_type
       fetch_sub(ptrdiff_t __d, memory_order __m = memory_order_seq_cst)
-      { return __sync_fetch_and_sub(&_M_p, __d); }
+      { return __sync_fetch_and_sub(&_M_p, __d * sizeof(_PTp)); }
 
       __pointer_type
       fetch_sub(ptrdiff_t __d,
 		memory_order __m = memory_order_seq_cst) volatile
-      { return __sync_fetch_and_sub(&_M_p, __d); }
+      { return __sync_fetch_and_sub(&_M_p, __d * sizeof(_PTp)); }
     };
 
 } // namespace __atomic2
Index: testsuite/29_atomics/atomic/operators/pointer_partial_void.cc
===================================================================
--- testsuite/29_atomics/atomic/operators/pointer_partial_void.cc	(revision 0)
+++ testsuite/29_atomics/atomic/operators/pointer_partial_void.cc	(revision 0)
@@ -0,0 +1,71 @@ 
+// { dg-require-atomic-builtins "" }
+// { 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 <atomic>
+#include <cstdlib> //std::abs
+#include <testsuite_hooks.h>
+
+// pointer arithimetic vs. atomic<void*>.
+// atomic<void*> vs. explicitly specialized w/o operators, like atomic_bool?
+int main(void)
+{
+  // bool test __attribute__((unused)) = true;
+
+  using namespace std;
+
+  typedef int 	value_type;
+  const size_t n = 2;
+  value_type value = 42;
+  value_type* p = &value;
+  void* vp = p;
+  ptrdiff_t dist(0);
+
+  atomic<void*> a(vp);
+
+  // operator++
+  void* vp2(a);
+  a++;
+  void* vp3(a);
+  dist = reinterpret_cast<char*>(vp2) - reinterpret_cast<char*>(vp3);
+  // VERIFY ( std::abs(dist) == sizeof(void*));
+
+  // operator--
+  void* vp4(a);
+  a--;
+  void* vp5(a);
+  dist = reinterpret_cast<char*>(vp4) - reinterpret_cast<char*>(vp5);
+  // VERIFY ( std::abs(dist) == sizeof(void*));
+
+  // operator+=
+  void* vp6(a);
+  a+=n;
+  void* vp7(a);
+  dist = reinterpret_cast<char*>(vp6) - reinterpret_cast<char*>(vp7);
+  // VERIFY ( std::abs(dist) == sizeof(void*) * n);
+
+  // operator-=
+  void* vp8(a);
+  a-=n;
+  void* vp9(a);
+  dist = reinterpret_cast<char*>(vp8) - reinterpret_cast<char*>(vp9);
+  //VERIFY ( std::abs(dist) == sizeof(void*) * n);
+
+  return 0;
+}
Index: testsuite/29_atomics/atomic/operators/51811.cc
===================================================================
--- testsuite/29_atomics/atomic/operators/51811.cc	(revision 0)
+++ testsuite/29_atomics/atomic/operators/51811.cc	(revision 0)
@@ -0,0 +1,90 @@ 
+// { 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 <atomic>
+#include <cstdlib> //std::abs
+#include <testsuite_hooks.h>
+
+// libstdc++/51811
+// pointer arithimetic vs. atomic<_Tp*> specialization
+int main(void)
+{
+  bool test __attribute__((unused)) = true;
+
+  using namespace std;
+
+  typedef int 	value_type;
+  const size_t n = 2;
+  value_type value = 42;
+  atomic<value_type*> p, p2, p3;
+
+  // operator++
+  {
+    p = &value;
+    p2 = p++;
+    VERIFY (p != p2);
+    
+    value_type* vp(p);
+    value_type* vp2(p2);
+    ptrdiff_t dist = reinterpret_cast<char*>(vp) - reinterpret_cast<char*>(vp2);
+    VERIFY ( std::abs(dist) == sizeof(value_type));
+  
+    p = &value;
+    p3 = ++p;
+    VERIFY (p == p3);
+  }
+
+  // operator--
+  {
+    p = &value;
+    p2 = p--;
+    VERIFY (p != p2);
+
+    value_type* vp(p);
+    value_type* vp2(p2);
+    ptrdiff_t dist = reinterpret_cast<char*>(vp) - reinterpret_cast<char*>(vp2);
+    VERIFY ( std::abs(dist) == sizeof(value_type));
+
+    p = &value;
+    p3 = --p;
+    VERIFY (p == p3);
+  }
+
+  // operator+=
+  {
+    p = &value;
+    value_type* vp(p);
+    p+=n;
+    value_type* vp2(p);
+    ptrdiff_t dist = reinterpret_cast<char*>(vp) - reinterpret_cast<char*>(vp2);
+    VERIFY ( std::abs(dist) == sizeof(value_type) * n);
+  }
+
+  // operator-=
+  {
+    p = &value;
+    value_type* vp(p);
+    p-=n;
+    value_type* vp2(p);
+    ptrdiff_t dist = reinterpret_cast<char*>(vp) - reinterpret_cast<char*>(vp2);
+    VERIFY ( std::abs(dist) == sizeof(value_type) * n);
+  }
+
+  return 0;
+}