[libcxx-commits] [libcxx] [libc++] Fix basic_string not allowing max_size() elements to be stored (PR #125423)
Nikolas Klauser via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Feb 22 13:42:12 PST 2025
https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/125423
>From dbdda777e24b118391f004e85dc8f3f62ed00b68 Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Sun, 2 Feb 2025 18:01:27 +0100
Subject: [PATCH] [libc++] Fix basic_string not allowing max_size() elements to
be stored
---
libcxx/include/string | 24 ++++--
.../string.capacity/max_size.pass.cpp | 23 ++---
.../string.capacity/max_size.pass.cpp | 12 ++-
.../string.capacity/resize_size.pass.cpp | 85 +++++++++++--------
.../string_append/pointer_size.pass.cpp | 12 +++
.../string_op+/char_string.pass.cpp | 11 +++
.../string_op+/pointer_string.pass.cpp | 11 +++
.../string_op+/string_char.pass.cpp | 11 +++
.../string_op+/string_pointer.pass.cpp | 11 +++
.../string_op+/string_string.pass.cpp | 14 +++
libcxx/test/support/min_allocator.h | 28 ++++++
11 files changed, 189 insertions(+), 53 deletions(-)
diff --git a/libcxx/include/string b/libcxx/include/string
index e2968621bb314..19f977ddf9f11 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1261,12 +1261,20 @@ public:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type length() const _NOEXCEPT { return size(); }
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type max_size() const _NOEXCEPT {
- size_type __m = __alloc_traits::max_size(__alloc_);
- if (__m <= std::numeric_limits<size_type>::max() / 2) {
- return __m - __alignment;
+ if (size_type __m = __alloc_traits::max_size(__alloc_); __m <= std::numeric_limits<size_type>::max() / 2) {
+ size_type __res = __m - __alignment;
+
+ // When the __endian_factor == 2, our string representation assumes that the capacity
+ // (including the null terminator) is always even, so we have to make sure the lowest bit isn't set when the
+ // string grows to max_size()
+ if (__endian_factor == 2)
+ __res &= ~size_type(1);
+
+ // We have to allocate space for the null terminator, but max_size() doesn't include it.
+ return __res - 1;
} else {
bool __uses_lsb = __endian_factor == 2;
- return __uses_lsb ? __m - __alignment : (__m / 2) - __alignment;
+ return __uses_lsb ? __m - __alignment - 1 : (__m / 2) - __alignment - 1;
}
}
@@ -2607,11 +2615,11 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
size_type __n_add,
const value_type* __p_new_stuff) {
size_type __ms = max_size();
- if (__delta_cap > __ms - __old_cap - 1)
- this->__throw_length_error();
+ if (__delta_cap > __ms - __old_cap)
+ __throw_length_error();
pointer __old_p = __get_pointer();
size_type __cap =
- __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
+ __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
__annotate_delete();
auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
auto __allocation = std::__allocate_at_least(__alloc_, __cap + 1);
@@ -2654,7 +2662,7 @@ _LIBCPP_DEPRECATED_("use __grow_by_without_replace") basic_string<_CharT, _Trait
this->__throw_length_error();
pointer __old_p = __get_pointer();
size_type __cap =
- __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
+ __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
auto __allocation = std::__allocate_at_least(__alloc_, __cap + 1);
pointer __p = __allocation.ptr;
__begin_lifetime(__p, __allocation.count);
diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
index 726570beb6d1a..2b5701ba1328a 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
@@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
// <string>
// This test ensures that the correct max_size() is returned depending on the platform.
@@ -23,44 +25,45 @@ static const std::size_t alignment = 8;
template <class = int>
TEST_CONSTEXPR_CXX20 void full_size() {
std::string str;
- assert(str.max_size() == std::numeric_limits<std::size_t>::max() - alignment);
+ assert(str.max_size() == std::numeric_limits<std::size_t>::max() - alignment - 1);
#ifndef TEST_HAS_NO_CHAR8_T
std::u8string u8str;
- assert(u8str.max_size() == std::numeric_limits<std::size_t>::max() - alignment);
+ assert(u8str.max_size() == std::numeric_limits<std::size_t>::max() - alignment - 1);
#endif
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
std::wstring wstr;
- assert(wstr.max_size() == std::numeric_limits<std::size_t>::max() / sizeof(wchar_t) - alignment);
+ assert(wstr.max_size() ==
+ ((std::numeric_limits<std::size_t>::max() / sizeof(wchar_t) - alignment) & ~std::size_t(-1)) - 1);
#endif
std::u16string u16str;
std::u32string u32str;
- assert(u16str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment);
- assert(u32str.max_size() == std::numeric_limits<std::size_t>::max() / 4 - alignment);
+ assert(u16str.max_size() == ((std::numeric_limits<std::size_t>::max() / 2 - alignment) & ~std::size_t(-1)) - 1);
+ assert(u32str.max_size() == ((std::numeric_limits<std::size_t>::max() / 4 - alignment) & ~std::size_t(-1)) - 1);
}
template <class = int>
TEST_CONSTEXPR_CXX20 void half_size() {
std::string str;
- assert(str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment);
+ assert(str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment - 1);
#ifndef TEST_HAS_NO_CHAR8_T
std::u8string u8str;
- assert(u8str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment);
+ assert(u8str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment - 1);
#endif
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
std::wstring wstr;
assert(wstr.max_size() ==
- std::numeric_limits<std::size_t>::max() / std::max<size_t>(2ul, sizeof(wchar_t)) - alignment);
+ std::numeric_limits<std::size_t>::max() / std::max<size_t>(2ul, sizeof(wchar_t)) - alignment - 1);
#endif
std::u16string u16str;
std::u32string u32str;
- assert(u16str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment);
- assert(u32str.max_size() == std::numeric_limits<std::size_t>::max() / 4 - alignment);
+ assert(u16str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment - 1);
+ assert(u32str.max_size() == std::numeric_limits<std::size_t>::max() / 4 - alignment - 1);
}
TEST_CONSTEXPR_CXX20 bool test() {
diff --git a/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp b/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
index b9ffffc0993af..f68f9e9d5fe29 100644
--- a/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
@@ -8,6 +8,8 @@
// UNSUPPORTED: no-exceptions
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
// After changing the alignment of the allocated pointer from 16 to 8, the exception
// thrown is no longer `bad_alloc` but instead length_error on systems using new
// headers but a dylib that doesn't contain 04ce0ba.
@@ -53,7 +55,7 @@ TEST_CONSTEXPR_CXX20 void test_resize_max_size(const S& s) {
} catch (const std::bad_alloc&) {
return;
}
- assert(s.size() == sz);
+ assert(s2.size() == sz);
}
template <class S>
@@ -91,8 +93,16 @@ TEST_CONSTEXPR_CXX20 bool test() {
test_string<std::string>();
#if TEST_STD_VER >= 11
test_string<std::basic_string<char, std::char_traits<char>, min_allocator<char> > >();
+ test_string<std::basic_string<char, std::char_traits<char>, tiny_size_allocator<64, char> > >();
#endif
+ { // Test resizing where we can assume that the allocation succeeds
+ std::basic_string<char, std::char_traits<char>, tiny_size_allocator<32, char> > str;
+ auto max_size = str.max_size();
+ str.resize(max_size);
+ assert(str.size() == max_size);
+ }
+
return true;
}
diff --git a/libcxx/test/std/strings/basic.string/string.capacity/resize_size.pass.cpp b/libcxx/test/std/strings/basic.string/string.capacity/resize_size.pass.cpp
index 7cf4b7ca3b6ef..e23b6cf40691f 100644
--- a/libcxx/test/std/strings/basic.string/string.capacity/resize_size.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.capacity/resize_size.pass.cpp
@@ -14,56 +14,73 @@
#include <stdexcept>
#include <cassert>
-#include "test_macros.h"
-#include "min_allocator.h"
#include "asan_testing.h"
+#include "make_string.h"
+#include "min_allocator.h"
+#include "test_macros.h"
template <class S>
TEST_CONSTEXPR_CXX20 void test(S s, typename S::size_type n, S expected) {
- if (n <= s.max_size()) {
- s.resize(n);
- LIBCPP_ASSERT(s.__invariants());
- assert(s == expected);
- LIBCPP_ASSERT(is_string_asan_correct(s));
+ s.resize(n);
+ LIBCPP_ASSERT(s.__invariants());
+ assert(s == expected);
+ LIBCPP_ASSERT(is_string_asan_correct(s));
+}
+
+template <class CharT, class Alloc>
+TEST_CONSTEXPR_CXX20 void test_string() {
+ {
+ using string_type = std::basic_string<CharT, std::char_traits<CharT>, Alloc>;
+ test(string_type(), 0, string_type());
+ test(string_type(), 1, string_type(1, '\0'));
+ test(string_type(), 10, string_type(10, '\0'));
+ test(string_type(), 100, string_type(100, '\0'));
+ test(string_type(MAKE_CSTRING(CharT, "12345")), 0, string_type());
+ test(string_type(MAKE_CSTRING(CharT, "12345")), 2, string_type(MAKE_CSTRING(CharT, "12")));
+ test(string_type(MAKE_CSTRING(CharT, "12345")), 5, string_type(MAKE_CSTRING(CharT, "12345")));
+ test(string_type(MAKE_CSTRING(CharT, "12345")),
+ 15,
+ string_type(MAKE_CSTRING(CharT, "12345\0\0\0\0\0\0\0\0\0\0"), 15));
+ test(string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")), 0, string_type());
+ test(string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")),
+ 10,
+ string_type(MAKE_CSTRING(CharT, "1234567890")));
+ test(string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")),
+ 50,
+ string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")));
+ test(
+ string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")),
+ 60,
+ string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890\0\0\0\0\0\0\0\0\0\0"), 60));
}
+
#ifndef TEST_HAS_NO_EXCEPTIONS
- else if (!TEST_IS_CONSTANT_EVALUATED) {
+ if (!TEST_IS_CONSTANT_EVALUATED) {
+ std::basic_string<CharT, std::char_traits<CharT>, Alloc> str;
try {
- s.resize(n);
+ str.resize(std::string::npos);
assert(false);
- } catch (std::length_error&) {
- assert(n > s.max_size());
+ } catch (const std::length_error&) {
}
}
#endif
-}
-template <class S>
-TEST_CONSTEXPR_CXX20 void test_string() {
- test(S(), 0, S());
- test(S(), 1, S(1, '\0'));
- test(S(), 10, S(10, '\0'));
- test(S(), 100, S(100, '\0'));
- test(S("12345"), 0, S());
- test(S("12345"), 2, S("12"));
- test(S("12345"), 5, S("12345"));
- test(S("12345"), 15, S("12345\0\0\0\0\0\0\0\0\0\0", 15));
- test(S("12345678901234567890123456789012345678901234567890"), 0, S());
- test(S("12345678901234567890123456789012345678901234567890"), 10, S("1234567890"));
- test(S("12345678901234567890123456789012345678901234567890"),
- 50,
- S("12345678901234567890123456789012345678901234567890"));
- test(S("12345678901234567890123456789012345678901234567890"),
- 60,
- S("12345678901234567890123456789012345678901234567890\0\0\0\0\0\0\0\0\0\0", 60));
- test(S(), S::npos, S("not going to happen"));
+ { // check that string can grow to max_size()
+ std::basic_string<CharT, std::char_traits<CharT>, tiny_size_allocator<32, CharT> > str;
+ str.resize(str.max_size());
+ assert(str.size() == str.max_size());
+ }
}
TEST_CONSTEXPR_CXX20 bool test() {
- test_string<std::string>();
+ test_string<char, std::allocator<char> >();
#if TEST_STD_VER >= 11
- test_string<std::basic_string<char, std::char_traits<char>, min_allocator<char>>>();
- test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char>>>();
+ test_string<char, min_allocator<char>>();
+ test_string<char, safe_allocator<char>>();
+#endif
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+ test_string<wchar_t, std::allocator<wchar_t> >();
#endif
return true;
diff --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_append/pointer_size.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_append/pointer_size.pass.cpp
index 41d4f1114c05b..425a481b71639 100644
--- a/libcxx/test/std/strings/basic.string/string.modifiers/string_append/pointer_size.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_append/pointer_size.pass.cpp
@@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
// <string>
// basic_string<charT,traits,Allocator>&
@@ -87,6 +89,16 @@ TEST_CONSTEXPR_CXX20 bool test() {
assert(s_long == "Lorem ipsum dolor sit amet, consectetur/Lorem ipsum dolor sit amet, consectetur/");
}
+ { // check that growing to max_size() works
+ using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
+ string_type str;
+ auto max_size = str.max_size();
+ str.resize(max_size / 2 + max_size % 2);
+ str.append(str.c_str(), max_size / 2);
+ assert(str.capacity() >= str.size());
+ assert(str.size() == str.max_size());
+ }
+
return true;
}
diff --git a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/char_string.pass.cpp b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/char_string.pass.cpp
index 68a250c593318..333787a0a0766 100644
--- a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/char_string.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/char_string.pass.cpp
@@ -51,6 +51,17 @@ TEST_CONSTEXPR_CXX20 bool test() {
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char> > >();
#endif
+ { // check that growing to max_size() works
+ using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
+ string_type str;
+ str.resize(str.max_size() - 1);
+ string_type result = 'a' + str;
+
+ assert(result.size() == result.max_size());
+ assert(result.front() == 'a');
+ assert(result.capacity() <= result.get_allocator().max_size());
+ }
+
return true;
}
diff --git a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/pointer_string.pass.cpp b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/pointer_string.pass.cpp
index 434f7292c149c..400b5165036ed 100644
--- a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/pointer_string.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/pointer_string.pass.cpp
@@ -61,6 +61,17 @@ TEST_CONSTEXPR_CXX20 bool test() {
test_string<std::basic_string<char, std::char_traits<char>, min_allocator<char>>>();
#endif
+ { // check that growing to max_size() works
+ using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
+ string_type str;
+ str.resize(str.max_size() - 1);
+ string_type result = "a" + str;
+
+ assert(result.size() == result.max_size());
+ assert(result.front() == 'a');
+ assert(result.capacity() <= result.get_allocator().max_size());
+ }
+
return true;
}
diff --git a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_char.pass.cpp b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_char.pass.cpp
index 5c3989ae304e9..20ac1fb4cc723 100644
--- a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_char.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_char.pass.cpp
@@ -51,6 +51,17 @@ TEST_CONSTEXPR_CXX20 bool test() {
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char> > >();
#endif
+ { // check that growing to max_size() works
+ using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
+ string_type str;
+ str.resize(str.max_size() - 1);
+ string_type result = str + 'a';
+
+ assert(result.size() == result.max_size());
+ assert(result.back() == 'a');
+ assert(result.capacity() <= result.get_allocator().max_size());
+ }
+
return true;
}
diff --git a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_pointer.pass.cpp b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_pointer.pass.cpp
index 589c737348a11..215a98223814b 100644
--- a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_pointer.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_pointer.pass.cpp
@@ -76,6 +76,17 @@ TEST_CONSTEXPR_CXX20 bool test() {
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char> > >();
#endif
+ { // check that growing to max_size() works
+ using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
+ string_type str;
+ str.resize(str.max_size() - 1);
+ string_type result = str + "a";
+
+ assert(result.size() == result.max_size());
+ assert(result.back() == 'a');
+ assert(result.capacity() <= result.get_allocator().max_size());
+ }
+
return true;
}
diff --git a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_string.pass.cpp b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_string.pass.cpp
index 2badb51753395..082b0007498f5 100644
--- a/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_string.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_string.pass.cpp
@@ -86,6 +86,20 @@ TEST_CONSTEXPR_CXX20 bool test() {
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char>>>();
#endif
+ { // check that growing to max_size() works
+ using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
+ string_type lhs;
+ lhs.resize(lhs.max_size() - 1);
+
+ string_type rhs = "a";
+
+ string_type result = lhs + rhs;
+
+ assert(result.size() == result.max_size());
+ assert(result.back() == 'a');
+ assert(result.capacity() <= result.get_allocator().max_size());
+ }
+
return true;
}
diff --git a/libcxx/test/support/min_allocator.h b/libcxx/test/support/min_allocator.h
index d3ee27a23bc89..0be7ee0554ca9 100644
--- a/libcxx/test/support/min_allocator.h
+++ b/libcxx/test/support/min_allocator.h
@@ -474,4 +474,32 @@ class safe_allocator {
TEST_CONSTEXPR_CXX20 friend bool operator!=(safe_allocator x, safe_allocator y) { return !(x == y); }
};
+template <std::size_t MaxSize, class T>
+struct tiny_size_allocator {
+ using value_type = T;
+ using size_type = unsigned;
+
+ template <class U>
+ struct rebind {
+ using other = tiny_size_allocator<MaxSize, T>;
+ };
+
+ tiny_size_allocator() = default;
+
+ template <class U>
+ TEST_CONSTEXPR_CXX20 tiny_size_allocator(tiny_size_allocator<MaxSize, U>) {}
+
+ TEST_CONSTEXPR_CXX20 T* allocate(std::size_t n) {
+ assert(n <= MaxSize);
+ return std::allocator<T>().allocate(n);
+ }
+
+ TEST_CONSTEXPR_CXX20 void deallocate(T* ptr, std::size_t n) { std::allocator<T>().deallocate(ptr, n); }
+
+ TEST_CONSTEXPR_CXX20 size_type max_size() const { return MaxSize; }
+
+ friend bool operator==(tiny_size_allocator, tiny_size_allocator) { return true; }
+ friend bool operator!=(tiny_size_allocator, tiny_size_allocator) { return false; }
+};
+
#endif // MIN_ALLOCATOR_H
More information about the libcxx-commits
mailing list