Skip to content

Commit 029cb8a

Browse files
authored
[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.
1 parent 1456eab commit 029cb8a

File tree

3 files changed

+293
-9
lines changed

3 files changed

+293
-9
lines changed

libcxx/include/__algorithm/copy_backward.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
5252
difference_type __dn = std::min(static_cast<difference_type>(__last.__ctz_), __n);
5353
__n -= __dn;
5454
unsigned __clz = __bits_per_word - __last.__ctz_;
55-
__storage_type __m = (~__storage_type(0) << (__last.__ctz_ - __dn)) & (~__storage_type(0) >> __clz);
55+
__storage_type __m = std::__middle_mask<__storage_type>(__clz, __last.__ctz_ - __dn);
5656
__storage_type __b = *__last.__seg_ & __m;
5757
*__result.__seg_ &= ~__m;
5858
*__result.__seg_ |= __b;
@@ -69,7 +69,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
6969
__n -= __nw * __bits_per_word;
7070
// do last word
7171
if (__n > 0) {
72-
__storage_type __m = ~__storage_type(0) << (__bits_per_word - __n);
72+
__storage_type __m = std::__leading_mask<__storage_type>(__bits_per_word - __n);
7373
__storage_type __b = *--__last.__seg_ & __m;
7474
*--__result.__seg_ &= ~__m;
7575
*__result.__seg_ |= __b;
@@ -94,12 +94,12 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
9494
difference_type __dn = std::min(static_cast<difference_type>(__last.__ctz_), __n);
9595
__n -= __dn;
9696
unsigned __clz_l = __bits_per_word - __last.__ctz_;
97-
__storage_type __m = (~__storage_type(0) << (__last.__ctz_ - __dn)) & (~__storage_type(0) >> __clz_l);
97+
__storage_type __m = std::__middle_mask<__storage_type>(__clz_l, __last.__ctz_ - __dn);
9898
__storage_type __b = *__last.__seg_ & __m;
9999
unsigned __clz_r = __bits_per_word - __result.__ctz_;
100100
__storage_type __ddn = std::min(__dn, static_cast<difference_type>(__result.__ctz_));
101101
if (__ddn > 0) {
102-
__m = (~__storage_type(0) << (__result.__ctz_ - __ddn)) & (~__storage_type(0) >> __clz_r);
102+
__m = std::__middle_mask<__storage_type>(__clz_r, __result.__ctz_ - __ddn);
103103
*__result.__seg_ &= ~__m;
104104
if (__result.__ctz_ > __last.__ctz_)
105105
*__result.__seg_ |= __b << (__result.__ctz_ - __last.__ctz_);
@@ -112,7 +112,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
112112
// __result.__ctz_ == 0
113113
--__result.__seg_;
114114
__result.__ctz_ = static_cast<unsigned>(-__dn & (__bits_per_word - 1));
115-
__m = ~__storage_type(0) << __result.__ctz_;
115+
__m = std::__leading_mask<__storage_type>(__result.__ctz_);
116116
*__result.__seg_ &= ~__m;
117117
__last.__ctz_ -= __dn + __ddn;
118118
*__result.__seg_ |= __b << (__result.__ctz_ - __last.__ctz_);
@@ -123,7 +123,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
123123
// __result.__ctz_ != 0 || __n == 0
124124
// do middle words
125125
unsigned __clz_r = __bits_per_word - __result.__ctz_;
126-
__storage_type __m = ~__storage_type(0) >> __clz_r;
126+
__storage_type __m = std::__trailing_mask<__storage_type>(__clz_r);
127127
for (; __n >= __bits_per_word; __n -= __bits_per_word) {
128128
__storage_type __b = *--__last.__seg_;
129129
*__result.__seg_ &= ~__m;
@@ -133,11 +133,11 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
133133
}
134134
// do last word
135135
if (__n > 0) {
136-
__m = ~__storage_type(0) << (__bits_per_word - __n);
136+
__m = std::__leading_mask<__storage_type>(__bits_per_word - __n);
137137
__storage_type __b = *--__last.__seg_ & __m;
138138
__clz_r = __bits_per_word - __result.__ctz_;
139139
__storage_type __dn = std::min(__n, static_cast<difference_type>(__result.__ctz_));
140-
__m = (~__storage_type(0) << (__result.__ctz_ - __dn)) & (~__storage_type(0) >> __clz_r);
140+
__m = std::__middle_mask<__storage_type>(__clz_r, __result.__ctz_ - __dn);
141141
*__result.__seg_ &= ~__m;
142142
*__result.__seg_ |= __b >> (__bits_per_word - __result.__ctz_);
143143
__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> _
146146
// __result.__ctz_ == 0
147147
--__result.__seg_;
148148
__result.__ctz_ = static_cast<unsigned>(-__n & (__bits_per_word - 1));
149-
__m = ~__storage_type(0) << __result.__ctz_;
149+
__m = std::__leading_mask<__storage_type>(__result.__ctz_);
150150
*__result.__seg_ &= ~__m;
151151
*__result.__seg_ |= __b << (__result.__ctz_ - (__bits_per_word - __n - __dn));
152152
}

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@
1313
// constexpr OutIter // constexpr after C++17
1414
// copy_backward(InIter first, InIter last, OutIter result);
1515

16+
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
17+
1618
#include <algorithm>
1719
#include <cassert>
1820
#include <vector>
1921

22+
#include "sized_allocator.h"
2023
#include "test_macros.h"
2124
#include "test_iterators.h"
2225
#include "type_algorithms.h"
@@ -111,6 +114,146 @@ TEST_CONSTEXPR_CXX20 bool test() {
111114
assert(test_vector_bool(256));
112115
}
113116

117+
// Validate std::copy_backward with std::vector<bool> iterators and custom storage types.
118+
// Ensure that assigned bits hold the intended values, while unassigned bits stay unchanged.
119+
// Related issue: https://github.com/llvm/llvm-project/issues/131718.
120+
{
121+
//// Tests for std::copy_backward with aligned bits
122+
123+
{ // Test the first (partial) word for uint8_t
124+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
125+
std::vector<bool, Alloc> in(7, false, Alloc(1));
126+
std::vector<bool, Alloc> out(8, true, Alloc(1));
127+
std::copy_backward(in.begin(), in.begin() + 1, out.begin() + 1);
128+
assert(out[0] == false);
129+
for (std::size_t i = 1; i < out.size(); ++i)
130+
assert(out[i] == true);
131+
}
132+
{ // Test the last (partial) word for uint8_t
133+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
134+
std::vector<bool, Alloc> in(8, false, Alloc(1));
135+
for (std::size_t i = 0; i < in.size(); i += 2)
136+
in[i] = true;
137+
std::vector<bool, Alloc> out(8, true, Alloc(1));
138+
std::copy_backward(in.end() - 4, in.end(), out.end());
139+
for (std::size_t i = 0; i < static_cast<std::size_t>(in.size() - 4); ++i)
140+
assert(out[i] == true);
141+
for (std::size_t i = in.size() + 4; i < out.size(); ++i)
142+
assert(in[i] == out[i]);
143+
}
144+
{ // Test the middle (whole) words for uint8_t
145+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
146+
std::vector<bool, Alloc> in(17, false, Alloc(1));
147+
for (std::size_t i = 0; i < in.size(); i += 2)
148+
in[i] = true;
149+
std::vector<bool, Alloc> out(24, true, Alloc(1));
150+
std::copy_backward(in.begin(), in.end(), out.begin() + in.size());
151+
for (std::size_t i = 0; i < in.size(); ++i)
152+
assert(in[i] == out[i]);
153+
for (std::size_t i = in.size(); i < out.size(); ++i)
154+
assert(out[i] == true);
155+
}
156+
157+
{ // Test the first (partial) word for uint16_t
158+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
159+
std::vector<bool, Alloc> in(14, false, Alloc(1));
160+
std::vector<bool, Alloc> out(16, true, Alloc(1));
161+
std::copy_backward(in.begin(), in.begin() + 2, out.begin() + 2);
162+
assert(out[0] == false);
163+
assert(out[1] == false);
164+
for (std::size_t i = 2; i < out.size(); ++i)
165+
assert(out[i] == true);
166+
}
167+
{ // Test the last (partial) word for uint16_t
168+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
169+
std::vector<bool, Alloc> in(16, false, Alloc(1));
170+
for (std::size_t i = 0; i < in.size(); i += 2)
171+
in[i] = true;
172+
std::vector<bool, Alloc> out(16, true, Alloc(1));
173+
std::copy_backward(in.end() - 8, in.end(), out.end());
174+
for (std::size_t i = 0; i < static_cast<std::size_t>(in.size() - 8); ++i)
175+
assert(out[i] == true);
176+
for (std::size_t i = in.size() + 8; i < out.size(); ++i)
177+
assert(in[i] == out[i]);
178+
}
179+
{ // Test the middle (whole) words for uint16_t
180+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
181+
std::vector<bool, Alloc> in(34, false, Alloc(1));
182+
for (std::size_t i = 0; i < in.size(); i += 2)
183+
in[i] = true;
184+
std::vector<bool, Alloc> out(48, true, Alloc(1));
185+
std::copy_backward(in.begin(), in.end(), out.begin() + in.size());
186+
for (std::size_t i = 0; i < in.size(); ++i)
187+
assert(in[i] == out[i]);
188+
for (std::size_t i = in.size(); i < out.size(); ++i)
189+
assert(out[i] == true);
190+
}
191+
192+
//// Tests for std::copy_backward with unaligned bits
193+
194+
{ // Test the first (partial) word for uint8_t
195+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
196+
std::vector<bool, Alloc> in(8, false, Alloc(1));
197+
std::vector<bool, Alloc> out(8, true, Alloc(1));
198+
std::copy_backward(in.begin(), in.begin() + 1, out.begin() + 1);
199+
assert(out[0] == false);
200+
for (std::size_t i = 1; i < out.size(); ++i)
201+
assert(out[i] == true);
202+
}
203+
{ // Test the last (partial) word for uint8_t
204+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
205+
std::vector<bool, Alloc> in(8, false, Alloc(1));
206+
std::vector<bool, Alloc> out(8, true, Alloc(1));
207+
std::copy_backward(in.end() - 1, in.end(), out.begin() + 1);
208+
assert(out[0] == false);
209+
for (std::size_t i = 1; i < out.size(); ++i)
210+
assert(out[i] == true);
211+
}
212+
{ // Test the middle (whole) words for uint8_t
213+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
214+
std::vector<bool, Alloc> in(16, false, Alloc(1));
215+
for (std::size_t i = 0; i < in.size(); i += 2)
216+
in[i] = true;
217+
std::vector<bool, Alloc> out(17, true, Alloc(1));
218+
std::copy_backward(in.begin(), in.end(), out.end());
219+
assert(out[0] == true);
220+
for (std::size_t i = 0; i < in.size(); ++i)
221+
assert(in[i] == out[i + 1]);
222+
}
223+
224+
{ // Test the first (partial) word for uint16_t
225+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
226+
std::vector<bool, Alloc> in(16, false, Alloc(1));
227+
std::vector<bool, Alloc> out(16, true, Alloc(1));
228+
std::copy_backward(in.begin(), in.begin() + 2, out.begin() + 2);
229+
assert(out[0] == false);
230+
assert(out[1] == false);
231+
for (std::size_t i = 2; i < out.size(); ++i)
232+
assert(out[i] == true);
233+
}
234+
{ // Test the last (partial) word for uint16_t
235+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
236+
std::vector<bool, Alloc> in(16, false, Alloc(1));
237+
std::vector<bool, Alloc> out(16, true, Alloc(1));
238+
std::copy_backward(in.end() - 2, in.end(), out.begin() + 2);
239+
assert(out[0] == false);
240+
assert(out[1] == false);
241+
for (std::size_t i = 2; i < out.size(); ++i)
242+
assert(out[i] == true);
243+
}
244+
{ // Test the middle (whole) words for uint16_t
245+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
246+
std::vector<bool, Alloc> in(32, false, Alloc(1));
247+
for (std::size_t i = 0; i < in.size(); i += 2)
248+
in[i] = true;
249+
std::vector<bool, Alloc> out(33, true, Alloc(1));
250+
std::copy_backward(in.begin(), in.end(), out.end());
251+
assert(out[0] == true);
252+
for (std::size_t i = 0; i < in.size(); ++i)
253+
assert(in[i] == out[i + 1]);
254+
}
255+
}
256+
114257
return true;
115258
}
116259

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_backward.pass.cpp

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <vector>
2929

3030
#include "almost_satisfies_types.h"
31+
#include "sized_allocator.h"
3132
#include "test_iterators.h"
3233
#include "test_macros.h"
3334

@@ -355,6 +356,146 @@ constexpr bool test() {
355356
assert(test_vector_bool(199));
356357
assert(test_vector_bool(256));
357358
}
359+
360+
// Validate std::ranges::copy_backward with std::vector<bool> iterators and custom storage types.
361+
// Ensure that assigned bits hold the intended values, while unassigned bits stay unchanged.
362+
// Related issue: https://github.com/llvm/llvm-project/issues/131718.
363+
{
364+
//// Tests for std::ranges::copy_backward with aligned bits
365+
366+
{ // Test the first (partial) word for uint8_t
367+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
368+
std::vector<bool, Alloc> in(7, false, Alloc(1));
369+
std::vector<bool, Alloc> out(8, true, Alloc(1));
370+
std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.begin() + 1), out.begin() + 1);
371+
assert(out[0] == false);
372+
for (std::size_t i = 1; i < out.size(); ++i)
373+
assert(out[i] == true);
374+
}
375+
{ // Test the last (partial) word for uint8_t
376+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
377+
std::vector<bool, Alloc> in(8, false, Alloc(1));
378+
for (std::size_t i = 0; i < in.size(); i += 2)
379+
in[i] = true;
380+
std::vector<bool, Alloc> out(8, true, Alloc(1));
381+
std::ranges::copy_backward(std::ranges::subrange(in.end() - 4, in.end()), out.end());
382+
for (std::size_t i = 0; i < static_cast<std::size_t>(in.size() - 4); ++i)
383+
assert(out[i] == true);
384+
for (std::size_t i = in.size() + 4; i < out.size(); ++i)
385+
assert(in[i] == out[i]);
386+
}
387+
{ // Test the middle (whole) words for uint8_t
388+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
389+
std::vector<bool, Alloc> in(17, false, Alloc(1));
390+
for (std::size_t i = 0; i < in.size(); i += 2)
391+
in[i] = true;
392+
std::vector<bool, Alloc> out(24, true, Alloc(1));
393+
std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.end()), out.begin() + in.size());
394+
for (std::size_t i = 0; i < in.size(); ++i)
395+
assert(in[i] == out[i]);
396+
for (std::size_t i = in.size(); i < out.size(); ++i)
397+
assert(out[i] == true);
398+
}
399+
400+
{ // Test the first (partial) word for uint16_t
401+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
402+
std::vector<bool, Alloc> in(14, false, Alloc(1));
403+
std::vector<bool, Alloc> out(16, true, Alloc(1));
404+
std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.begin() + 2), out.begin() + 2);
405+
assert(out[0] == false);
406+
assert(out[1] == false);
407+
for (std::size_t i = 2; i < out.size(); ++i)
408+
assert(out[i] == true);
409+
}
410+
{ // Test the last (partial) word for uint16_t
411+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
412+
std::vector<bool, Alloc> in(16, false, Alloc(1));
413+
for (std::size_t i = 0; i < in.size(); i += 2)
414+
in[i] = true;
415+
std::vector<bool, Alloc> out(16, true, Alloc(1));
416+
std::ranges::copy_backward(std::ranges::subrange(in.end() - 8, in.end()), out.end());
417+
for (std::size_t i = 0; i < static_cast<std::size_t>(in.size() - 8); ++i)
418+
assert(out[i] == true);
419+
for (std::size_t i = in.size() + 8; i < out.size(); ++i)
420+
assert(in[i] == out[i]);
421+
}
422+
{ // Test the middle (whole) words for uint16_t
423+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
424+
std::vector<bool, Alloc> in(34, false, Alloc(1));
425+
for (std::size_t i = 0; i < in.size(); i += 2)
426+
in[i] = true;
427+
std::vector<bool, Alloc> out(48, true, Alloc(1));
428+
std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.end()), out.begin() + in.size());
429+
for (std::size_t i = 0; i < in.size(); ++i)
430+
assert(in[i] == out[i]);
431+
for (std::size_t i = in.size(); i < out.size(); ++i)
432+
assert(out[i] == true);
433+
}
434+
435+
//// Tests for std::ranges::copy_backward with unaligned bits
436+
437+
{ // Test the first (partial) word for uint8_t
438+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
439+
std::vector<bool, Alloc> in(8, false, Alloc(1));
440+
std::vector<bool, Alloc> out(8, true, Alloc(1));
441+
std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.begin() + 1), out.begin() + 1);
442+
assert(out[0] == false);
443+
for (std::size_t i = 1; i < out.size(); ++i)
444+
assert(out[i] == true);
445+
}
446+
{ // Test the last (partial) word for uint8_t
447+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
448+
std::vector<bool, Alloc> in(8, false, Alloc(1));
449+
std::vector<bool, Alloc> out(8, true, Alloc(1));
450+
std::ranges::copy_backward(std::ranges::subrange(in.end() - 1, in.end()), out.begin() + 1);
451+
assert(out[0] == false);
452+
for (std::size_t i = 1; i < out.size(); ++i)
453+
assert(out[i] == true);
454+
}
455+
{ // Test the middle (whole) words for uint8_t
456+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
457+
std::vector<bool, Alloc> in(16, false, Alloc(1));
458+
for (std::size_t i = 0; i < in.size(); i += 2)
459+
in[i] = true;
460+
std::vector<bool, Alloc> out(17, true, Alloc(1));
461+
std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.end()), out.end());
462+
assert(out[0] == true);
463+
for (std::size_t i = 0; i < in.size(); ++i)
464+
assert(in[i] == out[i + 1]);
465+
}
466+
467+
{ // Test the first (partial) word for uint16_t
468+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
469+
std::vector<bool, Alloc> in(16, false, Alloc(1));
470+
std::vector<bool, Alloc> out(16, true, Alloc(1));
471+
std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.begin() + 2), out.begin() + 2);
472+
assert(out[0] == false);
473+
assert(out[1] == false);
474+
for (std::size_t i = 2; i < out.size(); ++i)
475+
assert(out[i] == true);
476+
}
477+
{ // Test the last (partial) word for uint16_t
478+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
479+
std::vector<bool, Alloc> in(16, false, Alloc(1));
480+
std::vector<bool, Alloc> out(16, true, Alloc(1));
481+
std::ranges::copy_backward(std::ranges::subrange(in.end() - 2, in.end()), out.begin() + 2);
482+
assert(out[0] == false);
483+
assert(out[1] == false);
484+
for (std::size_t i = 2; i < out.size(); ++i)
485+
assert(out[i] == true);
486+
}
487+
{ // Test the middle (whole) words for uint16_t
488+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
489+
std::vector<bool, Alloc> in(32, false, Alloc(1));
490+
for (std::size_t i = 0; i < in.size(); i += 2)
491+
in[i] = true;
492+
std::vector<bool, Alloc> out(33, true, Alloc(1));
493+
std::ranges::copy_backward(std::ranges::subrange(in.begin(), in.end()), out.end());
494+
assert(out[0] == true);
495+
for (std::size_t i = 0; i < in.size(); ++i)
496+
assert(in[i] == out[i + 1]);
497+
}
498+
}
358499
#endif
359500

360501
return true;

0 commit comments

Comments
 (0)