[libcxx-commits] [libcxx] 157f34a - [libc++] Fixes basic_string operator& hijacking.

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Tue Mar 7 10:04:02 PST 2023


Author: Mark de Wever
Date: 2023-03-07T19:03:56+01:00
New Revision: 157f34af7157686e6dcae631573034046340953d

URL: https://github.com/llvm/llvm-project/commit/157f34af7157686e6dcae631573034046340953d
DIFF: https://github.com/llvm/llvm-project/commit/157f34af7157686e6dcae631573034046340953d.diff

LOG: [libc++] Fixes basic_string operator& hijacking.

Avoids using operator& in basic_string since an evil char-like type can
hijack this operator. Added some more evil operators, this found a place
where equality was compared directly and not via the traits.

This adds a helper test string. This is now only used in a few tests,
but the intention is to use this in more tests for basic_string.

Reviewed By: #libc, ldionne

Differential Revision: https://reviews.llvm.org/D145257

Added: 
    libcxx/test/support/nasty_string.h

Modified: 
    libcxx/include/string
    libcxx/test/std/strings/basic.string/string.modifiers/string_append/initializer_list.pass.cpp
    libcxx/test/std/strings/basic.string/string.modifiers/string_assign/string.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/string b/libcxx/include/string
index ede917f3431e..787064d45490 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2522,7 +2522,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20
 basic_string<_CharT, _Traits, _Allocator>&
 basic_string<_CharT, _Traits, _Allocator>::operator=(const basic_string& __str)
 {
-  if (this != &__str) {
+  if (this != std::addressof(__str)) {
     __copy_assign_alloc(__str);
     if (!__is_long()) {
       if (!__str.__is_long()) {
@@ -3982,7 +3982,7 @@ basic_string<_CharT, _Traits, _Allocator>::__invariants() const
         return false;
     if (data() == nullptr)
         return false;
-    if (data()[size()] != value_type())
+    if (!_Traits::eq(data()[size()], value_type()))
         return false;
     return true;
 }

diff  --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_append/initializer_list.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_append/initializer_list.pass.cpp
index ebe4e49e16cd..6c5049f16bc6 100644
--- a/libcxx/test/std/strings/basic.string/string.modifiers/string_append/initializer_list.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_append/initializer_list.pass.cpp
@@ -17,25 +17,37 @@
 
 #include "test_macros.h"
 #include "min_allocator.h"
+#include "nasty_string.h"
+
+template <class S>
+TEST_CONSTEXPR_CXX20 void test() {
+  using CharT = typename S::value_type;
+
+  S s(CONVERT_TO_CSTRING(CharT, "123"));
+  s.append({CharT('a'), CharT('b'), CharT('c')});
+  assert(s == CONVERT_TO_CSTRING(CharT, "123abc"));
+}
 
 TEST_CONSTEXPR_CXX20 bool test() {
-  {
-    std::string s("123");
-    s.append({'a', 'b', 'c'});
-    assert(s == "123abc");
-  }
-  {
-    typedef std::basic_string<char, std::char_traits<char>, min_allocator<char>> S;
-    S s("123");
-    s.append({'a', 'b', 'c'});
-    assert(s == "123abc");
-  }
+  test<std::string>();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  test<std::wstring>();
+#endif
+#if TEST_STD_VER >= 20
+  test<std::u8string>();
+#endif
+  test<std::u16string>();
+  test<std::u32string>();
+
+  test<std::basic_string<char, std::char_traits<char>, min_allocator<char>>>();
+#ifndef TEST_HAS_NO_NASTY_STRING
+  test<nasty_string>();
+#endif
 
   return true;
 }
 
-int main(int, char**)
-{
+int main(int, char**) {
   test();
 #if TEST_STD_VER > 17
   static_assert(test());

diff  --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/string.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/string.pass.cpp
index c0822d403bc6..99503741743f 100644
--- a/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/string.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/string.pass.cpp
@@ -15,91 +15,89 @@
 #include <cassert>
 
 #include "test_macros.h"
+#include "nasty_string.h"
 #include "min_allocator.h"
 #include "test_allocator.h"
 
 template <class S>
-TEST_CONSTEXPR_CXX20 void
-test(S s, S str, S expected)
-{
-    s.assign(str);
-    LIBCPP_ASSERT(s.__invariants());
-    assert(s == expected);
+TEST_CONSTEXPR_CXX20 void test(S dest, S src) {
+  dest.assign(src);
+  LIBCPP_ASSERT(dest.__invariants());
+  assert(dest == src);
 }
 
 template <class S>
 TEST_CONSTEXPR_CXX20 void
-testAlloc(S s, S str, const typename S::allocator_type& a)
+testAlloc(S dest, S src, const typename S::allocator_type& a)
 {
-    s.assign(str);
-    LIBCPP_ASSERT(s.__invariants());
-    assert(s == str);
-    assert(s.get_allocator() == a);
+    dest.assign(src);
+    LIBCPP_ASSERT(dest.__invariants());
+    assert(dest == src);
+    assert(dest.get_allocator() == a);
+}
+
+template <class S>
+TEST_CONSTEXPR_CXX20 void test_assign() {
+    using CharT = typename S::value_type;
+
+    test(S(), S());
+    test(S(), S(CONVERT_TO_CSTRING(CharT, "12345")));
+    test(S(), S(CONVERT_TO_CSTRING(CharT, "1234567890")));
+    test(S(), S(CONVERT_TO_CSTRING(CharT, "12345678901234567890")));
+
+    test(S(CONVERT_TO_CSTRING(CharT, "12345")), S());
+    test(S(CONVERT_TO_CSTRING(CharT, "12345")), S(CONVERT_TO_CSTRING(CharT, "12345")));
+    test(S(CONVERT_TO_CSTRING(CharT, "12345")), S(CONVERT_TO_CSTRING(CharT, "1234567890")));
+    test(S(CONVERT_TO_CSTRING(CharT, "12345")), S(CONVERT_TO_CSTRING(CharT, "12345678901234567890")));
+
+    test(S(CONVERT_TO_CSTRING(CharT, "1234567890")), S());
+    test(S(CONVERT_TO_CSTRING(CharT, "1234567890")), S(CONVERT_TO_CSTRING(CharT, "12345")));
+    test(S(CONVERT_TO_CSTRING(CharT, "1234567890")), S(CONVERT_TO_CSTRING(CharT, "1234567890")));
+    test(S(CONVERT_TO_CSTRING(CharT, "1234567890")), S(CONVERT_TO_CSTRING(CharT, "12345678901234567890")));
+
+    test(S(CONVERT_TO_CSTRING(CharT, "12345678901234567890")), S());
+    test(S(CONVERT_TO_CSTRING(CharT, "12345678901234567890")), S(CONVERT_TO_CSTRING(CharT, "12345")));
+    test(S(CONVERT_TO_CSTRING(CharT, "12345678901234567890")), S(CONVERT_TO_CSTRING(CharT, "1234567890")));
+    test(S(CONVERT_TO_CSTRING(CharT, "12345678901234567890")), S(CONVERT_TO_CSTRING(CharT, "12345678901234567890")));
 }
 
 TEST_CONSTEXPR_CXX20 bool test() {
-  {
-    typedef std::string S;
-    test(S(), S(), S());
-    test(S(), S("12345"), S("12345"));
-    test(S(), S("1234567890"), S("1234567890"));
-    test(S(), S("12345678901234567890"), S("12345678901234567890"));
-
-    test(S("12345"), S(), S());
-    test(S("12345"), S("12345"), S("12345"));
-    test(S("12345"), S("1234567890"), S("1234567890"));
-    test(S("12345"), S("12345678901234567890"), S("12345678901234567890"));
-
-    test(S("1234567890"), S(), S());
-    test(S("1234567890"), S("12345"), S("12345"));
-    test(S("1234567890"), S("1234567890"), S("1234567890"));
-    test(S("1234567890"), S("12345678901234567890"), S("12345678901234567890"));
-
-    test(S("12345678901234567890"), S(), S());
-    test(S("12345678901234567890"), S("12345"), S("12345"));
-    test(S("12345678901234567890"), S("1234567890"), S("1234567890"));
-    test(S("12345678901234567890"), S("12345678901234567890"),
-         S("12345678901234567890"));
-
-    testAlloc(S(), S(), std::allocator<char>());
-    testAlloc(S(), S("12345"), std::allocator<char>());
-    testAlloc(S(), S("1234567890"), std::allocator<char>());
-    testAlloc(S(), S("12345678901234567890"), std::allocator<char>());
-  }
+    test_assign<std::string>();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+    test_assign<std::wstring>();
+#endif
+#if TEST_STD_VER >= 20
+    test_assign<std::u8string>();
+#endif
+#if TEST_STD_VER >= 11
+    test_assign<std::u16string>();
+    test_assign<std::u32string>();
+#endif
+#ifndef TEST_HAS_NO_NASTY_STRING
+    test_assign<nasty_string>();
+#endif
 
-  { //  LWG#5579 make sure assign takes the allocators where appropriate
-    typedef other_allocator<char> A;  // has POCCA --> true
-    typedef std::basic_string<char, std::char_traits<char>, A> S;
-    testAlloc(S(A(5)), S(A(3)), A(3));
-    testAlloc(S(A(5)), S("1"), A());
-    testAlloc(S(A(5)), S("1", A(7)), A(7));
-    testAlloc(S(A(5)), S("1234567890123456789012345678901234567890123456789012345678901234567890", A(7)), A(7));
-  }
+    {
+      typedef std::string S;
+      testAlloc(S(), S(), std::allocator<char>());
+      testAlloc(S(), S("12345"), std::allocator<char>());
+      testAlloc(S(), S("1234567890"), std::allocator<char>());
+      testAlloc(S(), S("12345678901234567890"), std::allocator<char>());
+    }
+
+    {                                  //  LWG#5579 make sure assign takes the allocators where appropriate
+      typedef other_allocator<char> A; // has POCCA --> true
+      typedef std::basic_string<char, std::char_traits<char>, A> S;
+      testAlloc(S(A(5)), S(A(3)), A(3));
+      testAlloc(S(A(5)), S("1"), A());
+      testAlloc(S(A(5)), S("1", A(7)), A(7));
+      testAlloc(S(A(5)), S("1234567890123456789012345678901234567890123456789012345678901234567890", A(7)), A(7));
+    }
 
 #if TEST_STD_VER >= 11
   {
     typedef std::basic_string<char, std::char_traits<char>, min_allocator<char>> S;
-    test(S(), S(), S());
-    test(S(), S("12345"), S("12345"));
-    test(S(), S("1234567890"), S("1234567890"));
-    test(S(), S("12345678901234567890"), S("12345678901234567890"));
-
-    test(S("12345"), S(), S());
-    test(S("12345"), S("12345"), S("12345"));
-    test(S("12345"), S("1234567890"), S("1234567890"));
-    test(S("12345"), S("12345678901234567890"), S("12345678901234567890"));
-
-    test(S("1234567890"), S(), S());
-    test(S("1234567890"), S("12345"), S("12345"));
-    test(S("1234567890"), S("1234567890"), S("1234567890"));
-    test(S("1234567890"), S("12345678901234567890"), S("12345678901234567890"));
-
-    test(S("12345678901234567890"), S(), S());
-    test(S("12345678901234567890"), S("12345"), S("12345"));
-    test(S("12345678901234567890"), S("1234567890"), S("1234567890"));
-    test(S("12345678901234567890"), S("12345678901234567890"),
-         S("12345678901234567890"));
-
+    test_assign<S>();
     testAlloc(S(), S(), min_allocator<char>());
     testAlloc(S(), S("12345"), min_allocator<char>());
     testAlloc(S(), S("1234567890"), min_allocator<char>());

diff  --git a/libcxx/test/support/nasty_string.h b/libcxx/test/support/nasty_string.h
new file mode 100644
index 000000000000..901700abef37
--- /dev/null
+++ b/libcxx/test/support/nasty_string.h
@@ -0,0 +1,184 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_SUPPORT_NASTY_STRING_H
+#define TEST_SUPPORT_NASTY_STRING_H
+
+#include <algorithm>
+#include <string>
+#include <type_traits>
+
+#include "make_string.h"
+#include "test_macros.h"
+
+// This defines a nasty_string similar to nasty_containers. This string's
+// value_type does operator hijacking, which allows us to ensure that the
+// library uses the provided `CharTraits` instead of using operations on
+// the value_type directly.
+
+
+// When using the code during constant evaluation it relies on
+//   P2647R1 Permitting static constexpr variables in constexpr functions
+// This is a C++23 feature, which is not supported by all compilers yet.
+// * GCC >= 13
+// * Clang >= 16
+// * MSVC no support yet
+//
+// TODO After there is proper compiler support use TEST_STD_VER >= 23 instead
+// of this macro in the tests.
+#if TEST_STD_VER < 23 || __cpp_constexpr < 202211L
+#  define TEST_HAS_NO_NASTY_STRING
+#endif
+
+#ifndef TEST_HAS_NO_NASTY_STRING
+// Make sure the char-like operations in strings do not depend on the char-like type.
+struct nasty_char {
+  template <typename T>
+  friend auto operator<=>(T, T) = delete;
+
+  template <typename T>
+  friend void operator+(T&&) = delete;
+
+  template <typename T>
+  friend void operator-(T&&) = delete;
+
+  template <typename T>
+  friend void operator&(T&&) = delete;
+
+  char c;
+};
+
+static_assert(std::is_trivial<nasty_char>::value, "");
+static_assert(std::is_standard_layout<nasty_char>::value, "");
+
+// These traits are based on the constexpr_traits test class.
+struct nasty_char_traits {
+  typedef nasty_char char_type;
+  typedef int int_type;
+  typedef std::streamoff off_type;
+  typedef std::streampos pos_type;
+  typedef std::mbstate_t state_type;
+  // The comparison_category is omitted so the class will have weak_ordering
+  // in C++20. This is intentional.
+
+  static constexpr void assign(char_type& c1, const char_type& c2) noexcept { c1 = c2; }
+
+  static constexpr bool eq(char_type c1, char_type c2) noexcept { return c1.c == c2.c; }
+
+  static constexpr bool lt(char_type c1, char_type c2) noexcept { return c1.c < c2.c; }
+
+  static constexpr int compare(const char_type* s1, const char_type* s2, size_t n);
+  static constexpr size_t length(const char_type* s);
+  static constexpr const char_type* find(const char_type* s, size_t n, const char_type& a);
+  static constexpr char_type* move(char_type* s1, const char_type* s2, size_t n);
+  static constexpr char_type* copy(char_type* s1, const char_type* s2, size_t n);
+  static constexpr char_type* assign(char_type* s, size_t n, char_type a);
+
+  static constexpr int_type not_eof(int_type c) noexcept { return eq_int_type(c, eof()) ? ~eof() : c; }
+
+  static constexpr char_type to_char_type(int_type c) noexcept { return char_type(c); }
+
+  static constexpr int_type to_int_type(char_type c) noexcept { return int_type(c.c); }
+
+  static constexpr bool eq_int_type(int_type c1, int_type c2) noexcept { return c1 == c2; }
+
+  static constexpr int_type eof() noexcept { return int_type(EOF); }
+};
+
+constexpr int nasty_char_traits::compare(const nasty_char* s1, const nasty_char* s2, size_t n) {
+  for (; n; --n, ++s1, ++s2) {
+    if (lt(*s1, *s2))
+      return -1;
+    if (lt(*s2, *s1))
+      return 1;
+  }
+  return 0;
+}
+
+constexpr size_t nasty_char_traits::length(const nasty_char* s) {
+  size_t len = 0;
+  for (; !eq(*s, nasty_char(0)); ++s)
+    ++len;
+  return len;
+}
+
+constexpr const nasty_char* nasty_char_traits::find(const nasty_char* s, size_t n, const nasty_char& a) {
+  for (; n; --n) {
+    if (eq(*s, a))
+      return s;
+    ++s;
+  }
+  return 0;
+}
+
+constexpr nasty_char* nasty_char_traits::move(nasty_char* s1, const nasty_char* s2, size_t n) {
+  nasty_char* r = s1;
+  if (s1 < s2) {
+    for (; n; --n, ++s1, ++s2)
+      assign(*s1, *s2);
+  } else if (s2 < s1) {
+    s1 += n;
+    s2 += n;
+    for (; n; --n)
+      assign(*--s1, *--s2);
+  }
+  return r;
+}
+
+constexpr nasty_char* nasty_char_traits::copy(nasty_char* s1, const nasty_char* s2, size_t n) {
+  if (!std::is_constant_evaluated()) // fails in constexpr because we might be comparing unrelated pointers
+    assert(s2 < s1 || s2 >= s1 + n);
+  nasty_char* r = s1;
+  for (; n; --n, ++s1, ++s2)
+    assign(*s1, *s2);
+  return r;
+}
+
+constexpr nasty_char* nasty_char_traits::assign(nasty_char* s, size_t n, nasty_char a) {
+  nasty_char* r = s;
+  for (; n; --n, ++s)
+    assign(*s, a);
+  return r;
+}
+
+using nasty_string = std::basic_string<nasty_char, nasty_char_traits>;
+
+template <size_t N>
+struct ToNastyChar {
+  constexpr ToNastyChar(const char (&r)[N]) {
+    std::transform(r, r + N, std::addressof(text[0]), [](char c) { return nasty_char{c}; });
+  }
+  nasty_char text[N];
+};
+
+template <size_t N>
+ToNastyChar(const char (&)[N]) -> ToNastyChar<N>;
+
+template <ToNastyChar t>
+constexpr auto to_nasty_char() {
+  return t;
+}
+
+// A macro like MAKE_CSTRING
+//
+// The 
diff erence is this macro can convert the nasty_char too.
+//
+// The lambda is a template, so the 'if constexpr' false branch is not evaluated for the nasty_char.
+#  define CONVERT_TO_CSTRING(CHAR, STR)                                                                                \
+    []<class CharT> {                                                                                                  \
+      if constexpr (std::is_same_v<CharT, nasty_char>) {                                                               \
+        static constexpr auto result = to_nasty_char<STR>();                                                           \
+        return result.text;                                                                                            \
+      } else                                                                                                           \
+        return MAKE_CSTRING(CharT, STR);                                                                               \
+    }.template operator()<CHAR>() /* */
+#else                             // TEST_HAS_NO_NASTY_STRING
+#  define CONVERT_TO_CSTRING(CharT, STR) MAKE_CSTRING(CharT, STR)
+#endif                            // TEST_HAS_NO_NASTY_STRING
+
+#endif                            // TEST_SUPPORT_NASTY_STRING_H


        


More information about the libcxx-commits mailing list