Skip to content

Commit c0350c8

Browse files
committed
[libcxx] Fix basic_string<large value type> buffer replacement
`basic_string` stores a union `__rep` holding one of two structure types, one (`__long`) with a pointer to an allocated buffer, and one (`__short`) with a built-in buffer of at least 2 characters (or however many more will fit). If you make a `basic_string` with an oversized value type, then the two members of this union are not the same size, and in particular their `__is_long_` flags (stored at the end in `_LIBCPP_ABI_ALTERNATE_STRING_LAYOUT` mode) don't alias each other. Therefore, when the union is constructed from a `__long`, the `__is_long_` flag in the `__short` union member is not initialized at all. Unfortunately, that is where the `__is_long()` method looks. Commit 28d3194 recently had the side effect that if `__short` is longer than `__long`, the memory after the `__long` is now uninitialized rather than zeroed, which can lead to crashes due to having what looks like a short string with an out-of-range length. However, even before that commit, the code was still not updating the string correctly: it would instead have created a short string with _zero_ length, which is less crash-prone but still not the right value. The test `string.modifiers/string_append/push_back.pass.cpp` was testing this case by calling `push_back` three times on a `basic_string` with oversized value type, but without checking anything about the value of the resulting string. Adding some assertions that the string has the right length at every step would have caught this bug (both before and after the recent commit).
1 parent 1de55c9 commit c0350c8

File tree

2 files changed

+12
-1
lines changed

2 files changed

+12
-1
lines changed

libcxx/include/string

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,14 @@ private:
918918

919919
__rep() = default;
920920
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__short __r) : __s(__r) {}
921-
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__long __r) : __l(__r) {}
921+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__long __r) : __s() {
922+
// For nonstandard value_type, __s can be bigger than __l, because it
923+
// contains at least two value_type. So if we're storing a long buffer,
924+
// we must set the is_long flag in __s, where __is_long() will look, in
925+
// case it isn't set by assigning to __l.
926+
__s.__is_long_ = true;
927+
__l = __r;
928+
}
922929
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__uninitialized_tag) {}
923930
};
924931

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,13 @@ TEST_CONSTEXPR_CXX20 bool test() {
8484
// https://llvm.org/PR31454
8585
std::basic_string<VeryLarge> s;
8686
VeryLarge vl = {};
87+
LIBCPP_ASSERT(s.size() == 0);
8788
s.push_back(vl);
89+
LIBCPP_ASSERT(s.size() == 1);
8890
s.push_back(vl);
91+
LIBCPP_ASSERT(s.size() == 2);
8992
s.push_back(vl);
93+
LIBCPP_ASSERT(s.size() == 3);
9094
}
9195

9296
return true;

0 commit comments

Comments
 (0)