Skip to content

[libc++] Fix basic_string not allowing max_size() elements to be stored #125423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions libcxx/include/string
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is an observable change I'd like a note in the release notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug fix as every other one is, and we usually don't release note them. What's different here?

}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//

// XFAIL: FROZEN-CXX03-HEADERS-FIXME

// <string>

// This test ensures that the correct max_size() is returned depending on the platform.
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//

// XFAIL: FROZEN-CXX03-HEADERS-FIXME

// <string>

// basic_string<charT,traits,Allocator>&
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading