[Bug libstdc++/99181] New: char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[Bug libstdc++/99181] New: char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[richardpku at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

            Bug ID: 99181
           Summary: char_traits<char> (and thus string_view) compares
                    strings differently in constexpr and non-constexpr
                    contexts
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: richardpku at gmail dot com
  Target Milestone: ---

Minimal program to produce bug (run it on a platform where char is a signed
type, such as i386/x86-64):

/tmp % cat a.cpp
#include <string_view>
#include <iostream>

using namespace std;

int main() {
        // constexpr
        constexpr bool i = ("\xff"sv > "aaa"sv);
        cout << i << ",";

        // not constexpr
        auto a = "\xff"sv, b = "aaa"sv;
        cout << (a > b) << endl;

        return 0;
}
/tmp % g++ -std=gnu++2a a.cpp && ./a.out
0,1

The expected result is "1,1".


In a non-constexpr context, std::char_traits<char>::compare invokes
__builtin_memcmp, which is required by C standard to interpret characters as
unsigned char.

In a constexpr context, however, std::char_traits<char>::compare invokes
__gnu_cxx::char_traits<char>::compare, which in turn calls
__gnu_cxx::char_traits<char>::lt to compare chars.
__gnu_cxx::char_traits<char>::lt (unlike std::char_traits<char>::lt) is not
specialized to compare chars as unsigned char.

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
      static _GLIBCXX_CONSTEXPR bool
      lt(const char_type& __c1, const char_type& __c2) _GLIBCXX_NOEXCEPT
      {
        // LWG 467.
        return (static_cast<unsigned char>(__c1)
                < static_cast<unsigned char>(__c2));
      }

      static _GLIBCXX17_CONSTEXPR int
      compare(const char_type* __s1, const char_type* __s2, size_t __n)
      {
        if (__n == 0)
          return 0;
#if __cplusplus >= 201703L
        if (__builtin_constant_p(__n)
            && __constant_char_array_p(__s1, __n)
            && __constant_char_array_p(__s2, __n))
          return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n);
#endif
        return __builtin_memcmp(__s1, __s2, __n);
      }

Seems the std::char_traits<char>::lt specialization implements LWG467, and so
does std::char_traits<char>::compare when not in constant expression
evaluation,
but the C++17 case above will call __gnu_cxx::char_traits<char_type>::compare
which doesn't know about LWG 467.

--- libstdc++-v3/include/bits/char_traits.h.jj  2021-01-04 10:26:03.558953845
+0100
+++ libstdc++-v3/include/bits/char_traits.h     2021-02-22 10:42:08.197199781
+0100
@@ -349,7 +349,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        if (__builtin_constant_p(__n)
            && __constant_char_array_p(__s1, __n)
            && __constant_char_array_p(__s2, __n))
-         return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n);
+         {
+           for (std::size_t __i = 0; __i < __n; ++__i)
+             if (lt(__s1[__i], __s2[__i]))
+               return -1;
+             else if (lt(__s2[__i], __s1[__i]))
+               return 1;
+           return 0;
+         }
 #endif
        return __builtin_memcmp(__s1, __s2, __n);
       }

seems to fix this, but not sure if there isn't a better way.

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2021-02-22
     Ever confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50234
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50234&action=edit
gcc11-pr99181.patch

Full untested patch.

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We're in namespace std here so size_t doesn't need to be qualified, but
otherwise the patch is fine.

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:efa64fcce12074dd542670feb02eaee53e810a30

commit r11-7340-gefa64fcce12074dd542670feb02eaee53e810a30
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Feb 23 09:30:18 2021 +0100

    libstdc++: Fix up constexpr std::char_traits<char>::compare [PR99181]

    Because of LWG 467, std::char_traits<char>::lt compares the values
    cast to unsigned char rather than char, so even when char is signed
    we get unsigned comparision.  std::char_traits<char>::compare uses
    __builtin_memcmp and that works the same, but during constexpr evaluation
    we were calling __gnu_cxx::char_traits<char_type>::compare.  As
    char_traits::lt is not virtual, __gnu_cxx::char_traits<char_type>::compare
    used __gnu_cxx::char_traits<char_type>::lt rather than
    std::char_traits<char>::lt and thus compared chars as signed if char is
    signed.
    This change fixes it by inlining __gnu_cxx::char_traits<char_type>::compare
    into std::char_traits<char>::compare by hand, so that it calls the right
    lt method.

    2021-02-23  Jakub Jelinek  <jakub@redhat.com>

            PR libstdc++/99181
            * include/bits/char_traits.h (char_traits<char>::compare): For
            constexpr evaluation don't call
            __gnu_cxx::char_traits<char_type>::compare but do the comparison
loop
            directly.

            * testsuite/21_strings/char_traits/requirements/char/99181.cc: New
            test.

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk, probably needs backporting to 10/9/8 branches.

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> Fixed on the trunk, probably needs backporting to 10/9/8 branches.

Yes, all three branches. It's been wrong since r249137 on trunk, which I
backported to gcc-7 (for 7.3.0) as r252030

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:311c57f6d8f285d69e44bf94152c753900cb1a0a

commit r10-9474-g311c57f6d8f285d69e44bf94152c753900cb1a0a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Feb 23 09:30:18 2021 +0100

    libstdc++: Fix up constexpr std::char_traits<char>::compare [PR99181]

    Because of LWG 467, std::char_traits<char>::lt compares the values
    cast to unsigned char rather than char, so even when char is signed
    we get unsigned comparision.  std::char_traits<char>::compare uses
    __builtin_memcmp and that works the same, but during constexpr evaluation
    we were calling __gnu_cxx::char_traits<char_type>::compare.  As
    char_traits::lt is not virtual, __gnu_cxx::char_traits<char_type>::compare
    used __gnu_cxx::char_traits<char_type>::lt rather than
    std::char_traits<char>::lt and thus compared chars as signed if char is
    signed.
    This change fixes it by inlining __gnu_cxx::char_traits<char_type>::compare
    into std::char_traits<char>::compare by hand, so that it calls the right
    lt method.

    2021-02-23  Jakub Jelinek  <jakub@redhat.com>

            PR libstdc++/99181
            * include/bits/char_traits.h (char_traits<char>::compare): For
            constexpr evaluation don't call
            __gnu_cxx::char_traits<char_type>::compare but do the comparison
loop
            directly.

            * testsuite/21_strings/char_traits/requirements/char/99181.cc: New
            test.

    (cherry picked from commit efa64fcce12074dd542670feb02eaee53e810a30)

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:03e18a40070c6fe89397a73d85a38a99371cf8a1

commit r9-9424-g03e18a40070c6fe89397a73d85a38a99371cf8a1
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Feb 23 09:30:18 2021 +0100

    libstdc++: Fix up constexpr std::char_traits<char>::compare [PR99181]

    Because of LWG 467, std::char_traits<char>::lt compares the values
    cast to unsigned char rather than char, so even when char is signed
    we get unsigned comparision.  std::char_traits<char>::compare uses
    __builtin_memcmp and that works the same, but during constexpr evaluation
    we were calling __gnu_cxx::char_traits<char_type>::compare.  As
    char_traits::lt is not virtual, __gnu_cxx::char_traits<char_type>::compare
    used __gnu_cxx::char_traits<char_type>::lt rather than
    std::char_traits<char>::lt and thus compared chars as signed if char is
    signed.
    This change fixes it by inlining __gnu_cxx::char_traits<char_type>::compare
    into std::char_traits<char>::compare by hand, so that it calls the right
    lt method.

    2021-02-23  Jakub Jelinek  <jakub@redhat.com>

            PR libstdc++/99181
            * include/bits/char_traits.h (char_traits<char>::compare): For
            constexpr evaluation don't call
            __gnu_cxx::char_traits<char_type>::compare but do the comparison
loop
            directly.

            * testsuite/21_strings/char_traits/requirements/char/99181.cc: New
            test.

    (cherry picked from commit 311c57f6d8f285d69e44bf94152c753900cb1a0a)

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:21a7edf3a0cea656f2de834ea8320870496d5a2f

commit r8-10889-g21a7edf3a0cea656f2de834ea8320870496d5a2f
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Feb 23 09:30:18 2021 +0100

    libstdc++: Fix up constexpr std::char_traits<char>::compare [PR99181]

    Because of LWG 467, std::char_traits<char>::lt compares the values
    cast to unsigned char rather than char, so even when char is signed
    we get unsigned comparision.  std::char_traits<char>::compare uses
    __builtin_memcmp and that works the same, but during constexpr evaluation
    we were calling __gnu_cxx::char_traits<char_type>::compare.  As
    char_traits::lt is not virtual, __gnu_cxx::char_traits<char_type>::compare
    used __gnu_cxx::char_traits<char_type>::lt rather than
    std::char_traits<char>::lt and thus compared chars as signed if char is
    signed.
    This change fixes it by inlining __gnu_cxx::char_traits<char_type>::compare
    into std::char_traits<char>::compare by hand, so that it calls the right
    lt method.

    2021-02-23  Jakub Jelinek  <jakub@redhat.com>

            PR libstdc++/99181
            * include/bits/char_traits.h (char_traits<char>::compare): For
            constexpr evaluation don't call
            __gnu_cxx::char_traits<char_type>::compare but do the comparison
loop
            directly.

            * testsuite/21_strings/char_traits/requirements/char/99181.cc: New
            test.

    (cherry picked from commit 311c57f6d8f285d69e44bf94152c753900cb1a0a)

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug libstdc++/99181] char_traits<char> (and thus string_view) compares strings differently in constexpr and non-constexpr contexts

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99181

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5