On 02/03/20 13:12 +0000, Jonathan Wakely wrote:
On 02/03/20 13:22 +0100, Christophe Lyon wrote:
On Fri, 28 Feb 2020 at 22:53, Jonathan Wakely <jwak...@redhat.com> wrote:
On 28/02/20 14:59 -0500, Patrick Palka wrote:
We were enabling the memcmp optimization in ranges::lexicographical_compare for
signed integral types and for integral types larger than a byte. But memcmp
gives the wrong answer for arrays of such types. This patch fixes this issue by
refining the condition that enables the memcmp optimization. It's now
consistent with the corresponding condition used in
std::lexicographical_compare.
libstdc++-v3/ChangeLog:
PR libstdc++/93972
* include/bits/ranges_algo.h (__lexicographical_compare_fn::operator()):
Fix condition for when to use memcmp, making it consistent with the
corresponding condition used in std::lexicographical_compare.
* testsuite/25_algorithms/lexicographical_compare/93972.cc: New test.
Hi,
The new test fails on aarch64 and arm, and other targets according to
gcc-testresults.
On aarch64, my log says:
FAIL: 25_algorithms/lexicographical_compare/93972.cc (test for excess errors)
Excess errors:
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/bits/ranges_algo.h:3490:
error: no matching function for call to '__memcmp(char*&, unsigned
char*&, const long int&)'
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/bits/ranges_algo.h:3490:
error: no matching function for call to '__memcmp(char*&, unsigned
char*&, const long int&)'
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/bits/ranges_algo.h:3490:
error: no matching function for call to '__memcmp(unsigned char*&,
char*&, const long int&)'
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/bits/ranges_algo.h:3490:
error: no matching function for call to '__memcmp(unsigned char*&,
char*&, const long int&)'
UNRESOLVED: 25_algorithms/lexicographical_compare/93972.cc compilation
failed to produce executable
Hmm, I think this was already broken in std::lexicographical_compare,
we just didn't have a test for it. I think the following will also
fail to compile on aarch64 and ARM (and any target where char is
unsigned):
#include <algorithm>
#include <assert.h>
int main()
{
unsigned char a[] = {1, 2, 3, 4};
char b[] = {1, 2, 3, 5};
assert( std::lexicographical_compare(a, a+4, b, b+4) );
}
So Patrick's ranges::lexicographical_compare didn't introduce the bug,
it just found it by having better tests.
The std::__memcmp function is broken in a similar way to the
std::__memmove function that I removed last week. I'll fix that
today...
Fixed with this patch, tested powerpc64le-linux and committed to
master.
Thanks for noticing the new FAIL.
commit d112e173ea093f55a16a14b26ef65088381ee09c
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Mon Mar 2 17:03:28 2020 +0000
libstdc++: Fix std::lexicographic_compare for unsigned char (PR 93972)
The new 25_algorithms/lexicographical_compare/93972.cc test fails on
targets where char is unsigned, revealing an existing regression with
the std::__memcmp helper that had gone unnoticed in
std::lexicographical_compare. When comparing char and unsigned char, the
memcmp optimisation is enabled, but the new std::__memcmp function fails
to compile for mismatched types.
PR libstdc++/93972
* include/bits/stl_algobase.h (__memcmp): Allow pointer types to
differ.
* testsuite/25_algorithms/lexicographical_compare/uchar.cc: New test.
diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 5ec2f25424d..7a9d932b421 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -84,11 +84,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* A constexpr wrapper for __builtin_memcmp.
* @param __num The number of elements of type _Tp (not bytes).
*/
- template<typename _Tp>
+ template<typename _Tp, typename _Up>
_GLIBCXX14_CONSTEXPR
inline int
- __memcmp(const _Tp* __first1, const _Tp* __first2, size_t __num)
+ __memcmp(const _Tp* __first1, const _Up* __first2, size_t __num)
{
+#if __cplusplus >= 201103L
+ static_assert(sizeof(_Tp) == sizeof(_Up), "can be compared with memcmp");
+#endif
#ifdef __cpp_lib_is_constant_evaluated
if (std::is_constant_evaluated())
{
diff --git a/libstdc++-v3/testsuite/25_algorithms/lexicographical_compare/uchar.cc b/libstdc++-v3/testsuite/25_algorithms/lexicographical_compare/uchar.cc
new file mode 100644
index 00000000000..990bb1e7489
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/lexicographical_compare/uchar.cc
@@ -0,0 +1,61 @@
+// 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 "-fchar8_t" }
+// { dg-do run }
+
+#include <algorithm>
+#include <testsuite_hooks.h>
+
+// Check that the memcmp optimization in std::lexicographical_compare
+// still works for mixed-type comparisons.
+
+void
+test01()
+{
+ unsigned char a = 0;
+ char8_t b = 1;
+ VERIFY( std::lexicographical_compare(&a, &a + 1, &b, &b + 1) );
+ VERIFY( ! std::lexicographical_compare(&b, &b + 1, &a, &a + 1) );
+}
+
+void
+test02()
+{
+ // N.B. if char is signed this won't actually use the memcmp optimization.
+ unsigned char a = 0;
+ char b = 1;
+ VERIFY( std::lexicographical_compare(&a, &a + 1, &b, &b + 1) );
+ VERIFY( ! std::lexicographical_compare(&b, &b + 1, &a, &a + 1) );
+}
+
+void
+test03()
+{
+ // N.B. if char is signed this won't actually use the memcmp optimization.
+ char a = 0;
+ char8_t b = 1;
+ VERIFY( std::lexicographical_compare(&a, &a + 1, &b, &b + 1) );
+ VERIFY( ! std::lexicographical_compare(&b, &b + 1, &a, &a + 1) );
+}
+
+int main()
+{
+ test01();
+ test02();
+ test03();
+}
