diff --git a/libcxx/include/string b/libcxx/include/string index 3f43e8fd8d586..fcb7405c1d086 100644 --- a/libcxx/include/string +++ b/libcxx/include/string @@ -3495,7 +3495,7 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat // The Standard mandates shrink_to_fit() does not increase the capacity. // With equal capacity keep the existing buffer. This avoids extra work // due to swapping the elements. - if (__allocation.count - 1 > capacity()) { + if (__allocation.count - 1 >= capacity()) { __alloc_traits::deallocate(__alloc_, __allocation.ptr, __allocation.count); return; } diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp index 73b70d6f10bd5..101838e8f525e 100644 --- a/libcxx/test/libcxx/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp +++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp @@ -12,12 +12,12 @@ // void shrink_to_fit(); // constexpr since C++20 -// Make sure we use an allocation returned by allocate_at_least if it is smaller than the current allocation -// even if it contains more bytes than we requested - #include #include +#include "asan_testing.h" +#include "increasing_allocator.h" + template struct oversizing_allocator { using value_type = T; @@ -37,6 +37,9 @@ bool operator==(oversizing_allocator, oversizing_allocator) { return true; } +// Make sure we use an allocation returned by allocate_at_least if it is smaller than the current allocation +// even if it contains more bytes than we requested. +// Fix issue: https://github.com/llvm/llvm-project/pull/115659 void test_oversizing_allocator() { std::basic_string, oversizing_allocator> s{ "String does not fit in the internal buffer and is a bit longer"}; @@ -48,8 +51,37 @@ void test_oversizing_allocator() { assert(s.size() == size); } +// Make sure libc++ shrink_to_fit does NOT swap buffer with equal allocation sizes +void test_no_swap_with_equal_allocation_size() { + { // Test with custom allocator with a minimum allocation size + std::basic_string, min_size_allocator<128, char> > s( + "A long string exceeding SSO limit but within min alloc size"); + std::size_t capacity = s.capacity(); + std::size_t size = s.size(); + auto data = s.data(); + s.shrink_to_fit(); + assert(s.capacity() <= capacity); + assert(s.size() == size); + assert(is_string_asan_correct(s)); + assert(s.capacity() == capacity && s.data() == data); + } + { // Test with custom allocator with a minimum power of two allocation size + std::basic_string, pow2_allocator > s( + "This is a long string that exceeds the SSO limit"); + std::size_t capacity = s.capacity(); + std::size_t size = s.size(); + auto data = s.data(); + s.shrink_to_fit(); + assert(s.capacity() <= capacity); + assert(s.size() == size); + assert(is_string_asan_correct(s)); + assert(s.capacity() == capacity && s.data() == data); + } +} + int main(int, char**) { test_oversizing_allocator(); + test_no_swap_with_equal_allocation_size(); return 0; } diff --git a/libcxx/test/std/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp b/libcxx/test/std/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp index 2d901e7afe2b6..9ca7825a4ba92 100644 --- a/libcxx/test/std/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp +++ b/libcxx/test/std/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp @@ -61,28 +61,25 @@ TEST_CONSTEXPR_CXX20 bool test() { test_string, safe_allocator>>(); #endif - return true; -} - #if TEST_STD_VER >= 23 -// https://github.com/llvm/llvm-project/issues/95161 -void test_increasing_allocator() { - std::basic_string, increasing_allocator> s{ - "String does not fit in the internal buffer"}; - std::size_t capacity = s.capacity(); - std::size_t size = s.size(); - s.shrink_to_fit(); - assert(s.capacity() <= capacity); - assert(s.size() == size); - LIBCPP_ASSERT(is_string_asan_correct(s)); + { // Make sure shrink_to_fit never increases capacity + // See: https://github.com/llvm/llvm-project/issues/95161 + std::basic_string, increasing_allocator> s{ + "String does not fit in the internal buffer"}; + std::size_t capacity = s.capacity(); + std::size_t size = s.size(); + s.shrink_to_fit(); + assert(s.capacity() <= capacity); + assert(s.size() == size); + LIBCPP_ASSERT(is_string_asan_correct(s)); + } +#endif + + return true; } -#endif // TEST_STD_VER >= 23 int main(int, char**) { test(); -#if TEST_STD_VER >= 23 - test_increasing_allocator(); -#endif #if TEST_STD_VER > 17 static_assert(test()); #endif diff --git a/libcxx/test/support/increasing_allocator.h b/libcxx/test/support/increasing_allocator.h index 30bd6f40c8dad..7e5aeaf7188e6 100644 --- a/libcxx/test/support/increasing_allocator.h +++ b/libcxx/test/support/increasing_allocator.h @@ -10,6 +10,7 @@ #define TEST_SUPPORT_INCREASING_ALLOCATOR_H #include +#include #include #include "test_macros.h" @@ -49,4 +50,75 @@ TEST_CONSTEXPR_CXX20 bool operator==(increasing_allocator, increasing_allocat return true; } +template +class min_size_allocator { +public: + using value_type = T; + min_size_allocator() = default; + + template + TEST_CONSTEXPR_CXX20 min_size_allocator(const min_size_allocator&) TEST_NOEXCEPT {} + + TEST_NODISCARD TEST_CONSTEXPR_CXX20 T* allocate(std::size_t n) { + if (n < MinAllocSize) + n = MinAllocSize; + return static_cast(::operator new(n * sizeof(T))); + } + + TEST_CONSTEXPR_CXX20 void deallocate(T* p, std::size_t) TEST_NOEXCEPT { ::operator delete(static_cast(p)); } + + template + struct rebind { + using other = min_size_allocator; + }; +}; + +template +TEST_CONSTEXPR_CXX20 bool +operator==(const min_size_allocator&, const min_size_allocator&) { + return true; +} + +template +TEST_CONSTEXPR_CXX20 bool +operator!=(const min_size_allocator&, const min_size_allocator&) { + return false; +} + +template +class pow2_allocator { +public: + using value_type = T; + pow2_allocator() = default; + + template + TEST_CONSTEXPR_CXX20 pow2_allocator(const pow2_allocator&) TEST_NOEXCEPT {} + + TEST_NODISCARD TEST_CONSTEXPR_CXX20 T* allocate(std::size_t n) { + return static_cast(::operator new(next_power_of_two(n) * sizeof(T))); + } + + TEST_CONSTEXPR_CXX20 void deallocate(T* p, std::size_t) TEST_NOEXCEPT { ::operator delete(static_cast(p)); } + +private: + TEST_CONSTEXPR_CXX20 std::size_t next_power_of_two(std::size_t n) const { + if ((n & (n - 1)) == 0) + return n; + for (std::size_t shift = 1; shift < std::numeric_limits::digits; shift <<= 1) { + n |= n >> shift; + } + return n + 1; + } +}; + +template +TEST_CONSTEXPR_CXX20 bool operator==(const pow2_allocator&, const pow2_allocator&) { + return true; +} + +template +TEST_CONSTEXPR_CXX20 bool operator!=(const pow2_allocator&, const pow2_allocator&) { + return false; +} + #endif // TEST_SUPPORT_INCREASING_ALLOCATOR_H