[gcc/devel/gccgo] libstdc++: Fix incorrect use of memset in ranges::fill_n (PR 94017)

Ian Lance Taylor ian@gcc.gnu.org
Sat Apr 4 21:39:28 GMT 2020


https://gcc.gnu.org/g:712b182a8bc2d7510d7a2fbede43bf134c539f25

commit 712b182a8bc2d7510d7a2fbede43bf134c539f25
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue Mar 3 16:16:05 2020 -0500

    libstdc++: Fix incorrect use of memset in ranges::fill_n (PR 94017)
    
    When deciding whether to perform the memset optimization in ranges::fill_n, we
    were crucially neglecting to check that the output pointer's value type is a
    byte type.  This patch adds such a check to the problematic condition in
    ranges::fill_n.
    
    At the same time, this patch relaxes the overly conservative
    __is_byte<_Tp>::__value check that requires the fill type be a byte type.  It's
    overly conservative because it means we won't enable the memset optimization in
    the following example
    
      char c[100];
      ranges::fill(c, 37);
    
    because the fill type is deduced to be int here.  Rather than requiring that the
    fill type be a byte type, it seems safe to just require the fill type be an
    integral type, which is what this patch does.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/94017
            * include/bits/ranges_algobase.h (__fill_n_fn::operator()): Refine
            condition for when to use memset, making sure to additionally check that
            the output pointer's value type is a non-volatile byte type.  Instead of
            requiring that the fill type is a byte type, just require that it's an
            integral type.
            * testsuite/20_util/specialized_algorithms/uninitialized_fill/94017.cc:
            New test.
            * testsuite/20_util/specialized_algorithms/uninitialized_fill_n/94017.cc:
            New test.
            * testsuite/25_algorithms/fill/94013.cc: Uncomment part that was blocked
            by PR 94017.
            * testsuite/25_algorithms/fill/94017.cc: New test.
            * testsuite/25_algorithms/fill_n/94017.cc: New test.

Diff:
---
 libstdc++-v3/ChangeLog                             | 15 +++++
 libstdc++-v3/include/bits/ranges_algobase.h        |  7 +-
 .../uninitialized_fill/94017.cc                    | 77 ++++++++++++++++++++++
 .../uninitialized_fill_n/94017.cc                  | 77 ++++++++++++++++++++++
 libstdc++-v3/testsuite/25_algorithms/fill/94013.cc |  5 +-
 libstdc++-v3/testsuite/25_algorithms/fill/94017.cc | 76 +++++++++++++++++++++
 .../testsuite/25_algorithms/fill_n/94017.cc        | 76 +++++++++++++++++++++
 7 files changed, 328 insertions(+), 5 deletions(-)

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 3e4e58c1a7c..ff8ae64d477 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,5 +1,20 @@
 2020-03-04  Patrick Palka  <ppalka@redhat.com>
 
+	PR libstdc++/94017
+	* include/bits/ranges_algobase.h (__fill_n_fn::operator()): Refine
+	condition for when to use memset, making sure to additionally check that
+	the output pointer's value type is a non-volatile byte type.  Instead of
+	requiring that the fill type is a byte type, just require that it's an
+	integral type.
+	* testsuite/20_util/specialized_algorithms/uninitialized_fill/94017.cc:
+	New test.
+	* testsuite/20_util/specialized_algorithms/uninitialized_fill_n/94017.cc:
+	New test.
+	* testsuite/25_algorithms/fill/94013.cc: Uncomment part of test that was
+	blocked by PR 94017.
+	* testsuite/25_algorithms/fill/94017.cc: New test.
+	* testsuite/25_algorithms/fill_n/94017.cc: New test.
+
 	LWG 3355 The memory algorithms should support move-only input iterators
 	introduced by P1207
 	* include/bits/ranges_uninitialized.h
diff --git a/libstdc++-v3/include/bits/ranges_algobase.h b/libstdc++-v3/include/bits/ranges_algobase.h
index c0102f5ab11..80c9a774301 100644
--- a/libstdc++-v3/include/bits/ranges_algobase.h
+++ b/libstdc++-v3/include/bits/ranges_algobase.h
@@ -516,8 +516,11 @@ namespace ranges
 	if (__n <= 0)
 	  return __first;
 
-	// TODO: is __is_byte the best condition?
-	if constexpr (is_pointer_v<_Out> && __is_byte<_Tp>::__value)
+	// TODO: Generalize this optimization to contiguous iterators.
+	if constexpr (is_pointer_v<_Out>
+		      // Note that __is_byte already implies !is_volatile.
+		      && __is_byte<remove_pointer_t<_Out>>::__value
+		      && integral<_Tp>)
 	  {
 	    __builtin_memset(__first, static_cast<unsigned char>(__value), __n);
 	    return __first + __n;
diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_fill/94017.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_fill/94017.cc
new file mode 100644
index 00000000000..1686d1ba8d5
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_fill/94017.cc
@@ -0,0 +1,77 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <algorithm>
+#include <memory>
+#include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
+
+using __gnu_test::test_output_range;
+
+namespace ranges = std::ranges;
+
+template<typename Out, auto value>
+void
+test01()
+{
+    {
+      Out x[5];
+      ranges::uninitialized_fill(x, value);
+      VERIFY( ranges::count(x, static_cast<Out>(value)) == ranges::size(x) );
+    }
+
+    {
+      Out x[5];
+      test_output_range<Out> rx(x);
+      ranges::uninitialized_fill(x, value);
+      VERIFY( ranges::count(x, static_cast<Out>(value)) == ranges::size(x) );
+    }
+}
+
+int
+main()
+{
+  test01<char, 'a'>();
+  test01<char, 100>();
+  test01<char, 150>();
+  test01<char, 300>();
+  test01<char, 1000>();
+  test01<char, -10000L>();
+
+  test01<signed char, 'a'>();
+  test01<signed char, 100>();
+  test01<signed char, 150>();
+  test01<signed char, 300>();
+
+  test01<unsigned char, 'a'>();
+  test01<unsigned char, 100>();
+  test01<unsigned char, 150>();
+  test01<unsigned char, 300>();
+
+  test01<int, 'a'>();
+  test01<int, u8'a'>();
+  test01<int, (signed char)'a'>();
+  test01<int, (unsigned char)'a'>();
+
+  test01<volatile int, 'a'>();
+  test01<volatile int, 'a'>();
+  test01<volatile int, 500>();
+  test01<volatile char, 500>();
+}
diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_fill_n/94017.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_fill_n/94017.cc
new file mode 100644
index 00000000000..b5325080e02
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_fill_n/94017.cc
@@ -0,0 +1,77 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <algorithm>
+#include <memory>
+#include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
+
+using __gnu_test::test_output_range;
+
+namespace ranges = std::ranges;
+
+template<typename Out, auto value>
+void
+test01()
+{
+    {
+      Out x[5];
+      ranges::uninitialized_fill_n(x, 5, value);
+      VERIFY( ranges::count(x, static_cast<Out>(value)) == ranges::size(x) );
+    }
+
+    {
+      Out x[5];
+      test_output_range<Out> rx(x);
+      ranges::uninitialized_fill_n(x, 5, value);
+      VERIFY( ranges::count(x, static_cast<Out>(value)) == ranges::size(x) );
+    }
+}
+
+int
+main()
+{
+  test01<char, 'a'>();
+  test01<char, 100>();
+  test01<char, 150>();
+  test01<char, 300>();
+  test01<char, 1000>();
+  test01<char, -10000L>();
+
+  test01<signed char, 'a'>();
+  test01<signed char, 100>();
+  test01<signed char, 150>();
+  test01<signed char, 300>();
+
+  test01<unsigned char, 'a'>();
+  test01<unsigned char, 100>();
+  test01<unsigned char, 150>();
+  test01<unsigned char, 300>();
+
+  test01<int, 'a'>();
+  test01<int, u8'a'>();
+  test01<int, (signed char)'a'>();
+  test01<int, (unsigned char)'a'>();
+
+  test01<volatile int, 'a'>();
+  test01<volatile int, 'a'>();
+  test01<volatile int, 500>();
+  test01<volatile char, 500>();
+}
diff --git a/libstdc++-v3/testsuite/25_algorithms/fill/94013.cc b/libstdc++-v3/testsuite/25_algorithms/fill/94013.cc
index b28eb76157b..9785d740a35 100644
--- a/libstdc++-v3/testsuite/25_algorithms/fill/94013.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/fill/94013.cc
@@ -33,9 +33,8 @@ test01()
   c = 4;
   std::ranges::fill(a, c);
   VERIFY( a[0] == 4 && a[1] == 4 );
-  // currently fails, see PR 94017
-  // unsigned char c2 = 5;
-  // std::ranges::fill(a, c2);
+  unsigned char c2 = 5;
+  std::ranges::fill(a, c2);
 #endif
 }
 
diff --git a/libstdc++-v3/testsuite/25_algorithms/fill/94017.cc b/libstdc++-v3/testsuite/25_algorithms/fill/94017.cc
new file mode 100644
index 00000000000..ace4cc9c87f
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/fill/94017.cc
@@ -0,0 +1,76 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <algorithm>
+#include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
+
+using __gnu_test::test_output_range;
+
+namespace ranges = std::ranges;
+
+template<typename Out, auto value>
+void
+test01()
+{
+    {
+      Out x[5];
+      ranges::fill(x, value);
+      VERIFY( ranges::count(x, static_cast<Out>(value)) == ranges::size(x) );
+    }
+
+    {
+      Out x[5];
+      test_output_range<Out> rx(x);
+      ranges::fill(x, value);
+      VERIFY( ranges::count(x, static_cast<Out>(value)) == ranges::size(x) );
+    }
+}
+
+int
+main()
+{
+  test01<char, 'a'>();
+  test01<char, 100>();
+  test01<char, 150>();
+  test01<char, 300>();
+  test01<char, 1000>();
+  test01<char, -10000L>();
+
+  test01<signed char, 'a'>();
+  test01<signed char, 100>();
+  test01<signed char, 150>();
+  test01<signed char, 300>();
+
+  test01<unsigned char, 'a'>();
+  test01<unsigned char, 100>();
+  test01<unsigned char, 150>();
+  test01<unsigned char, 300>();
+
+  test01<int, 'a'>();
+  test01<int, u8'a'>();
+  test01<int, (signed char)'a'>();
+  test01<int, (unsigned char)'a'>();
+
+  test01<volatile int, 'a'>();
+  test01<volatile int, 'a'>();
+  test01<volatile int, 500>();
+  test01<volatile char, 500>();
+}
diff --git a/libstdc++-v3/testsuite/25_algorithms/fill_n/94017.cc b/libstdc++-v3/testsuite/25_algorithms/fill_n/94017.cc
new file mode 100644
index 00000000000..fc93dd5ab26
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/fill_n/94017.cc
@@ -0,0 +1,76 @@
+// Copyright (C) 2020 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/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <algorithm>
+#include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
+
+using __gnu_test::test_output_range;
+
+namespace ranges = std::ranges;
+
+template<typename Out, auto value>
+void
+test01()
+{
+    {
+      Out x[5];
+      ranges::fill_n(x, 5, value);
+      VERIFY( ranges::count(x, static_cast<Out>(value)) == ranges::size(x) );
+    }
+
+    {
+      Out x[5];
+      test_output_range<Out> rx(x);
+      ranges::fill_n(x, 5, value);
+      VERIFY( ranges::count(x, static_cast<Out>(value)) == ranges::size(x) );
+    }
+}
+
+int
+main()
+{
+  test01<char, 'a'>();
+  test01<char, 100>();
+  test01<char, 150>();
+  test01<char, 300>();
+  test01<char, 1000>();
+  test01<char, -10000L>();
+
+  test01<signed char, 'a'>();
+  test01<signed char, 100>();
+  test01<signed char, 150>();
+  test01<signed char, 300>();
+
+  test01<unsigned char, 'a'>();
+  test01<unsigned char, 100>();
+  test01<unsigned char, 150>();
+  test01<unsigned char, 300>();
+
+  test01<int, 'a'>();
+  test01<int, u8'a'>();
+  test01<int, (signed char)'a'>();
+  test01<int, (unsigned char)'a'>();
+
+  test01<volatile int, 'a'>();
+  test01<volatile int, 'a'>();
+  test01<volatile int, 500>();
+  test01<volatile char, 500>();
+}


More information about the Libstdc++-cvs mailing list