Skip to content

Commit 7961fa3

Browse files
ldionneldionne
authored
[libc++] Fix uninitialized algorithms when using unconstrained comparison operators (#69373)
If an iterator passed to std::uninitialized_copy & friends provided an unconstrained comparison operator, we would trigger an ambiguous overload resolution because we used to compare against __unreachable_sentinel in our implementation. This patch fixes that by only comparing the output iterator when it is actually required, i.e. in the <ranges> versions of the algorithms. Fixes #69334
1 parent e1a5843 commit 7961fa3

11 files changed

+373
-42
lines changed

‎libcxx/include/__memory/ranges_uninitialized_algorithms.h

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,9 @@ struct __fn {
196196
operator()(_InputIterator __ifirst, _Sentinel1 __ilast, _OutputIterator __ofirst, _Sentinel2 __olast) const {
197197
using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
198198

199-
auto __result = _VSTD::__uninitialized_copy<_ValueType>(_VSTD::move(__ifirst), _VSTD::move(__ilast),
200-
_VSTD::move(__ofirst), _VSTD::move(__olast));
199+
auto __stop_copying = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
200+
auto __result = std::__uninitialized_copy<_ValueType>(
201+
std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __stop_copying);
201202
return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
202203
}
203204

@@ -232,8 +233,9 @@ struct __fn {
232233
operator()(_InputIterator __ifirst, iter_difference_t<_InputIterator> __n,
233234
_OutputIterator __ofirst, _Sentinel __olast) const {
234235
using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
235-
auto __result = _VSTD::__uninitialized_copy_n<_ValueType>(_VSTD::move(__ifirst), __n,
236-
_VSTD::move(__ofirst), _VSTD::move(__olast));
236+
auto __stop_copying = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
237+
auto __result =
238+
std::__uninitialized_copy_n<_ValueType>(std::move(__ifirst), __n, std::move(__ofirst), __stop_copying);
237239
return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
238240
}
239241
};
@@ -261,8 +263,9 @@ struct __fn {
261263
operator()(_InputIterator __ifirst, _Sentinel1 __ilast, _OutputIterator __ofirst, _Sentinel2 __olast) const {
262264
using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
263265
auto __iter_move = [](auto&& __iter) -> decltype(auto) { return ranges::iter_move(__iter); };
264-
auto __result = _VSTD::__uninitialized_move<_ValueType>(_VSTD::move(__ifirst), _VSTD::move(__ilast),
265-
_VSTD::move(__ofirst), _VSTD::move(__olast), __iter_move);
266+
auto __stop_moving = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
267+
auto __result = std::__uninitialized_move<_ValueType>(
268+
std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __stop_moving, __iter_move);
266269
return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
267270
}
268271

@@ -298,8 +301,9 @@ struct __fn {
298301
_OutputIterator __ofirst, _Sentinel __olast) const {
299302
using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
300303
auto __iter_move = [](auto&& __iter) -> decltype(auto) { return ranges::iter_move(__iter); };
301-
auto __result = _VSTD::__uninitialized_move_n<_ValueType>(_VSTD::move(__ifirst), __n,
302-
_VSTD::move(__ofirst), _VSTD::move(__olast), __iter_move);
304+
auto __stop_moving = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
305+
auto __result = std::__uninitialized_move_n<_ValueType>(
306+
std::move(__ifirst), __n, std::move(__ofirst), __stop_moving, __iter_move);
303307
return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
304308
}
305309
};

‎libcxx/include/__memory/uninitialized_algorithms.h

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,26 +44,23 @@
4444

4545
_LIBCPP_BEGIN_NAMESPACE_STD
4646

47-
// This is a simplified version of C++20 `unreachable_sentinel` that doesn't use concepts and thus can be used in any
48-
// language mode.
49-
struct __unreachable_sentinel {
50-
template <class _Iter>
51-
_LIBCPP_HIDE_FROM_ABI friend _LIBCPP_CONSTEXPR bool operator!=(const _Iter&, __unreachable_sentinel) _NOEXCEPT {
52-
return true;
47+
struct __always_false {
48+
template <class... _Args>
49+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool operator()(_Args&&...) const _NOEXCEPT {
50+
return false;
5351
}
5452
};
5553

5654
// uninitialized_copy
5755

58-
template <class _ValueType, class _InputIterator, class _Sentinel1, class _ForwardIterator, class _Sentinel2>
59-
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator>
60-
__uninitialized_copy(_InputIterator __ifirst, _Sentinel1 __ilast,
61-
_ForwardIterator __ofirst, _Sentinel2 __olast) {
56+
template <class _ValueType, class _InputIterator, class _Sentinel1, class _ForwardIterator, class _EndPredicate>
57+
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitialized_copy(
58+
_InputIterator __ifirst, _Sentinel1 __ilast, _ForwardIterator __ofirst, _EndPredicate __stop_copying) {
6259
_ForwardIterator __idx = __ofirst;
6360
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
6461
try {
6562
#endif
66-
for (; __ifirst != __ilast && __idx != __olast; ++__ifirst, (void)++__idx)
63+
for (; __ifirst != __ilast && !__stop_copying(__idx); ++__ifirst, (void)++__idx)
6764
::new (_VSTD::__voidify(*__idx)) _ValueType(*__ifirst);
6865
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
6966
} catch (...) {
@@ -80,22 +77,21 @@ _LIBCPP_HIDE_FROM_ABI
8077
_ForwardIterator uninitialized_copy(_InputIterator __ifirst, _InputIterator __ilast,
8178
_ForwardIterator __ofirst) {
8279
typedef typename iterator_traits<_ForwardIterator>::value_type _ValueType;
83-
auto __result = _VSTD::__uninitialized_copy<_ValueType>(_VSTD::move(__ifirst), _VSTD::move(__ilast),
84-
_VSTD::move(__ofirst), __unreachable_sentinel());
80+
auto __result = std::__uninitialized_copy<_ValueType>(
81+
std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __always_false());
8582
return _VSTD::move(__result.second);
8683
}
8784

8885
// uninitialized_copy_n
8986

90-
template <class _ValueType, class _InputIterator, class _Size, class _ForwardIterator, class _Sentinel>
91-
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator>
92-
__uninitialized_copy_n(_InputIterator __ifirst, _Size __n,
93-
_ForwardIterator __ofirst, _Sentinel __olast) {
87+
template <class _ValueType, class _InputIterator, class _Size, class _ForwardIterator, class _EndPredicate>
88+
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitialized_copy_n(
89+
_InputIterator __ifirst, _Size __n, _ForwardIterator __ofirst, _EndPredicate __stop_copying) {
9490
_ForwardIterator __idx = __ofirst;
9591
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
9692
try {
9793
#endif
98-
for (; __n > 0 && __idx != __olast; ++__ifirst, (void)++__idx, (void)--__n)
94+
for (; __n > 0 && !__stop_copying(__idx); ++__ifirst, (void)++__idx, (void)--__n)
9995
::new (_VSTD::__voidify(*__idx)) _ValueType(*__ifirst);
10096
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
10197
} catch (...) {
@@ -111,8 +107,8 @@ template <class _InputIterator, class _Size, class _ForwardIterator>
111107
inline _LIBCPP_HIDE_FROM_ABI _ForwardIterator uninitialized_copy_n(_InputIterator __ifirst, _Size __n,
112108
_ForwardIterator __ofirst) {
113109
typedef typename iterator_traits<_ForwardIterator>::value_type _ValueType;
114-
auto __result = _VSTD::__uninitialized_copy_n<_ValueType>(_VSTD::move(__ifirst), __n, _VSTD::move(__ofirst),
115-
__unreachable_sentinel());
110+
auto __result =
111+
std::__uninitialized_copy_n<_ValueType>(std::move(__ifirst), __n, std::move(__ofirst), __always_false());
116112
return _VSTD::move(__result.second);
117113
}
118114

@@ -300,16 +296,23 @@ _ForwardIterator uninitialized_value_construct_n(_ForwardIterator __first, _Size
300296

301297
// uninitialized_move
302298

303-
template <class _ValueType, class _InputIterator, class _Sentinel1, class _ForwardIterator, class _Sentinel2,
299+
template <class _ValueType,
300+
class _InputIterator,
301+
class _Sentinel1,
302+
class _ForwardIterator,
303+
class _EndPredicate,
304304
class _IterMove>
305-
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator>
306-
__uninitialized_move(_InputIterator __ifirst, _Sentinel1 __ilast,
307-
_ForwardIterator __ofirst, _Sentinel2 __olast, _IterMove __iter_move) {
305+
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitialized_move(
306+
_InputIterator __ifirst,
307+
_Sentinel1 __ilast,
308+
_ForwardIterator __ofirst,
309+
_EndPredicate __stop_moving,
310+
_IterMove __iter_move) {
308311
auto __idx = __ofirst;
309312
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
310313
try {
311314
#endif
312-
for (; __ifirst != __ilast && __idx != __olast; ++__idx, (void)++__ifirst) {
315+
for (; __ifirst != __ilast && !__stop_moving(__idx); ++__idx, (void)++__ifirst) {
313316
::new (_VSTD::__voidify(*__idx)) _ValueType(__iter_move(__ifirst));
314317
}
315318
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
@@ -328,22 +331,26 @@ inline _LIBCPP_HIDE_FROM_ABI _ForwardIterator uninitialized_move(_InputIterator
328331
using _ValueType = typename iterator_traits<_ForwardIterator>::value_type;
329332
auto __iter_move = [](auto&& __iter) -> decltype(auto) { return _VSTD::move(*__iter); };
330333

331-
auto __result = _VSTD::__uninitialized_move<_ValueType>(_VSTD::move(__ifirst), _VSTD::move(__ilast),
332-
_VSTD::move(__ofirst), __unreachable_sentinel(), __iter_move);
334+
auto __result = std::__uninitialized_move<_ValueType>(
335+
std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __always_false(), __iter_move);
333336
return _VSTD::move(__result.second);
334337
}
335338

336339
// uninitialized_move_n
337340

338-
template <class _ValueType, class _InputIterator, class _Size, class _ForwardIterator, class _Sentinel, class _IterMove>
339-
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator>
340-
__uninitialized_move_n(_InputIterator __ifirst, _Size __n,
341-
_ForwardIterator __ofirst, _Sentinel __olast, _IterMove __iter_move) {
341+
template <class _ValueType,
342+
class _InputIterator,
343+
class _Size,
344+
class _ForwardIterator,
345+
class _EndPredicate,
346+
class _IterMove>
347+
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitialized_move_n(
348+
_InputIterator __ifirst, _Size __n, _ForwardIterator __ofirst, _EndPredicate __stop_moving, _IterMove __iter_move) {
342349
auto __idx = __ofirst;
343350
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
344351
try {
345352
#endif
346-
for (; __n > 0 && __idx != __olast; ++__idx, (void)++__ifirst, --__n)
353+
for (; __n > 0 && !__stop_moving(__idx); ++__idx, (void)++__ifirst, --__n)
347354
::new (_VSTD::__voidify(*__idx)) _ValueType(__iter_move(__ifirst));
348355
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
349356
} catch (...) {
@@ -361,8 +368,8 @@ uninitialized_move_n(_InputIterator __ifirst, _Size __n, _ForwardIterator __ofir
361368
using _ValueType = typename iterator_traits<_ForwardIterator>::value_type;
362369
auto __iter_move = [](auto&& __iter) -> decltype(auto) { return _VSTD::move(*__iter); };
363370

364-
return _VSTD::__uninitialized_move_n<_ValueType>(_VSTD::move(__ifirst), __n, _VSTD::move(__ofirst),
365-
__unreachable_sentinel(), __iter_move);
371+
return std::__uninitialized_move_n<_ValueType>(
372+
std::move(__ifirst), __n, std::move(__ofirst), __always_false(), __iter_move);
366373
}
367374

368375
// TODO: Rewrite this to iterate left to right and use reverse_iterators when calling

‎libcxx/test/std/utilities/memory/specialized.algorithms/overload_compare_iterator.h

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// -*- C++ -*-
2+
//===----------------------------------------------------------------------===//
3+
//
4+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
5+
// See https://llvm.org/LICENSE.txt for license information.
6+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#ifndef LIBCPP_TEST_STD_UTILITIES_MEMORY_SPECIALIZED_ALGORITHMS_OVERLOAD_COMPARE_ITERATOR_H
11+
#define LIBCPP_TEST_STD_UTILITIES_MEMORY_SPECIALIZED_ALGORITHMS_OVERLOAD_COMPARE_ITERATOR_H
12+
13+
#include <iterator>
14+
#include <memory>
15+
16+
#include "test_macros.h"
17+
18+
// An iterator type that overloads operator== and operator!= without any constraints, which
19+
// can trip up some algorithms if we compare iterators against types that we're not allowed to.
20+
//
21+
// See https://github.com/llvm/llvm-project/issues/69334 for details.
22+
template <class Iterator>
23+
struct overload_compare_iterator {
24+
using value_type = typename std::iterator_traits<Iterator>::value_type;
25+
using difference_type = typename std::iterator_traits<Iterator>::difference_type;
26+
using reference = typename std::iterator_traits<Iterator>::reference;
27+
using pointer = typename std::iterator_traits<Iterator>::pointer;
28+
using iterator_category = typename std::iterator_traits<Iterator>::iterator_category;
29+
30+
overload_compare_iterator() = default;
31+
32+
explicit overload_compare_iterator(Iterator it) : it_(it) {}
33+
34+
overload_compare_iterator(overload_compare_iterator const&) = default;
35+
overload_compare_iterator(overload_compare_iterator&&) = default;
36+
overload_compare_iterator& operator=(overload_compare_iterator const&) = default;
37+
overload_compare_iterator& operator=(overload_compare_iterator&&) = default;
38+
39+
reference operator*() const TEST_NOEXCEPT { return *it_; }
40+
41+
pointer operator->() const TEST_NOEXCEPT { return std::addressof(*it_); }
42+
43+
overload_compare_iterator& operator++() TEST_NOEXCEPT {
44+
++it_;
45+
return *this;
46+
}
47+
48+
overload_compare_iterator operator++(int) const TEST_NOEXCEPT {
49+
overload_compare_iterator old(*this);
50+
++(*this);
51+
return old;
52+
}
53+
54+
bool operator==(overload_compare_iterator const& other) const TEST_NOEXCEPT { return this->it_ == other.it_; }
55+
56+
bool operator!=(overload_compare_iterator const& other) const TEST_NOEXCEPT { return !this->operator==(other); }
57+
58+
// Hostile overloads
59+
template <class Sentinel>
60+
friend bool operator==(overload_compare_iterator const& lhs, Sentinel const& rhs) TEST_NOEXCEPT {
61+
return static_cast<Iterator const&>(lhs) == rhs;
62+
}
63+
64+
template <class Sentinel>
65+
friend bool operator!=(overload_compare_iterator const& lhs, Sentinel const& rhs) TEST_NOEXCEPT {
66+
return static_cast<Iterator const&>(lhs) != rhs;
67+
}
68+
69+
private:
70+
Iterator it_;
71+
};
72+
73+
#endif // LIBCPP_TEST_STD_UTILITIES_MEMORY_SPECIALIZED_ALGORITHMS_OVERLOAD_COMPARE_ITERATOR_H

‎libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.copy/ranges_uninitialized_copy.pass.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include "../buffer.h"
2929
#include "../counted.h"
30+
#include "../overload_compare_iterator.h"
3031
#include "test_macros.h"
3132
#include "test_iterators.h"
3233

@@ -396,5 +397,36 @@ int main(int, char**) {
396397
}
397398
}
398399

400+
// Test with an iterator that overloads operator== and operator!= as the input and output iterators
401+
{
402+
using T = int;
403+
using Iterator = overload_compare_iterator<T*>;
404+
const int N = 5;
405+
406+
// input
407+
{
408+
char pool[sizeof(T) * N] = {0};
409+
T* p = reinterpret_cast<T*>(pool);
410+
T* p_end = reinterpret_cast<T*>(pool) + N;
411+
T array[N] = {1, 2, 3, 4, 5};
412+
std::ranges::uninitialized_copy(Iterator(array), Iterator(array + N), p, p_end);
413+
for (int i = 0; i != N; ++i) {
414+
assert(array[i] == p[i]);
415+
}
416+
}
417+
418+
// output
419+
{
420+
char pool[sizeof(T) * N] = {0};
421+
T* p = reinterpret_cast<T*>(pool);
422+
T* p_end = reinterpret_cast<T*>(pool) + N;
423+
T array[N] = {1, 2, 3, 4, 5};
424+
std::ranges::uninitialized_copy(array, array + N, Iterator(p), Iterator(p_end));
425+
for (int i = 0; i != N; ++i) {
426+
assert(array[i] == p[i]);
427+
}
428+
}
429+
}
430+
399431
return 0;
400432
}

‎libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.copy/ranges_uninitialized_copy_n.pass.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
#include "../buffer.h"
2626
#include "../counted.h"
27+
#include "../overload_compare_iterator.h"
2728
#include "test_macros.h"
2829
#include "test_iterators.h"
2930

@@ -161,5 +162,36 @@ int main(int, char**) {
161162
std::ranges::uninitialized_copy_n(std::move(in), N, out.begin(), out.end());
162163
}
163164

165+
// Test with an iterator that overloads operator== and operator!= as the input and output iterators
166+
{
167+
using T = int;
168+
using Iterator = overload_compare_iterator<T*>;
169+
const int N = 5;
170+
171+
// input
172+
{
173+
char pool[sizeof(T) * N] = {0};
174+
T* p = reinterpret_cast<T*>(pool);
175+
T* p_end = reinterpret_cast<T*>(pool) + N;
176+
T array[N] = {1, 2, 3, 4, 5};
177+
std::ranges::uninitialized_copy_n(Iterator(array), N, p, p_end);
178+
for (int i = 0; i != N; ++i) {
179+
assert(array[i] == p[i]);
180+
}
181+
}
182+
183+
// output
184+
{
185+
char pool[sizeof(T) * N] = {0};
186+
T* p = reinterpret_cast<T*>(pool);
187+
T* p_end = reinterpret_cast<T*>(pool) + N;
188+
T array[N] = {1, 2, 3, 4, 5};
189+
std::ranges::uninitialized_copy_n(array, N, Iterator(p), Iterator(p_end));
190+
for (int i = 0; i != N; ++i) {
191+
assert(array[i] == p[i]);
192+
}
193+
}
194+
}
195+
164196
return 0;
165197
}

0 commit comments

Comments
 (0)