Skip to content

Commit 6454700

Browse files
jyknightcopybara-github
authored andcommitted
[libcxx] Don't deallocate non-pointer data in string assignment. (#67200)
Previously, assignment to a std::basic_string type with a _custom_ allocator could under certain conditions attempt to interpret part of the target string's "short" string-content as if it was a "long" data pointer, and attempt to deallocate a garbage value. This is a serious bug, but code in which it might happen is rare. It required: 1. the basic_string must be using a custom allocator type which sets the propagate_on_container_copy_assignment trait to true (thus, it does not affect the default allocator, nor most custom allocators). 2. the allocator for the target string must compare not equal to the allocator for the source string (many allocators always compare equal). 3. the source of the copy must currently contain a "long" string, and the assignment-target must currently contain a "short" string. Finally, the issue would've typically been innocuous when the bytes misinterpreted as a pointer were all zero, as deallocating a nullptr is typically a no-op. This is why existing test cases did not exhibit an issue: they were all zero-length strings, which do not have data in the bytes interpreted as a pointer. NOKEYCHECK=True GitOrigin-RevId: b0e19cfb6205d7cf36aa0fc3cb395205f7dc36ba
1 parent a75061b commit 6454700

File tree

2 files changed

+3
-1
lines changed

2 files changed

+3
-1
lines changed

include/string

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1969,7 +1969,8 @@ private:
19691969
allocator_type __a = __str.__alloc();
19701970
auto __allocation = std::__allocate_at_least(__a, __str.__get_long_cap());
19711971
__begin_lifetime(__allocation.ptr, __allocation.count);
1972-
__alloc_traits::deallocate(__alloc(), __get_long_pointer(), __get_long_cap());
1972+
if (__is_long())
1973+
__alloc_traits::deallocate(__alloc(), __get_long_pointer(), __get_long_cap());
19731974
__alloc() = std::move(__a);
19741975
__set_long_pointer(__allocation.ptr);
19751976
__set_long_cap(__allocation.count);

test/std/strings/basic.string/string.modifiers/string_assign/string.pass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ TEST_CONSTEXPR_CXX20 bool test() {
9090
testAlloc(S(A(5)), S("1"), A());
9191
testAlloc(S(A(5)), S("1", A(7)), A(7));
9292
testAlloc(S(A(5)), S("1234567890123456789012345678901234567890123456789012345678901234567890", A(7)), A(7));
93+
testAlloc(S("12345678901234567890", A(5)), S("1234567890123456789012345678901234567890123456789012345678901234567890", A(7)), A(7));
9394
}
9495

9596
#if TEST_STD_VER >= 11

0 commit comments

Comments
 (0)