[libcxx-commits] [libcxx] 029cb8a - [libc++] Fix copy_backward for vector<bool> with small storage types (#131560)
via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Mar 19 08:58:05 PDT 2025
Author: Peng Liu
Date: 2025-03-19T11:58:02-04:00
New Revision: 029cb8aff1450ed6d69f671a5e5e3e226316b42a
URL: https://github.com/llvm/llvm-project/commit/029cb8aff1450ed6d69f671a5e5e3e226316b42a
DIFF: https://github.com/llvm/llvm-project/commit/029cb8aff1450ed6d69f671a5e5e3e226316b42a.diff
LOG: [libc++] Fix copy_backward for vector<bool> with small storage types (#131560)
This patch fixes an issue in libc++ where `std::copy_backward` and
`ranges::copy_backward` incorrectly copy `std::vector<bool>` with small
storage types (e.g., `uint8_t`, `uint16_t`). The problem arises from
flawed bit mask computations involving integral promotions and sign-bit
extension, leading to unintended zeroing of bits. This patch corrects
the bitwise operations to ensure correct bit-level copying.
Fixes #131718.
Added:
Modified:
libcxx/include/__algorithm/copy_backward.h
libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp
libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_backward.pass.cpp
Removed:
################################################################################
diff --git a/libcxx/include/__algorithm/copy_backward.h b/libcxx/include/__algorithm/copy_backward.h
index 02ffc14361e6a..9f890645a41d3 100644
--- a/libcxx/include/__algorithm/copy_backward.h
+++ b/libcxx/include/__algorithm/copy_backward.h
@@ -52,7 +52,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
diff erence_type __dn = std::min(static_cast<
diff erence_type>(__last.__ctz_), __n);
__n -= __dn;
unsigned __clz = __bits_per_word - __last.__ctz_;
- __storage_type __m = (~__storage_type(0) << (__last.__ctz_ - __dn)) & (~__storage_type(0) >> __clz);
+ __storage_type __m = std::__middle_mask<__storage_type>(__clz, __last.__ctz_ - __dn);
__storage_type __b = *__last.__seg_ & __m;
*__result.__seg_ &= ~__m;
*__result.__seg_ |= __b;
@@ -69,7 +69,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
__n -= __nw * __bits_per_word;
// do last word
if (__n > 0) {
- __storage_type __m = ~__storage_type(0) << (__bits_per_word - __n);
+ __storage_type __m = std::__leading_mask<__storage_type>(__bits_per_word - __n);
__storage_type __b = *--__last.__seg_ & __m;
*--__result.__seg_ &= ~__m;
*__result.__seg_ |= __b;
@@ -94,12 +94,12 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
diff erence_type __dn = std::min(static_cast<
diff erence_type>(__last.__ctz_), __n);
__n -= __dn;
unsigned __clz_l = __bits_per_word - __last.__ctz_;
- __storage_type __m = (~__storage_type(0) << (__last.__ctz_ - __dn)) & (~__storage_type(0) >> __clz_l);
+ __storage_type __m = std::__middle_mask<__storage_type>(__clz_l, __last.__ctz_ - __dn);
__storage_type __b = *__last.__seg_ & __m;
unsigned __clz_r = __bits_per_word - __result.__ctz_;
__storage_type __ddn = std::min(__dn, static_cast<
diff erence_type>(__result.__ctz_));
if (__ddn > 0) {
- __m = (~__storage_type(0) << (__result.__ctz_ - __ddn)) & (~__storage_type(0) >> __clz_r);
+ __m = std::__middle_mask<__storage_type>(__clz_r, __result.__ctz_ - __ddn);
*__result.__seg_ &= ~__m;
if (__result.__ctz_ > __last.__ctz_)
*__result.__seg_ |= __b << (__result.__ctz_ - __last.__ctz_);
@@ -112,7 +112,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
// __result.__ctz_ == 0
--__result.__seg_;
__result.__ctz_ = static_cast<unsigned>(-__dn & (__bits_per_word - 1));
- __m = ~__storage_type(0) << __result.__ctz_;
+ __m = std::__leading_mask<__storage_type>(__result.__ctz_);
*__result.__seg_ &= ~__m;
__last.__ctz_ -= __dn + __ddn;
*__result.__seg_ |= __b << (__result.__ctz_ - __last.__ctz_);
@@ -123,7 +123,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
// __result.__ctz_ != 0 || __n == 0
// do middle words
unsigned __clz_r = __bits_per_word - __result.__ctz_;
- __storage_type __m = ~__storage_type(0) >> __clz_r;
+ __storage_type __m = std::__trailing_mask<__storage_type>(__clz_r);
for (; __n >= __bits_per_word; __n -= __bits_per_word) {
__storage_type __b = *--__last.__seg_;
*__result.__seg_ &= ~__m;
@@ -133,11 +133,11 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
}
// do last word
if (__n > 0) {
- __m = ~__storage_type(0) << (__bits_per_word - __n);
+ __m = std::__leading_mask<__storage_type>(__bits_per_word - __n);
__storage_type __b = *--__last.__seg_ & __m;
__clz_r = __bits_per_word - __result.__ctz_;
__storage_type __dn = std::min(__n, static_cast<
diff erence_type>(__result.__ctz_));
- __m = (~__storage_type(0) << (__result.__ctz_ - __dn)) & (~__storage_type(0) >> __clz_r);
+ __m = std::__middle_mask<__storage_type>(__clz_r, __result.__ctz_ - __dn);
*__result.__seg_ &= ~__m;
*__result.__seg_ |= __b >> (__bits_per_word - __result.__ctz_);
__result.__ctz_ = static_cast<unsigned>(((-__dn & (__bits_per_word - 1)) + __result.__ctz_) % __bits_per_word);
@@ -146,7 +146,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
// __result.__ctz_ == 0
--__result.__seg_;
__result.__ctz_ = static_cast<unsigned>(-__n & (__bits_per_word - 1));
- __m = ~__storage_type(0) << __result.__ctz_;
+ __m = std::__leading_mask<__storage_type>(__result.__ctz_);
*__result.__seg_ &= ~__m;
*__result.__seg_ |= __b << (__result.__ctz_ - (__bits_per_word - __n - __dn));
}
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp
index 8a528a96f5294..def192d4d6637 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp
@@ -13,10 +13,13 @@
// constexpr OutIter // constexpr after C++17
// copy_backward(InIter first, InIter last, OutIter result);
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
#include <algorithm>
#include <cassert>
#include <vector>
+#include "sized_allocator.h"
#include "test_macros.h"
#include "test_iterators.h"
#include "type_algorithms.h"
@@ -111,6 +114,146 @@ TEST_CONSTEXPR_CXX20 bool test() {
assert(test_vector_bool(256));
}
+ // Validate std::copy_backward with std::vector<bool> iterators and custom storage types.
+ // Ensure that assigned bits hold the intended values, while unassigned bits stay unchanged.
+ // Related issue: https://github.com/llvm/llvm-project/issues/131718.
+ {
+ //// Tests for std::copy_backward with aligned bits
+
+ { // Test the first (partial) word for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(7, false, Alloc(1));
+ std::vector<bool, Alloc> out(8, true, Alloc(1));
+ std::copy_backward(in.begin(), in.begin() + 1, out.begin() + 1);
+ assert(out[0] == false);
+ for (std::size_t i = 1; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the last (partial) word for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(8, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(8, true, Alloc(1));
+ std::copy_backward(in.end() - 4, in.end(), out.end());
+ for (std::size_t i = 0; i < static_cast<std::size_t>(in.size() - 4); ++i)
+ assert(out[i] == true);
+ for (std::size_t i = in.size() + 4; i < out.size(); ++i)
+ assert(in[i] == out[i]);
+ }
+ { // Test the middle (whole) words for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(17, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(24, true, Alloc(1));
+ std::copy_backward(in.begin(), in.end(), out.begin() + in.size());
+ for (std::size_t i = 0; i < in.size(); ++i)
+ assert(in[i] == out[i]);
+ for (std::size_t i = in.size(); i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+
+ { // Test the first (partial) word for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(14, false, Alloc(1));
+ std::vector<bool, Alloc> out(16, true, Alloc(1));
+ std::copy_backward(in.begin(), in.begin() + 2, out.begin() + 2);
+ assert(out[0] == false);
+ assert(out[1] == false);
+ for (std::size_t i = 2; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the last (partial) word for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(16, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(16, true, Alloc(1));
+ std::copy_backward(in.end() - 8, in.end(), out.end());
+ for (std::size_t i = 0; i < static_cast<std::size_t>(in.size() - 8); ++i)
+ assert(out[i] == true);
+ for (std::size_t i = in.size() + 8; i < out.size(); ++i)
+ assert(in[i] == out[i]);
+ }
+ { // Test the middle (whole) words for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(34, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(48, true, Alloc(1));
+ std::copy_backward(in.begin(), in.end(), out.begin() + in.size());
+ for (std::size_t i = 0; i < in.size(); ++i)
+ assert(in[i] == out[i]);
+ for (std::size_t i = in.size(); i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+
+ //// Tests for std::copy_backward with unaligned bits
+
+ { // Test the first (partial) word for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(8, false, Alloc(1));
+ std::vector<bool, Alloc> out(8, true, Alloc(1));
+ std::copy_backward(in.begin(), in.begin() + 1, out.begin() + 1);
+ assert(out[0] == false);
+ for (std::size_t i = 1; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the last (partial) word for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(8, false, Alloc(1));
+ std::vector<bool, Alloc> out(8, true, Alloc(1));
+ std::copy_backward(in.end() - 1, in.end(), out.begin() + 1);
+ assert(out[0] == false);
+ for (std::size_t i = 1; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the middle (whole) words for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(16, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(17, true, Alloc(1));
+ std::copy_backward(in.begin(), in.end(), out.end());
+ assert(out[0] == true);
+ for (std::size_t i = 0; i < in.size(); ++i)
+ assert(in[i] == out[i + 1]);
+ }
+
+ { // Test the first (partial) word for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(16, false, Alloc(1));
+ std::vector<bool, Alloc> out(16, true, Alloc(1));
+ std::copy_backward(in.begin(), in.begin() + 2, out.begin() + 2);
+ assert(out[0] == false);
+ assert(out[1] == false);
+ for (std::size_t i = 2; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the last (partial) word for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(16, false, Alloc(1));
+ std::vector<bool, Alloc> out(16, true, Alloc(1));
+ std::copy_backward(in.end() - 2, in.end(), out.begin() + 2);
+ assert(out[0] == false);
+ assert(out[1] == false);
+ for (std::size_t i = 2; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the middle (whole) words for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(32, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(33, true, Alloc(1));
+ std::copy_backward(in.begin(), in.end(), out.end());
+ assert(out[0] == true);
+ for (std::size_t i = 0; i < in.size(); ++i)
+ assert(in[i] == out[i + 1]);
+ }
+ }
+
return true;
}
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_backward.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_backward.pass.cpp
index a7fa3db23e6ba..e7251ab905db5 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_backward.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_backward.pass.cpp
@@ -28,6 +28,7 @@
#include <vector>
#include "almost_satisfies_types.h"
+#include "sized_allocator.h"
#include "test_iterators.h"
#include "test_macros.h"
@@ -355,6 +356,146 @@ constexpr bool test() {
assert(test_vector_bool(199));
assert(test_vector_bool(256));
}
+
+ // Validate std::ranges::copy_backward with std::vector<bool> iterators and custom storage types.
+ // Ensure that assigned bits hold the intended values, while unassigned bits stay unchanged.
+ // Related issue: https://github.com/llvm/llvm-project/issues/131718.
+ {
+ //// Tests for std::ranges::copy_backward with aligned bits
+
+ { // Test the first (partial) word for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(7, false, Alloc(1));
+ std::vector<bool, Alloc> out(8, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.begin() + 1), out.begin() + 1);
+ assert(out[0] == false);
+ for (std::size_t i = 1; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the last (partial) word for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(8, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(8, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.end() - 4, in.end()), out.end());
+ for (std::size_t i = 0; i < static_cast<std::size_t>(in.size() - 4); ++i)
+ assert(out[i] == true);
+ for (std::size_t i = in.size() + 4; i < out.size(); ++i)
+ assert(in[i] == out[i]);
+ }
+ { // Test the middle (whole) words for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(17, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(24, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.end()), out.begin() + in.size());
+ for (std::size_t i = 0; i < in.size(); ++i)
+ assert(in[i] == out[i]);
+ for (std::size_t i = in.size(); i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+
+ { // Test the first (partial) word for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(14, false, Alloc(1));
+ std::vector<bool, Alloc> out(16, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.begin() + 2), out.begin() + 2);
+ assert(out[0] == false);
+ assert(out[1] == false);
+ for (std::size_t i = 2; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the last (partial) word for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(16, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(16, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.end() - 8, in.end()), out.end());
+ for (std::size_t i = 0; i < static_cast<std::size_t>(in.size() - 8); ++i)
+ assert(out[i] == true);
+ for (std::size_t i = in.size() + 8; i < out.size(); ++i)
+ assert(in[i] == out[i]);
+ }
+ { // Test the middle (whole) words for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(34, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(48, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.end()), out.begin() + in.size());
+ for (std::size_t i = 0; i < in.size(); ++i)
+ assert(in[i] == out[i]);
+ for (std::size_t i = in.size(); i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+
+ //// Tests for std::ranges::copy_backward with unaligned bits
+
+ { // Test the first (partial) word for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(8, false, Alloc(1));
+ std::vector<bool, Alloc> out(8, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.begin() + 1), out.begin() + 1);
+ assert(out[0] == false);
+ for (std::size_t i = 1; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the last (partial) word for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(8, false, Alloc(1));
+ std::vector<bool, Alloc> out(8, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.end() - 1, in.end()), out.begin() + 1);
+ assert(out[0] == false);
+ for (std::size_t i = 1; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the middle (whole) words for uint8_t
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(16, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(17, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.end()), out.end());
+ assert(out[0] == true);
+ for (std::size_t i = 0; i < in.size(); ++i)
+ assert(in[i] == out[i + 1]);
+ }
+
+ { // Test the first (partial) word for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(16, false, Alloc(1));
+ std::vector<bool, Alloc> out(16, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.begin() + 2), out.begin() + 2);
+ assert(out[0] == false);
+ assert(out[1] == false);
+ for (std::size_t i = 2; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the last (partial) word for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(16, false, Alloc(1));
+ std::vector<bool, Alloc> out(16, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.end() - 2, in.end()), out.begin() + 2);
+ assert(out[0] == false);
+ assert(out[1] == false);
+ for (std::size_t i = 2; i < out.size(); ++i)
+ assert(out[i] == true);
+ }
+ { // Test the middle (whole) words for uint16_t
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(32, false, Alloc(1));
+ for (std::size_t i = 0; i < in.size(); i += 2)
+ in[i] = true;
+ std::vector<bool, Alloc> out(33, true, Alloc(1));
+ std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.end()), out.end());
+ assert(out[0] == true);
+ for (std::size_t i = 0; i < in.size(); ++i)
+ assert(in[i] == out[i + 1]);
+ }
+ }
#endif
return true;
More information about the libcxx-commits
mailing list