[PATCH] Protect more algorithms from overloaded comma operators

Jonathan Wakely jwakely@redhat.com
Thu Oct 26 23:53:00 GMT 2017


LWG 2133 makes it a requirement for some of the algorithms to protect
against overlaoded comma operators. This does so, and adapts some
tests to verify it, using our iterator wrappers. Those wrappers have a
member oeprator, but that only handles the case where the iterator is
the left operand, so I added extra deleted (non-member) functions to
handle iterators as the right operand.

	* include/bits/stl_algo.h (__find_if_not_n, generate_n): Cast to void
	to ensure overloaded comma not used.
	* include/bits/stl_algobase.h (__fill_n_a, equal): Likewise.
	* include/bits/stl_uninitialized.h (__uninitialized_fill_n)
	(__uninitialized_fill_n_a, __uninitialized_default_n_1)
	(__uninitialized_default_n_a, __uninitialized_copy_n)
	(__uninitialized_copy_n_pair): Likewise
	* testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc:
	Use test iterator wrappers with overloaded comma operator.
	* testsuite/25_algorithms/fill_n/1.cc: Likewise.
	* testsuite/25_algorithms/generate_n/1.cc: New test.
	* testsuite/25_algorithms/stable_partition/1.cc: New test.
	* testsuite/util/testsuite_iterators.h (operator,): Add deleted
	non-member comma operator with iterator wrappers as right operand.

Tested powerpc64le-linux, committed to trunk.

-------------- next part --------------
commit f358d44717b3fa3f204a88fff098d03e4b5890c1
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 26 18:41:41 2017 +0100

    Protect more algorithms from overloaded comma operators
    
            * include/bits/stl_algo.h (__find_if_not_n, generate_n): Cast to void
            to ensure overloaded comma not used.
            * include/bits/stl_algobase.h (__fill_n_a, equal): Likewise.
            * include/bits/stl_uninitialized.h (__uninitialized_fill_n)
            (__uninitialized_fill_n_a, __uninitialized_default_n_1)
            (__uninitialized_default_n_a, __uninitialized_copy_n)
            (__uninitialized_copy_n_pair): Likewise
            * testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc:
            Use test iterator wrappers with overloaded comma operator.
            * testsuite/25_algorithms/fill_n/1.cc: Likewise.
            * testsuite/25_algorithms/generate_n/1.cc: New test.
            * testsuite/25_algorithms/stable_partition/1.cc: New test.
            * testsuite/util/testsuite_iterators.h (operator,): Add deleted
            non-member comma operator with iterator wrappers as right operand.

diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
index ea413b1b155..b5facef55dd 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -180,7 +180,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     _InputIterator
     __find_if_not_n(_InputIterator __first, _Distance& __len, _Predicate __pred)
     {
-      for (; __len; --__len, ++__first)
+      for (; __len; --__len,  (void) ++__first)
 	if (!__pred(__first))
 	  break;
       return __first;
@@ -4462,7 +4462,7 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
 	    __typeof__(__gen())>)
 
       for (__decltype(__n + 0) __niter = __n;
-	   __niter > 0; --__niter, ++__first)
+	   __niter > 0; --__niter, (void) ++__first)
 	*__first = __gen();
       return __first;
     }
diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index a80934c4faa..bfdfc7ded5e 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -738,7 +738,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     __fill_n_a(_OutputIterator __first, _Size __n, const _Tp& __value)
     {
       for (__decltype(__n + 0) __niter = __n;
-	   __niter > 0; --__niter, ++__first)
+	   __niter > 0; --__niter, (void) ++__first)
 	*__first = __value;
       return __first;
     }
@@ -750,7 +750,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       const _Tp __tmp = __value;
       for (__decltype(__n + 0) __niter = __n;
-	   __niter > 0; --__niter, ++__first)
+	   __niter > 0; --__niter, (void) ++__first)
 	*__first = __tmp;
       return __first;
     }
@@ -796,7 +796,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	static bool
 	equal(_II1 __first1, _II1 __last1, _II2 __first2)
 	{
-	  for (; __first1 != __last1; ++__first1, (void)++__first2)
+	  for (; __first1 != __last1; ++__first1, (void) ++__first2)
 	    if (!(*__first1 == *__first2))
 	      return false;
 	  return true;
diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h b/libstdc++-v3/include/bits/stl_uninitialized.h
index c2ba863ed98..bac3fd01f7a 100644
--- a/libstdc++-v3/include/bits/stl_uninitialized.h
+++ b/libstdc++-v3/include/bits/stl_uninitialized.h
@@ -206,7 +206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  _ForwardIterator __cur = __first;
 	  __try
 	    {
-	      for (; __n > 0; --__n, ++__cur)
+	      for (; __n > 0; --__n, (void) ++__cur)
 		std::_Construct(std::__addressof(*__cur), __x);
 	      return __cur;
 	    }
@@ -347,7 +347,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __try
 	{
 	  typedef __gnu_cxx::__alloc_traits<_Allocator> __traits;
-	  for (; __n > 0; --__n, ++__cur)
+	  for (; __n > 0; --__n, (void) ++__cur)
 	    __traits::construct(__alloc, std::__addressof(*__cur), __x);
 	  return __cur;
 	}
@@ -523,7 +523,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  _ForwardIterator __cur = __first;
 	  __try
 	    {
-	      for (; __n > 0; --__n, ++__cur)
+	      for (; __n > 0; --__n, (void) ++__cur)
 		std::_Construct(std::__addressof(*__cur));
 	      return __cur;
 	    }
@@ -627,7 +627,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __try
 	{
 	  typedef __gnu_cxx::__alloc_traits<_Allocator> __traits;
-	  for (; __n > 0; --__n, ++__cur)
+	  for (; __n > 0; --__n, (void) ++__cur)
 	    __traits::construct(__alloc, std::__addressof(*__cur));
 	  return __cur;
 	}
@@ -687,7 +687,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  _ForwardIterator __cur = __first;
 	  __try
 	    {
-	      for (; __n > 0; --__n, ++__cur)
+	      for (; __n > 0; --__n, (void) ++__cur)
 		std::_Construct_novalue(std::__addressof(*__cur));
 	      return __cur;
 	    }
@@ -747,7 +747,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _ForwardIterator __cur = __result;
       __try
 	{
-	  for (; __n > 0; --__n, ++__first, ++__cur)
+	  for (; __n > 0; --__n, (void) ++__first, ++__cur)
 	    std::_Construct(std::__addressof(*__cur), *__first);
 	  return __cur;
 	}
@@ -775,7 +775,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _ForwardIterator __cur = __result;
       __try
 	{
-	  for (; __n > 0; --__n, ++__first, ++__cur)
+	  for (; __n > 0; --__n, (void) ++__first, ++__cur)
 	    std::_Construct(std::__addressof(*__cur), *__first);
 	  return {__first, __cur};
 	}
diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc
index 39d3e76ee82..d118dd18ce9 100644
--- a/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc
+++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc
@@ -19,8 +19,8 @@
 
 #include <memory>
 #include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
 #include <string>
-#include <array>
 #include <vector>
 #include <sstream>
 
@@ -56,36 +56,42 @@ struct ThrowAfterN
   DelCount dc;
 };
 
+template<typename T>
+  using FwdIteratorRange
+    = __gnu_test::test_container<T, __gnu_test::forward_iterator_wrapper>;
+
 void test01()
 {
   char test_data[] = "123456";
-  std::uninitialized_default_construct(std::begin(test_data),
-				       std::end(test_data));
+  FwdIteratorRange<char> r(test_data);
+  std::uninitialized_default_construct(std::begin(r), std::end(r));
   VERIFY(std::string(test_data) == "123456");
 }
 
 void test02()
 {
   char test_data[] = "123456";
-  std::uninitialized_value_construct(std::begin(test_data),
-				       std::end(test_data));
+  FwdIteratorRange<char> r(test_data);
+  std::uninitialized_value_construct(std::begin(r), std::end(r));
   VERIFY(std::string(test_data, 6) == std::string("\0\0\0\0\0\0", 6));
 }
 
 void test03()
 {
   char test_data[] = "123456";
-  auto end = std::uninitialized_default_construct_n(std::begin(test_data), 6);
+  FwdIteratorRange<char> r(test_data);
+  auto end = std::uninitialized_default_construct_n(std::begin(r), 6);
   VERIFY(std::string(test_data) == "123456");
-  VERIFY( end == test_data + 6 );
+  VERIFY( end == std::next(r.begin(), 6) );
 }
 
 void test04()
 {
   char test_data[] = "123456";
-  auto end = std::uninitialized_value_construct_n(std::begin(test_data), 5);
+  FwdIteratorRange<char> r(test_data);
+  auto end = std::uninitialized_value_construct_n(std::begin(r), 5);
   VERIFY(std::string(test_data, 6) == std::string("\0\0\0\0\0" "6", 6));
-  VERIFY( end == test_data + 5 );
+  VERIFY( end == std::next(r.begin(), 5) );
 }
 
 void test05()
@@ -122,33 +128,43 @@ void test07()
   free(x);
 }
 
+struct MoveOnly
+{
+  MoveOnly() : val(-1) { }
+  MoveOnly(MoveOnly&& m) : val(m.val) { m.val = -1; }
+  int val;
+};
+
 void test08()
 {
-  std::vector<std::unique_ptr<int>> source;
-  for (int i = 0; i < 10; ++i) source.push_back(std::make_unique<int>(i));
-  std::unique_ptr<int>* target =
-    (std::unique_ptr<int>*)malloc(sizeof(std::unique_ptr<int>)*10);
-  std::uninitialized_move(source.begin(), source.end(), target);
-  for (const auto& x : source) VERIFY(!x);
-  for (int i = 0; i < 10; ++i) VERIFY(bool(*(target+i)));
-  auto end = std::destroy_n(target, 10);
-  VERIFY( end == target + 10 );
+  MoveOnly source[10];
+  for (int i = 0; i < 10; ++i) source[i].val = i;
+  FwdIteratorRange<MoveOnly> src(source);
+  MoveOnly* target = (MoveOnly*)malloc(sizeof(MoveOnly)*10);
+  FwdIteratorRange<MoveOnly> tgt(target, target+10);
+  auto end = std::uninitialized_move(src.begin(), src.end(), tgt.begin());
+  VERIFY( end == std::next(tgt.begin(), 10) );
+  for (const auto& x : source) VERIFY( x.val == -1 );
+  for (int i = 0; i < 10; ++i) VERIFY( target[i].val == i );
+  auto end2 = std::destroy_n(tgt.begin(), 10);
+  VERIFY( end2 == std::next(tgt.begin(), 10) );
   free(target);
 }
 
 void test09()
 {
-  std::vector<std::unique_ptr<int>> source;
-  for (int i = 0; i < 10; ++i) source.push_back(std::make_unique<int>(i));
-  std::unique_ptr<int>* target =
-    (std::unique_ptr<int>*)malloc(sizeof(std::unique_ptr<int>)*10);
-  auto end = std::uninitialized_move_n(source.begin(), 10, target);
-  VERIFY( end.first == source.begin() + 10 );
-  VERIFY( end.second == target + 10 );
-  for (const auto& x : source) VERIFY(!x);
-  for (int i = 0; i < 10; ++i) VERIFY(bool(*(target+i)));
-  auto end2 = std::destroy_n(target, 10);
-  VERIFY( end2 == target + 10 );
+  MoveOnly source[10];
+  for (int i = 0; i < 10; ++i) source[i].val = i;
+  FwdIteratorRange<MoveOnly> src(source);
+  MoveOnly* target = (MoveOnly*)malloc(sizeof(MoveOnly)*10);
+  FwdIteratorRange<MoveOnly> tgt(target, target+10);
+  auto end = std::uninitialized_move_n(src.begin(), 10, tgt.begin());
+  VERIFY( end.first == std::next(src.begin(), 10) );
+  VERIFY( end.second == std::next(tgt.begin(), 10) );
+  for (const auto& x : source) VERIFY( x.val == -1 );
+  for (int i = 0; i < 10; ++i) VERIFY( target[i].val == i );
+  auto end2 = std::destroy_n(tgt.begin(), 10);
+  VERIFY( end2 == std::next(tgt.begin(), 10) );
   free(target);
 }
 
@@ -156,7 +172,8 @@ void test10()
 {
   char* x = (char*)malloc(sizeof(char)*10);
   for (int i = 0; i < 10; ++i) new (x+i) char;
-  std::destroy(x, x+10);
+  FwdIteratorRange<char> r(x, x+10);
+  std::destroy(r.begin(), r.end());
   free(x);
 }
 
@@ -164,8 +181,9 @@ void test11()
 {
   char* x = (char*)malloc(sizeof(char)*10);
   for (int i = 0; i < 10; ++i) new (x+i) char;
-  auto end = std::destroy_n(x, 10);
-  VERIFY( end == x + 10 );
+  FwdIteratorRange<char> r(x, x+10);
+  auto end = std::destroy_n(r.begin(), 10);
+  VERIFY( end == std::next(r.begin(), 10) );
   free(x);
 }
 
diff --git a/libstdc++-v3/testsuite/25_algorithms/fill_n/1.cc b/libstdc++-v3/testsuite/25_algorithms/fill_n/1.cc
index d446e7a7b60..4d27f0bd22b 100644
--- a/libstdc++-v3/testsuite/25_algorithms/fill_n/1.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/fill_n/1.cc
@@ -22,21 +22,36 @@
 #include <algorithm>
 #include <vector>
 #include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
+
+// Non-scalar type to exercise partial specialization of fill_n implementation.
+struct Value
+{
+  Value(int n) : n(n) { }
+
+  operator int() const { return n; }
+
+private:
+  int n;
+};
 
 void
 test01()
 {
   using namespace std;
+  using __gnu_test::test_container;
+  using __gnu_test::output_iterator_wrapper;
 
   const int A1[] = {3, 3, 3, 3, 3, 3, 3, 3, 3, 3};
   const int N1 = sizeof(A1) / sizeof(int);
   
   int i1[N1];
-  fill_n(i1, N1, 3);
+  test_container<int, output_iterator_wrapper> c1(i1, i1 + N1);
+  fill_n(c1.begin(), N1, 3);
   VERIFY( equal(i1, i1 + N1, A1) );
 
   vector<int> v1(N1);
-  fill_n(v1.begin(), N1, 3);
+  fill_n(v1.begin(), N1, Value(3));
   VERIFY( equal(v1.begin(), v1.end(), A1) );
 
   const char A2[] = {'\3', '\3', '\3', '\3', '\3',
diff --git a/libstdc++-v3/testsuite/25_algorithms/generate_n/1.cc b/libstdc++-v3/testsuite/25_algorithms/generate_n/1.cc
new file mode 100644
index 00000000000..dc3cb9f943b
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/generate_n/1.cc
@@ -0,0 +1,47 @@
+// Copyright (C) 2017 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 <algorithm>
+#include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
+
+struct Inc
+{
+  int operator()() { return ++i; }
+
+  int i;
+};
+
+void
+test01()
+{
+  const int N = 3;
+  int array[N];
+
+  using __gnu_test::test_container;
+  using __gnu_test::output_iterator_wrapper;
+  test_container<int, output_iterator_wrapper> c(array, array + N);
+  std::generate_n(c.begin(), N, Inc());
+  VERIFY(array[0] == 1);
+  VERIFY(array[1] == 2);
+  VERIFY(array[2] == 3);
+}
+
+int main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/25_algorithms/stable_partition/1.cc b/libstdc++-v3/testsuite/25_algorithms/stable_partition/1.cc
index 75851578267..f29d8170a83 100644
--- a/libstdc++-v3/testsuite/25_algorithms/stable_partition/1.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/stable_partition/1.cc
@@ -21,6 +21,7 @@
 #include <functional>
 #include <testsuite_new_operators.h>
 #include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
 
 const int A[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17};
 const int B[] = {2, 4, 6, 8, 10, 12, 14, 16, 1, 3, 5, 7, 9, 11, 13, 15, 17};
@@ -41,11 +42,16 @@ void
 test01()
 {
   using std::stable_partition;
+  using __gnu_test::test_container;
+  using __gnu_test::forward_iterator_wrapper;
 
   int s1[N];
   std::copy(A, A + N, s1);
 
-  VERIFY( stable_partition(s1, s1 + N, Pred()) == s1 + M );
+  test_container<int, forward_iterator_wrapper> c(s1, s1+N);
+  forward_iterator_wrapper<int> expected = c.begin();
+  std::advance(expected, M);
+  VERIFY( stable_partition(c.begin(), c.end(), Pred()) == expected);
   VERIFY( std::equal(s1, s1 + N, B) );
 }
 
diff --git a/libstdc++-v3/testsuite/util/testsuite_iterators.h b/libstdc++-v3/testsuite/util/testsuite_iterators.h
index d61b2162c14..d6b550dd416 100644
--- a/libstdc++-v3/testsuite/util/testsuite_iterators.h
+++ b/libstdc++-v3/testsuite/util/testsuite_iterators.h
@@ -180,6 +180,11 @@ namespace __gnu_test
 #endif
   };
 
+#if __cplusplus >= 201103L
+  template<typename T, typename U>
+    void operator,(const T&, const output_iterator_wrapper<U>&) = delete;
+#endif
+
   /**
    * @brief input_iterator wrapper for pointer
    *
@@ -270,6 +275,10 @@ namespace __gnu_test
 #endif
   };
 
+#if __cplusplus >= 201103L
+  template<typename T, typename U>
+    void operator,(const T&, const input_iterator_wrapper<U>&) = delete;
+#endif
 
   /**
    * @brief forward_iterator wrapper for pointer


More information about the Libstdc++ mailing list