[libcxx-commits] [libcxx] [libc++] Fix basic_string not allowing max_size() elements to be stored (PR #125423)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Feb 2 09:08:05 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Nikolas Klauser (philnik777)
<details>
<summary>Changes</summary>
Without this patch `basic_string` cannot be properly resized to be `max_size()` elements in size, even if an allocation is successful. `__grow_by` allocates one less element than required, resulting in an out-of-bounds access. At the same time, `max_size()` has an off-by-one error, since there has to be space to store the null terminator, which is currently ignored.
---
Full diff: https://github.com/llvm/llvm-project/pull/125423.diff
5 Files Affected:
- (modified) libcxx/include/string (+3-3)
- (modified) libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp (+10-10)
- (modified) libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp (+9-1)
- (modified) libcxx/test/std/strings/basic.string/string.capacity/resize_size.pass.cpp (+27-17)
- (modified) libcxx/test/support/min_allocator.h (+28)
``````````diff
diff --git a/libcxx/include/string b/libcxx/include/string
index fdd8085106dcc6..892ae97518f719 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1305,10 +1305,10 @@ public:
_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;
+ return __m - __alignment - 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;
}
}
@@ -2558,7 +2558,7 @@ _LIBCPP_DEPRECATED_("use __grow_by_without_replace") basic_string<_CharT, _Trait
__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 726570beb6d1ae..e097c70ec79a7c 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
@@ -23,44 +23,44 @@ 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 - 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);
}
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 b9ffffc0993af8..ff38a59cb00e58 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
@@ -53,7 +53,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 +91,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 7cf4b7ca3b6efd..99b654cf6b6a51 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
@@ -20,22 +20,10 @@
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));
- }
-#ifndef TEST_HAS_NO_EXCEPTIONS
- else if (!TEST_IS_CONSTANT_EVALUATED) {
- try {
- s.resize(n);
- assert(false);
- } catch (std::length_error&) {
- assert(n > s.max_size());
- }
- }
-#endif
+ s.resize(n);
+ LIBCPP_ASSERT(s.__invariants());
+ assert(s == expected);
+ LIBCPP_ASSERT(is_string_asan_correct(s));
}
template <class S>
@@ -56,7 +44,26 @@ TEST_CONSTEXPR_CXX20 void test_string() {
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"));
+}
+
+template <class CharT>
+TEST_CONSTEXPR_CXX20 void test_max_size() {
+#ifndef TEST_HAS_NO_EXCEPTIONS
+ if (!TEST_IS_CONSTANT_EVALUATED) {
+ std::basic_string<CharT> str;
+ try {
+ str.resize(std::string::npos);
+ assert(false);
+ } catch (const std::length_error&) {
+ }
+ }
+#endif
+
+ {
+ 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() {
@@ -66,6 +73,9 @@ TEST_CONSTEXPR_CXX20 bool test() {
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char>>>();
#endif
+ test_max_size<char>();
+ test_max_size<wchar_t>();
+
return true;
}
diff --git a/libcxx/test/support/min_allocator.h b/libcxx/test/support/min_allocator.h
index 18f51f8072640d..f050267dc4ef75 100644
--- a/libcxx/test/support/min_allocator.h
+++ b/libcxx/test/support/min_allocator.h
@@ -480,4 +480,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
``````````
</details>
https://github.com/llvm/llvm-project/pull/125423
More information about the libcxx-commits
mailing list