Skip to content

Commit 1586044

Browse files
authored
[libc++] Fix basic_string not allowing max_size() elements to be stored (#125423)
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.
1 parent 303825d commit 1586044

File tree

11 files changed

+189
-53
lines changed

11 files changed

+189
-53
lines changed

libcxx/include/string

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,12 +1261,20 @@ public:
12611261
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type length() const _NOEXCEPT { return size(); }
12621262

12631263
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type max_size() const _NOEXCEPT {
1264-
size_type __m = __alloc_traits::max_size(__alloc_);
1265-
if (__m <= std::numeric_limits<size_type>::max() / 2) {
1266-
return __m - __alignment;
1264+
if (size_type __m = __alloc_traits::max_size(__alloc_); __m <= std::numeric_limits<size_type>::max() / 2) {
1265+
size_type __res = __m - __alignment;
1266+
1267+
// When the __endian_factor == 2, our string representation assumes that the capacity
1268+
// (including the null terminator) is always even, so we have to make sure the lowest bit isn't set when the
1269+
// string grows to max_size()
1270+
if (__endian_factor == 2)
1271+
__res &= ~size_type(1);
1272+
1273+
// We have to allocate space for the null terminator, but max_size() doesn't include it.
1274+
return __res - 1;
12671275
} else {
12681276
bool __uses_lsb = __endian_factor == 2;
1269-
return __uses_lsb ? __m - __alignment : (__m / 2) - __alignment;
1277+
return __uses_lsb ? __m - __alignment - 1 : (__m / 2) - __alignment - 1;
12701278
}
12711279
}
12721280

@@ -2607,11 +2615,11 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
26072615
size_type __n_add,
26082616
const value_type* __p_new_stuff) {
26092617
size_type __ms = max_size();
2610-
if (__delta_cap > __ms - __old_cap - 1)
2611-
this->__throw_length_error();
2618+
if (__delta_cap > __ms - __old_cap)
2619+
__throw_length_error();
26122620
pointer __old_p = __get_pointer();
26132621
size_type __cap =
2614-
__old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
2622+
__old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
26152623
__annotate_delete();
26162624
auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
26172625
auto __allocation = std::__allocate_at_least(__alloc_, __cap + 1);
@@ -2654,7 +2662,7 @@ _LIBCPP_DEPRECATED_("use __grow_by_without_replace") basic_string<_CharT, _Trait
26542662
this->__throw_length_error();
26552663
pointer __old_p = __get_pointer();
26562664
size_type __cap =
2657-
__old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
2665+
__old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
26582666
auto __allocation = std::__allocate_at_least(__alloc_, __cap + 1);
26592667
pointer __p = __allocation.ptr;
26602668
__begin_lifetime(__p, __allocation.count);

libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
10+
911
// <string>
1012

1113
// 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;
2325
template <class = int>
2426
TEST_CONSTEXPR_CXX20 void full_size() {
2527
std::string str;
26-
assert(str.max_size() == std::numeric_limits<std::size_t>::max() - alignment);
28+
assert(str.max_size() == std::numeric_limits<std::size_t>::max() - alignment - 1);
2729

2830
#ifndef TEST_HAS_NO_CHAR8_T
2931
std::u8string u8str;
30-
assert(u8str.max_size() == std::numeric_limits<std::size_t>::max() - alignment);
32+
assert(u8str.max_size() == std::numeric_limits<std::size_t>::max() - alignment - 1);
3133
#endif
3234

3335
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
3436
std::wstring wstr;
35-
assert(wstr.max_size() == std::numeric_limits<std::size_t>::max() / sizeof(wchar_t) - alignment);
37+
assert(wstr.max_size() ==
38+
((std::numeric_limits<std::size_t>::max() / sizeof(wchar_t) - alignment) & ~std::size_t(1)) - 1);
3639
#endif
3740

3841
std::u16string u16str;
3942
std::u32string u32str;
40-
assert(u16str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment);
41-
assert(u32str.max_size() == std::numeric_limits<std::size_t>::max() / 4 - alignment);
43+
assert(u16str.max_size() == ((std::numeric_limits<std::size_t>::max() / 2 - alignment) & ~std::size_t(1)) - 1);
44+
assert(u32str.max_size() == ((std::numeric_limits<std::size_t>::max() / 4 - alignment) & ~std::size_t(1)) - 1);
4245
}
4346

4447
template <class = int>
4548
TEST_CONSTEXPR_CXX20 void half_size() {
4649
std::string str;
47-
assert(str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment);
50+
assert(str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment - 1);
4851

4952
#ifndef TEST_HAS_NO_CHAR8_T
5053
std::u8string u8str;
51-
assert(u8str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment);
54+
assert(u8str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment - 1);
5255
#endif
5356

5457
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
5558
std::wstring wstr;
5659
assert(wstr.max_size() ==
57-
std::numeric_limits<std::size_t>::max() / std::max<size_t>(2ul, sizeof(wchar_t)) - alignment);
60+
std::numeric_limits<std::size_t>::max() / std::max<size_t>(2ul, sizeof(wchar_t)) - alignment - 1);
5861
#endif
5962

6063
std::u16string u16str;
6164
std::u32string u32str;
62-
assert(u16str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment);
63-
assert(u32str.max_size() == std::numeric_limits<std::size_t>::max() / 4 - alignment);
65+
assert(u16str.max_size() == std::numeric_limits<std::size_t>::max() / 2 - alignment - 1);
66+
assert(u32str.max_size() == std::numeric_limits<std::size_t>::max() / 4 - alignment - 1);
6467
}
6568

6669
TEST_CONSTEXPR_CXX20 bool test() {

libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
// UNSUPPORTED: no-exceptions
1010

11+
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
12+
1113
// After changing the alignment of the allocated pointer from 16 to 8, the exception
1214
// thrown is no longer `bad_alloc` but instead length_error on systems using new
1315
// headers but a dylib that doesn't contain 04ce0ba.
@@ -53,7 +55,7 @@ TEST_CONSTEXPR_CXX20 void test_resize_max_size(const S& s) {
5355
} catch (const std::bad_alloc&) {
5456
return;
5557
}
56-
assert(s.size() == sz);
58+
assert(s2.size() == sz);
5759
}
5860

5961
template <class S>
@@ -91,8 +93,16 @@ TEST_CONSTEXPR_CXX20 bool test() {
9193
test_string<std::string>();
9294
#if TEST_STD_VER >= 11
9395
test_string<std::basic_string<char, std::char_traits<char>, min_allocator<char> > >();
96+
test_string<std::basic_string<char, std::char_traits<char>, tiny_size_allocator<64, char> > >();
9497
#endif
9598

99+
{ // Test resizing where we can assume that the allocation succeeds
100+
std::basic_string<char, std::char_traits<char>, tiny_size_allocator<32, char> > str;
101+
auto max_size = str.max_size();
102+
str.resize(max_size);
103+
assert(str.size() == max_size);
104+
}
105+
96106
return true;
97107
}
98108

libcxx/test/std/strings/basic.string/string.capacity/resize_size.pass.cpp

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,56 +14,73 @@
1414
#include <stdexcept>
1515
#include <cassert>
1616

17-
#include "test_macros.h"
18-
#include "min_allocator.h"
1917
#include "asan_testing.h"
18+
#include "make_string.h"
19+
#include "min_allocator.h"
20+
#include "test_macros.h"
2021

2122
template <class S>
2223
TEST_CONSTEXPR_CXX20 void test(S s, typename S::size_type n, S expected) {
23-
if (n <= s.max_size()) {
24-
s.resize(n);
25-
LIBCPP_ASSERT(s.__invariants());
26-
assert(s == expected);
27-
LIBCPP_ASSERT(is_string_asan_correct(s));
24+
s.resize(n);
25+
LIBCPP_ASSERT(s.__invariants());
26+
assert(s == expected);
27+
LIBCPP_ASSERT(is_string_asan_correct(s));
28+
}
29+
30+
template <class CharT, class Alloc>
31+
TEST_CONSTEXPR_CXX20 void test_string() {
32+
{
33+
using string_type = std::basic_string<CharT, std::char_traits<CharT>, Alloc>;
34+
test(string_type(), 0, string_type());
35+
test(string_type(), 1, string_type(1, '\0'));
36+
test(string_type(), 10, string_type(10, '\0'));
37+
test(string_type(), 100, string_type(100, '\0'));
38+
test(string_type(MAKE_CSTRING(CharT, "12345")), 0, string_type());
39+
test(string_type(MAKE_CSTRING(CharT, "12345")), 2, string_type(MAKE_CSTRING(CharT, "12")));
40+
test(string_type(MAKE_CSTRING(CharT, "12345")), 5, string_type(MAKE_CSTRING(CharT, "12345")));
41+
test(string_type(MAKE_CSTRING(CharT, "12345")),
42+
15,
43+
string_type(MAKE_CSTRING(CharT, "12345\0\0\0\0\0\0\0\0\0\0"), 15));
44+
test(string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")), 0, string_type());
45+
test(string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")),
46+
10,
47+
string_type(MAKE_CSTRING(CharT, "1234567890")));
48+
test(string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")),
49+
50,
50+
string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")));
51+
test(
52+
string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890")),
53+
60,
54+
string_type(MAKE_CSTRING(CharT, "12345678901234567890123456789012345678901234567890\0\0\0\0\0\0\0\0\0\0"), 60));
2855
}
56+
2957
#ifndef TEST_HAS_NO_EXCEPTIONS
30-
else if (!TEST_IS_CONSTANT_EVALUATED) {
58+
if (!TEST_IS_CONSTANT_EVALUATED) {
59+
std::basic_string<CharT, std::char_traits<CharT>, Alloc> str;
3160
try {
32-
s.resize(n);
61+
str.resize(std::string::npos);
3362
assert(false);
34-
} catch (std::length_error&) {
35-
assert(n > s.max_size());
63+
} catch (const std::length_error&) {
3664
}
3765
}
3866
#endif
39-
}
4067

41-
template <class S>
42-
TEST_CONSTEXPR_CXX20 void test_string() {
43-
test(S(), 0, S());
44-
test(S(), 1, S(1, '\0'));
45-
test(S(), 10, S(10, '\0'));
46-
test(S(), 100, S(100, '\0'));
47-
test(S("12345"), 0, S());
48-
test(S("12345"), 2, S("12"));
49-
test(S("12345"), 5, S("12345"));
50-
test(S("12345"), 15, S("12345\0\0\0\0\0\0\0\0\0\0", 15));
51-
test(S("12345678901234567890123456789012345678901234567890"), 0, S());
52-
test(S("12345678901234567890123456789012345678901234567890"), 10, S("1234567890"));
53-
test(S("12345678901234567890123456789012345678901234567890"),
54-
50,
55-
S("12345678901234567890123456789012345678901234567890"));
56-
test(S("12345678901234567890123456789012345678901234567890"),
57-
60,
58-
S("12345678901234567890123456789012345678901234567890\0\0\0\0\0\0\0\0\0\0", 60));
59-
test(S(), S::npos, S("not going to happen"));
68+
{ // check that string can grow to max_size()
69+
std::basic_string<CharT, std::char_traits<CharT>, tiny_size_allocator<32, CharT> > str;
70+
str.resize(str.max_size());
71+
assert(str.size() == str.max_size());
72+
}
6073
}
6174

6275
TEST_CONSTEXPR_CXX20 bool test() {
63-
test_string<std::string>();
76+
test_string<char, std::allocator<char> >();
6477
#if TEST_STD_VER >= 11
65-
test_string<std::basic_string<char, std::char_traits<char>, min_allocator<char>>>();
66-
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char>>>();
78+
test_string<char, min_allocator<char>>();
79+
test_string<char, safe_allocator<char>>();
80+
#endif
81+
82+
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
83+
test_string<wchar_t, std::allocator<wchar_t> >();
6784
#endif
6885

6986
return true;

libcxx/test/std/strings/basic.string/string.modifiers/string_append/pointer_size.pass.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
10+
911
// <string>
1012

1113
// basic_string<charT,traits,Allocator>&
@@ -87,6 +89,16 @@ TEST_CONSTEXPR_CXX20 bool test() {
8789
assert(s_long == "Lorem ipsum dolor sit amet, consectetur/Lorem ipsum dolor sit amet, consectetur/");
8890
}
8991

92+
{ // check that growing to max_size() works
93+
using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
94+
string_type str;
95+
auto max_size = str.max_size();
96+
str.resize(max_size / 2 + max_size % 2);
97+
str.append(str.c_str(), max_size / 2);
98+
assert(str.capacity() >= str.size());
99+
assert(str.size() == str.max_size());
100+
}
101+
90102
return true;
91103
}
92104

libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/char_string.pass.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,17 @@ TEST_CONSTEXPR_CXX20 bool test() {
5151
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char> > >();
5252
#endif
5353

54+
{ // check that growing to max_size() works
55+
using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
56+
string_type str;
57+
str.resize(str.max_size() - 1);
58+
string_type result = 'a' + str;
59+
60+
assert(result.size() == result.max_size());
61+
assert(result.front() == 'a');
62+
assert(result.capacity() <= result.get_allocator().max_size());
63+
}
64+
5465
return true;
5566
}
5667

libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/pointer_string.pass.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,17 @@ TEST_CONSTEXPR_CXX20 bool test() {
6161
test_string<std::basic_string<char, std::char_traits<char>, min_allocator<char>>>();
6262
#endif
6363

64+
{ // check that growing to max_size() works
65+
using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
66+
string_type str;
67+
str.resize(str.max_size() - 1);
68+
string_type result = "a" + str;
69+
70+
assert(result.size() == result.max_size());
71+
assert(result.front() == 'a');
72+
assert(result.capacity() <= result.get_allocator().max_size());
73+
}
74+
6475
return true;
6576
}
6677

libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_char.pass.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,17 @@ TEST_CONSTEXPR_CXX20 bool test() {
5151
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char> > >();
5252
#endif
5353

54+
{ // check that growing to max_size() works
55+
using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
56+
string_type str;
57+
str.resize(str.max_size() - 1);
58+
string_type result = str + 'a';
59+
60+
assert(result.size() == result.max_size());
61+
assert(result.back() == 'a');
62+
assert(result.capacity() <= result.get_allocator().max_size());
63+
}
64+
5465
return true;
5566
}
5667

libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_pointer.pass.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,17 @@ TEST_CONSTEXPR_CXX20 bool test() {
7676
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char> > >();
7777
#endif
7878

79+
{ // check that growing to max_size() works
80+
using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
81+
string_type str;
82+
str.resize(str.max_size() - 1);
83+
string_type result = str + "a";
84+
85+
assert(result.size() == result.max_size());
86+
assert(result.back() == 'a');
87+
assert(result.capacity() <= result.get_allocator().max_size());
88+
}
89+
7990
return true;
8091
}
8192

libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/string_string.pass.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,20 @@ TEST_CONSTEXPR_CXX20 bool test() {
8686
test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char>>>();
8787
#endif
8888

89+
{ // check that growing to max_size() works
90+
using string_type = std::basic_string<char, std::char_traits<char>, tiny_size_allocator<29, char> >;
91+
string_type lhs;
92+
lhs.resize(lhs.max_size() - 1);
93+
94+
string_type rhs = "a";
95+
96+
string_type result = lhs + rhs;
97+
98+
assert(result.size() == result.max_size());
99+
assert(result.back() == 'a');
100+
assert(result.capacity() <= result.get_allocator().max_size());
101+
}
102+
89103
return true;
90104
}
91105

0 commit comments

Comments
 (0)