[libcxx-commits] [libcxx] [libc++] Fix UB in bitwise logic of {std, ranges}::{fill, fill_n} algorithms (PR #122410)
Peng Liu via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jan 15 20:40:54 PST 2025
https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/122410
>From 69dd756791362e78c0a0d4526d58e769e91518b5 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Thu, 9 Jan 2025 21:58:49 -0500
Subject: [PATCH 1/2] Fix UB in bitwise logic of std/ranges::fill/fill_n
algorithms
---
libcxx/include/__algorithm/fill_n.h | 4 +-
libcxx/include/__fwd/bit_reference.h | 18 ++
.../alg.fill/fill.pass.cpp | 35 +++
.../alg.fill/fill_n.pass.cpp | 220 ++++++++++--------
.../alg.fill/ranges.fill.pass.cpp | 52 ++++-
.../alg.fill/ranges.fill_n.pass.cpp | 44 +++-
libcxx/test/support/sized_allocator.h | 58 +++++
7 files changed, 321 insertions(+), 110 deletions(-)
create mode 100644 libcxx/test/support/sized_allocator.h
diff --git a/libcxx/include/__algorithm/fill_n.h b/libcxx/include/__algorithm/fill_n.h
index a7e01c45b92220..bbdc70c83e54fe 100644
--- a/libcxx/include/__algorithm/fill_n.h
+++ b/libcxx/include/__algorithm/fill_n.h
@@ -41,7 +41,7 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename __size_difference_typ
if (__first.__ctz_ != 0) {
__storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
__storage_type __dn = std::min(__clz_f, __n);
- __storage_type __m = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
+ __storage_type __m = std::__middle_mask<__storage_type>(__first.__ctz_, __clz_f - __dn);
if (_FillVal)
*__first.__seg_ |= __m;
else
@@ -56,7 +56,7 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename __size_difference_typ
// do last partial word
if (__n > 0) {
__first.__seg_ += __nw;
- __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
+ __storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
if (_FillVal)
*__first.__seg_ |= __m;
else
diff --git a/libcxx/include/__fwd/bit_reference.h b/libcxx/include/__fwd/bit_reference.h
index 30462b6ce4c92f..3dce495c6626c7 100644
--- a/libcxx/include/__fwd/bit_reference.h
+++ b/libcxx/include/__fwd/bit_reference.h
@@ -10,6 +10,8 @@
#define _LIBCPP___FWD_BIT_REFERENCE_H
#include <__config>
+#include <__type_traits/enable_if.h>
+#include <__type_traits/is_unsigned.h>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
@@ -23,6 +25,22 @@ class __bit_iterator;
template <class, class = void>
struct __size_difference_type_traits;
+template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __leading_mask(unsigned __shift) {
+ return static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __shift);
+}
+
+template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __shift) {
+ return static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __shift);
+}
+
+template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __lshift, unsigned __rshift) {
+ return static_cast<_StorageType>(
+ std::__leading_mask<_StorageType>(__lshift) & std::__trailing_mask<_StorageType>(__rshift));
+}
+
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___FWD_BIT_REFERENCE_H
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
index 619dc7242a3660..a1797fab6cc140 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
@@ -19,6 +19,7 @@
#include <cstddef>
#include <vector>
+#include "sized_allocator.h"
#include "test_macros.h"
#include "test_iterators.h"
@@ -46,6 +47,37 @@ struct Test {
}
};
+TEST_CONSTEXPR_CXX20 void test_bititer_with_custom_sized_types() {
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(100, false, Alloc(1));
+ std::vector<bool, Alloc> expected(100, true, Alloc(1));
+ std::fill(in.begin(), in.end(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::fill(in.begin(), in.end(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::fill(in.begin(), in.end(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::fill(in.begin(), in.end(), true);
+ assert(in == expected);
+ }
+}
+
TEST_CONSTEXPR_CXX20 bool test() {
types::for_each(types::forward_iterator_list<char*>(), Test<char>());
types::for_each(types::forward_iterator_list<int*>(), Test<int>());
@@ -93,6 +125,9 @@ TEST_CONSTEXPR_CXX20 bool test() {
assert(in == expected);
}
}
+
+ test_bititer_with_custom_sized_types();
+
return true;
}
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp
index 7d6770de702bf3..582889dbb21d10 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp
@@ -15,159 +15,175 @@
#include <algorithm>
#include <cassert>
+#include <vector>
+#include "sized_allocator.h"
#include "test_macros.h"
#include "test_iterators.h"
#include "user_defined_integral.h"
#if TEST_STD_VER > 17
TEST_CONSTEXPR bool test_constexpr() {
- const std::size_t N = 5;
- int ib[] = {0, 0, 0, 0, 0, 0}; // one bigger than N
-
- auto it = std::fill_n(std::begin(ib), N, 5);
- return it == (std::begin(ib) + N)
- && std::all_of(std::begin(ib), it, [](int a) {return a == 5; })
- && *it == 0 // don't overwrite the last value in the output array
- ;
- }
+ const std::size_t N = 5;
+ int ib[] = {0, 0, 0, 0, 0, 0}; // one bigger than N
+
+ auto it = std::fill_n(std::begin(ib), N, 5);
+ return it == (std::begin(ib) + N) && std::all_of(std::begin(ib), it, [](int a) { return a == 5; }) &&
+ *it == 0 // don't overwrite the last value in the output array
+ ;
+}
#endif
typedef UserDefinedIntegral<unsigned> UDI;
template <class Iter>
-void
-test_char()
-{
- char a[4] = {};
- Iter it = std::fill_n(Iter(a), UDI(4), char(1));
- assert(base(it) == a + 4);
- assert(a[0] == 1);
- assert(a[1] == 1);
- assert(a[2] == 1);
- assert(a[3] == 1);
+void test_char() {
+ char a[4] = {};
+ Iter it = std::fill_n(Iter(a), UDI(4), char(1));
+ assert(base(it) == a + 4);
+ assert(a[0] == 1);
+ assert(a[1] == 1);
+ assert(a[2] == 1);
+ assert(a[3] == 1);
}
template <class Iter>
-void
-test_int()
-{
- int a[4] = {};
- Iter it = std::fill_n(Iter(a), UDI(4), 1);
- assert(base(it) == a + 4);
- assert(a[0] == 1);
- assert(a[1] == 1);
- assert(a[2] == 1);
- assert(a[3] == 1);
+void test_int() {
+ int a[4] = {};
+ Iter it = std::fill_n(Iter(a), UDI(4), 1);
+ assert(base(it) == a + 4);
+ assert(a[0] == 1);
+ assert(a[1] == 1);
+ assert(a[2] == 1);
+ assert(a[3] == 1);
}
-void
-test_int_array()
-{
- int a[4] = {};
- assert(std::fill_n(a, UDI(4), static_cast<char>(1)) == a + 4);
- assert(a[0] == 1);
- assert(a[1] == 1);
- assert(a[2] == 1);
- assert(a[3] == 1);
+void test_int_array() {
+ int a[4] = {};
+ assert(std::fill_n(a, UDI(4), static_cast<char>(1)) == a + 4);
+ assert(a[0] == 1);
+ assert(a[1] == 1);
+ assert(a[2] == 1);
+ assert(a[3] == 1);
}
struct source {
- source() : i(0) { }
+ source() : i(0) {}
- operator int() const { return i++; }
- mutable int i;
+ operator int() const { return i++; }
+ mutable int i;
};
-void
-test_int_array_struct_source()
-{
- int a[4] = {};
- assert(std::fill_n(a, UDI(4), source()) == a + 4);
- assert(a[0] == 0);
- assert(a[1] == 1);
- assert(a[2] == 2);
- assert(a[3] == 3);
+void test_int_array_struct_source() {
+ int a[4] = {};
+ assert(std::fill_n(a, UDI(4), source()) == a + 4);
+ assert(a[0] == 0);
+ assert(a[1] == 1);
+ assert(a[2] == 2);
+ assert(a[3] == 3);
}
struct test1 {
- test1() : c(0) { }
- test1(char xc) : c(xc + 1) { }
- char c;
+ test1() : c(0) {}
+ test1(char xc) : c(xc + 1) {}
+ char c;
};
-void
-test_struct_array()
-{
- test1 test1a[4] = {};
- assert(std::fill_n(test1a, UDI(4), static_cast<char>(10)) == test1a + 4);
- assert(test1a[0].c == 11);
- assert(test1a[1].c == 11);
- assert(test1a[2].c == 11);
- assert(test1a[3].c == 11);
+void test_struct_array() {
+ test1 test1a[4] = {};
+ assert(std::fill_n(test1a, UDI(4), static_cast<char>(10)) == test1a + 4);
+ assert(test1a[0].c == 11);
+ assert(test1a[1].c == 11);
+ assert(test1a[2].c == 11);
+ assert(test1a[3].c == 11);
}
-class A
-{
- char a_;
+class A {
+ char a_;
+
public:
- A() {}
- explicit A(char a) : a_(a) {}
- operator unsigned char() const {return 'b';}
+ A() {}
+ explicit A(char a) : a_(a) {}
+ operator unsigned char() const { return 'b'; }
- friend bool operator==(const A& x, const A& y)
- {return x.a_ == y.a_;}
+ friend bool operator==(const A& x, const A& y) { return x.a_ == y.a_; }
};
-void
-test5()
-{
- A a[3];
- assert(std::fill_n(&a[0], UDI(3), A('a')) == a+3);
- assert(a[0] == A('a'));
- assert(a[1] == A('a'));
- assert(a[2] == A('a'));
+void test5() {
+ A a[3];
+ assert(std::fill_n(&a[0], UDI(3), A('a')) == a + 3);
+ assert(a[0] == A('a'));
+ assert(a[1] == A('a'));
+ assert(a[2] == A('a'));
}
-struct Storage
-{
- union
- {
+struct Storage {
+ union {
unsigned char a;
unsigned char b;
};
};
-void test6()
-{
+void test6() {
Storage foo[5];
std::fill_n(&foo[0], UDI(5), Storage());
}
+TEST_CONSTEXPR_CXX20 void test_bititer_with_custom_sized_types() {
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(100, false, Alloc(1));
+ std::vector<bool, Alloc> expected(100, true, Alloc(1));
+ std::fill_n(in.begin(), in.size(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::fill_n(in.begin(), in.size(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::fill_n(in.begin(), in.size(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::fill_n(in.begin(), in.size(), true);
+ assert(in == expected);
+ }
+}
+
+int main(int, char**) {
+ test_char<cpp17_output_iterator<char*> >();
+ test_char<forward_iterator<char*> >();
+ test_char<bidirectional_iterator<char*> >();
+ test_char<random_access_iterator<char*> >();
+ test_char<char*>();
-int main(int, char**)
-{
- test_char<cpp17_output_iterator<char*> >();
- test_char<forward_iterator<char*> >();
- test_char<bidirectional_iterator<char*> >();
- test_char<random_access_iterator<char*> >();
- test_char<char*>();
+ test_int<cpp17_output_iterator<int*> >();
+ test_int<forward_iterator<int*> >();
+ test_int<bidirectional_iterator<int*> >();
+ test_int<random_access_iterator<int*> >();
+ test_int<int*>();
- test_int<cpp17_output_iterator<int*> >();
- test_int<forward_iterator<int*> >();
- test_int<bidirectional_iterator<int*> >();
- test_int<random_access_iterator<int*> >();
- test_int<int*>();
+ test_int_array();
+ test_int_array_struct_source();
+ test_struct_array();
- test_int_array();
- test_int_array_struct_source();
- test_struct_array();
+ test5();
+ test6();
- test5();
- test6();
+ test_bititer_with_custom_sized_types();
#if TEST_STD_VER > 17
- static_assert(test_constexpr());
+ static_assert(test_constexpr());
#endif
return 0;
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill.pass.cpp
index 5dc375e0e8dc0d..870f05bc555143 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill.pass.cpp
@@ -20,9 +20,12 @@
#include <cassert>
#include <ranges>
#include <string>
+#include <vector>
+#include "sized_allocator.h"
#include "almost_satisfies_types.h"
#include "test_iterators.h"
+#include "test_macros.h"
template <class Iter, class Sent = sentinel_wrapper<Iter>>
concept HasFillIt = requires(Iter iter, Sent sent) { std::ranges::fill(iter, sent, int{}); };
@@ -53,7 +56,7 @@ constexpr void test_iterators() {
}
{
int a[3];
- auto range = std::ranges::subrange(It(a), Sent(It(a + 3)));
+ auto range = std::ranges::subrange(It(a), Sent(It(a + 3)));
std::same_as<It> auto ret = std::ranges::fill(range, 1);
assert(std::all_of(a, a + 3, [](int i) { return i == 1; }));
assert(base(ret) == a + 3);
@@ -69,12 +72,47 @@ constexpr void test_iterators() {
{
std::array<int, 0> a;
auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data())));
- auto ret = std::ranges::fill(range, 1);
+ auto ret = std::ranges::fill(range, 1);
assert(base(ret) == a.data());
}
}
}
+// The `ranges::{fill, fill_n}` algorithms require `vector<bool, Alloc>::iterator` to satisfy the
+// `std::indirectly_writable` concept when used with `vector<bool, Alloc>`, which is only met since C++23.
+#if TEST_STD_VER >= 23
+TEST_CONSTEXPR_CXX20 void test_bititer_with_custom_sized_types() {
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(100, false, Alloc(1));
+ std::vector<bool, Alloc> expected(100, true, Alloc(1));
+ std::ranges::fill(in, true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::ranges::fill(in, true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::ranges::fill(in, true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::ranges::fill(in, true);
+ assert(in == expected);
+ }
+}
+#endif
+
constexpr bool test() {
test_iterators<cpp17_output_iterator<int*>, sentinel_wrapper<cpp17_output_iterator<int*>>>();
test_iterators<cpp20_output_iterator<int*>, sentinel_wrapper<cpp20_output_iterator<int*>>>();
@@ -94,19 +132,19 @@ constexpr bool test() {
};
{
S a[5];
- std::ranges::fill(a, a + 5, S {true});
+ std::ranges::fill(a, a + 5, S{true});
assert(std::all_of(a, a + 5, [](S& s) { return s.copied; }));
}
{
S a[5];
- std::ranges::fill(a, S {true});
+ std::ranges::fill(a, S{true});
assert(std::all_of(a, a + 5, [](S& s) { return s.copied; }));
}
}
{ // check that std::ranges::dangling is returned
[[maybe_unused]] std::same_as<std::ranges::dangling> decltype(auto) ret =
- std::ranges::fill(std::array<int, 10> {}, 1);
+ std::ranges::fill(std::array<int, 10>{}, 1);
}
{ // check that std::ranges::dangling isn't returned with a borrowing range
@@ -131,6 +169,10 @@ constexpr bool test() {
}
}
+#if TEST_STD_VER >= 23
+ test_bititer_with_custom_sized_types();
+#endif
+
return true;
}
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill_n.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill_n.pass.cpp
index 10ff385d474281..fc3289772b886b 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill_n.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/ranges.fill_n.pass.cpp
@@ -18,9 +18,12 @@
#include <cassert>
#include <ranges>
#include <string>
+#include <vector>
+#include "sized_allocator.h"
#include "almost_satisfies_types.h"
#include "test_iterators.h"
+#include "test_macros.h"
template <class Iter>
concept HasFillN = requires(Iter iter) { std::ranges::fill_n(iter, int{}, int{}); };
@@ -48,6 +51,41 @@ constexpr void test_iterators() {
}
}
+// The `ranges::{fill, fill_n}` algorithms require `vector<bool, Alloc>::iterator` to satisfy the
+// `std::indirectly_writable` concept when used with `vector<bool, Alloc>`, which is only met since C++23.
+#if TEST_STD_VER >= 23
+TEST_CONSTEXPR_CXX20 void test_bititer_with_custom_sized_types() {
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(100, false, Alloc(1));
+ std::vector<bool, Alloc> expected(100, true, Alloc(1));
+ std::ranges::fill_n(std::ranges::begin(in), in.size(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::ranges::fill_n(std::ranges::begin(in), in.size(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::ranges::fill_n(std::ranges::begin(in), in.size(), true);
+ assert(in == expected);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
+ std::vector<bool, Alloc> in(200, false, Alloc(1));
+ std::vector<bool, Alloc> expected(200, true, Alloc(1));
+ std::ranges::fill_n(std::ranges::begin(in), in.size(), true);
+ assert(in == expected);
+ }
+}
+#endif
+
constexpr bool test() {
test_iterators<cpp17_output_iterator<int*>, sentinel_wrapper<cpp17_output_iterator<int*>>>();
test_iterators<cpp20_output_iterator<int*>, sentinel_wrapper<cpp20_output_iterator<int*>>>();
@@ -68,7 +106,7 @@ constexpr bool test() {
};
S a[5];
- std::ranges::fill_n(a, 5, S {});
+ std::ranges::fill_n(a, 5, S{});
assert(std::all_of(a, a + 5, [](S& s) { return s.copied; }));
}
@@ -79,6 +117,10 @@ constexpr bool test() {
assert(std::all_of(a.begin(), a.end(), [](auto& s) { return s == "long long string so no SSO"; }));
}
+#if TEST_STD_VER >= 23
+ test_bititer_with_custom_sized_types();
+#endif
+
return true;
}
diff --git a/libcxx/test/support/sized_allocator.h b/libcxx/test/support/sized_allocator.h
new file mode 100644
index 00000000000000..a877252e82962c
--- /dev/null
+++ b/libcxx/test/support/sized_allocator.h
@@ -0,0 +1,58 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_SUPPORT_SIZED_ALLOCATOR_H
+#define TEST_SUPPORT_SIZED_ALLOCATOR_H
+
+#include <cstddef>
+#include <limits>
+#include <memory>
+#include <new>
+
+#include "test_macros.h"
+
+template <typename T, typename SIZE_TYPE = std::size_t, typename DIFF_TYPE = std::ptrdiff_t>
+class sized_allocator {
+ template <typename U, typename Sz, typename Diff>
+ friend class sized_allocator;
+
+public:
+ using value_type = T;
+ using size_type = SIZE_TYPE;
+ using difference_type = DIFF_TYPE;
+ using propagate_on_container_swap = std::true_type;
+
+ TEST_CONSTEXPR_CXX20 explicit sized_allocator(int d = 0) : data_(d) {}
+
+ template <typename U, typename Sz, typename Diff>
+ TEST_CONSTEXPR_CXX20 sized_allocator(const sized_allocator<U, Sz, Diff>& a) TEST_NOEXCEPT : data_(a.data_) {}
+
+ TEST_CONSTEXPR_CXX20 T* allocate(size_type n) {
+ if (n > max_size())
+ TEST_THROW(std::bad_array_new_length());
+ return std::allocator<T>().allocate(n);
+ }
+
+ TEST_CONSTEXPR_CXX20 void deallocate(T* p, size_type n) TEST_NOEXCEPT { std::allocator<T>().deallocate(p, n); }
+
+ TEST_CONSTEXPR size_type max_size() const TEST_NOEXCEPT {
+ return std::numeric_limits<size_type>::max() / sizeof(value_type);
+ }
+
+private:
+ int data_;
+
+ TEST_CONSTEXPR friend bool operator==(const sized_allocator& a, const sized_allocator& b) {
+ return a.data_ == b.data_;
+ }
+ TEST_CONSTEXPR friend bool operator!=(const sized_allocator& a, const sized_allocator& b) {
+ return a.data_ != b.data_;
+ }
+};
+
+#endif
>From 2e64e4f0f75b093f87b87537a81a35ed16b1f9b7 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 15 Jan 2025 09:16:14 -0500
Subject: [PATCH 2/2] Address @ldionne's review comments
---
libcxx/include/__algorithm/fill_n.h | 12 ++----------
libcxx/include/__bit_reference | 23 +++++++++++++++++++++++
libcxx/include/__fwd/bit_reference.h | 20 +++++---------------
libcxx/test/support/sized_allocator.h | 8 ++++----
libcxx/utils/libcxx/test/params.py | 1 +
5 files changed, 35 insertions(+), 29 deletions(-)
diff --git a/libcxx/include/__algorithm/fill_n.h b/libcxx/include/__algorithm/fill_n.h
index bbdc70c83e54fe..a0141fafbc29cb 100644
--- a/libcxx/include/__algorithm/fill_n.h
+++ b/libcxx/include/__algorithm/fill_n.h
@@ -41,11 +41,7 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename __size_difference_typ
if (__first.__ctz_ != 0) {
__storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
__storage_type __dn = std::min(__clz_f, __n);
- __storage_type __m = std::__middle_mask<__storage_type>(__first.__ctz_, __clz_f - __dn);
- if (_FillVal)
- *__first.__seg_ |= __m;
- else
- *__first.__seg_ &= ~__m;
+ std::__fill_masked_range(std::__to_address(__first.__seg_), __first.__ctz_, __clz_f - __dn, _FillVal);
__n -= __dn;
++__first.__seg_;
}
@@ -56,11 +52,7 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename __size_difference_typ
// do last partial word
if (__n > 0) {
__first.__seg_ += __nw;
- __storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
- if (_FillVal)
- *__first.__seg_ |= __m;
- else
- *__first.__seg_ &= ~__m;
+ std::__fill_masked_range(std::__to_address(__first.__seg_), 0u, __bits_per_word - __n, _FillVal);
}
}
diff --git a/libcxx/include/__bit_reference b/libcxx/include/__bit_reference
index 67abb023122edf..2576a9a649bd63 100644
--- a/libcxx/include/__bit_reference
+++ b/libcxx/include/__bit_reference
@@ -12,6 +12,7 @@
#include <__algorithm/copy_n.h>
#include <__algorithm/min.h>
+#include <__assert>
#include <__bit/countr.h>
#include <__compare/ordering.h>
#include <__config>
@@ -22,9 +23,12 @@
#include <__memory/construct_at.h>
#include <__memory/pointer_traits.h>
#include <__type_traits/conditional.h>
+#include <__type_traits/enable_if.h>
#include <__type_traits/is_constant_evaluated.h>
+#include <__type_traits/is_unsigned.h>
#include <__type_traits/void_t.h>
#include <__utility/swap.h>
+#include <climits>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
@@ -55,6 +59,25 @@ struct __size_difference_type_traits<_Cp, __void_t<typename _Cp::difference_type
using size_type = typename _Cp::size_type;
};
+// This function is designed to operate correctly even for smaller integral types like `uint8_t`, `uint16_t`,
+// or `unsigned short`. Casting back to _StorageType is crucial to prevent undefined behavior that can arise
+// from integral promotions.
+// See https://github.com/llvm/llvm-project/pull/122410
+template <class _StoragePointer,
+ __enable_if_t<is_unsigned<typename pointer_traits<_StoragePointer>::element_type>::value, int> >
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
+__fill_masked_range(_StoragePointer __word, unsigned __ctz, unsigned __clz, bool __fill_val) {
+ using _StorageType = typename pointer_traits<_StoragePointer>::element_type;
+ _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+ __ctz + __clz < sizeof(_StorageType) * CHAR_BIT, "__fill_masked_range called with invalid range");
+ _StorageType __m = static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz) &
+ static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz);
+ if (__fill_val)
+ *__word |= __m;
+ else
+ *__word &= static_cast<_StorageType>(~__m);
+}
+
template <class _Cp, bool = __has_storage_type<_Cp>::value>
class __bit_reference {
using __storage_type _LIBCPP_NODEBUG = typename _Cp::__storage_type;
diff --git a/libcxx/include/__fwd/bit_reference.h b/libcxx/include/__fwd/bit_reference.h
index 3dce495c6626c7..278f61bf875a74 100644
--- a/libcxx/include/__fwd/bit_reference.h
+++ b/libcxx/include/__fwd/bit_reference.h
@@ -10,6 +10,7 @@
#define _LIBCPP___FWD_BIT_REFERENCE_H
#include <__config>
+#include <__memory/pointer_traits.h>
#include <__type_traits/enable_if.h>
#include <__type_traits/is_unsigned.h>
@@ -25,21 +26,10 @@ class __bit_iterator;
template <class, class = void>
struct __size_difference_type_traits;
-template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __leading_mask(unsigned __shift) {
- return static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __shift);
-}
-
-template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __shift) {
- return static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __shift);
-}
-
-template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __lshift, unsigned __rshift) {
- return static_cast<_StorageType>(
- std::__leading_mask<_StorageType>(__lshift) & std::__trailing_mask<_StorageType>(__rshift));
-}
+template <class _StoragePointer,
+ __enable_if_t<is_unsigned<typename pointer_traits<_StoragePointer>::element_type>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
+__fill_masked_range(_StoragePointer __word, unsigned __ctz, unsigned __clz, bool __fill_val);
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/test/support/sized_allocator.h b/libcxx/test/support/sized_allocator.h
index a877252e82962c..a9cec8ee28f949 100644
--- a/libcxx/test/support/sized_allocator.h
+++ b/libcxx/test/support/sized_allocator.h
@@ -16,15 +16,15 @@
#include "test_macros.h"
-template <typename T, typename SIZE_TYPE = std::size_t, typename DIFF_TYPE = std::ptrdiff_t>
+template <typename T, typename Size = std::size_t, typename Difference = std::ptrdiff_t>
class sized_allocator {
template <typename U, typename Sz, typename Diff>
friend class sized_allocator;
public:
using value_type = T;
- using size_type = SIZE_TYPE;
- using difference_type = DIFF_TYPE;
+ using size_type = Size;
+ using difference_type = Difference;
using propagate_on_container_swap = std::true_type;
TEST_CONSTEXPR_CXX20 explicit sized_allocator(int d = 0) : data_(d) {}
@@ -55,4 +55,4 @@ class sized_allocator {
}
};
-#endif
+#endif // TEST_SUPPORT_SIZED_ALLOCATOR_H
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index 947cfd26513643..f5595e4d24449f 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -31,6 +31,7 @@
"-Wno-reserved-module-identifier",
'-Wdeprecated-copy',
'-Wdeprecated-copy-dtor',
+ "-Wshift-negative-value",
# GCC warns about places where we might want to add sized allocation/deallocation
# functions, but we know better what we're doing/testing in the test suite.
"-Wno-sized-deallocation",
More information about the libcxx-commits
mailing list