Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] Fix uninitialized algorithms when using unconstrained comparison operators #69373

Merged
merged 2 commits into from
Oct 20, 2023
Merged

[libc++] Fix uninitialized algorithms when using unconstrained comparison operators #69373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
43a8d8f
[libc++] Fix uninitialized algorithms when using unconstrained compar…
ldionne Oct 17, 2023
71b5263
Apply review comments and fix C++03/C++11 builds
ldionne Oct 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions libcxx/include/__memory/ranges_uninitialized_algorithms.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,9 @@ struct __fn {
operator()(_InputIterator __ifirst, _Sentinel1 __ilast, _OutputIterator __ofirst, _Sentinel2 __olast) const {
using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;

auto __result = _VSTD::__uninitialized_copy<_ValueType>(_VSTD::move(__ifirst), _VSTD::move(__ilast),
_VSTD::move(__ofirst), _VSTD::move(__olast));
auto __stop_copying = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
auto __result = std::__uninitialized_copy<_ValueType>(
std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __stop_copying);
return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
}

Expand Down Expand Up @@ -232,8 +233,9 @@ struct __fn {
operator()(_InputIterator __ifirst, iter_difference_t<_InputIterator> __n,
_OutputIterator __ofirst, _Sentinel __olast) const {
using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
auto __result = _VSTD::__uninitialized_copy_n<_ValueType>(_VSTD::move(__ifirst), __n,
_VSTD::move(__ofirst), _VSTD::move(__olast));
auto __stop_copying = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
auto __result =
std::__uninitialized_copy_n<_ValueType>(std::move(__ifirst), __n, std::move(__ofirst), __stop_copying);
return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
}
};
Expand Down Expand Up @@ -261,8 +263,9 @@ struct __fn {
operator()(_InputIterator __ifirst, _Sentinel1 __ilast, _OutputIterator __ofirst, _Sentinel2 __olast) const {
using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
auto __iter_move = [](auto&& __iter) -> decltype(auto) { return ranges::iter_move(__iter); };
auto __result = _VSTD::__uninitialized_move<_ValueType>(_VSTD::move(__ifirst), _VSTD::move(__ilast),
_VSTD::move(__ofirst), _VSTD::move(__olast), __iter_move);
auto __stop_moving = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
auto __result = std::__uninitialized_move<_ValueType>(
std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __stop_moving, __iter_move);
return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
}

Expand Down Expand Up @@ -298,8 +301,9 @@ struct __fn {
_OutputIterator __ofirst, _Sentinel __olast) const {
using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
auto __iter_move = [](auto&& __iter) -> decltype(auto) { return ranges::iter_move(__iter); };
auto __result = _VSTD::__uninitialized_move_n<_ValueType>(_VSTD::move(__ifirst), __n,
_VSTD::move(__ofirst), _VSTD::move(__olast), __iter_move);
auto __stop_moving = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
auto __result = std::__uninitialized_move_n<_ValueType>(
std::move(__ifirst), __n, std::move(__ofirst), __stop_moving, __iter_move);
return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
}
};
Expand Down
75 changes: 41 additions & 34 deletions libcxx/include/__memory/uninitialized_algorithms.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,23 @@

_LIBCPP_BEGIN_NAMESPACE_STD

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

// uninitialized_copy

template <class _ValueType, class _InputIterator, class _Sentinel1, class _ForwardIterator, class _Sentinel2>
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator>
__uninitialized_copy(_InputIterator __ifirst, _Sentinel1 __ilast,
_ForwardIterator __ofirst, _Sentinel2 __olast) {
template <class _ValueType, class _InputIterator, class _Sentinel1, class _ForwardIterator, class _EndPredicate>
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitialized_copy(
_InputIterator __ifirst, _Sentinel1 __ilast, _ForwardIterator __ofirst, _EndPredicate __stop_copying) {
_ForwardIterator __idx = __ofirst;
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
try {
#endif
for (; __ifirst != __ilast && __idx != __olast; ++__ifirst, (void)++__idx)
for (; __ifirst != __ilast && !__stop_copying(__idx); ++__ifirst, (void)++__idx)
::new (_VSTD::__voidify(*__idx)) _ValueType(*__ifirst);
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
Expand All @@ -80,22 +77,21 @@ _LIBCPP_HIDE_FROM_ABI
_ForwardIterator uninitialized_copy(_InputIterator __ifirst, _InputIterator __ilast,
_ForwardIterator __ofirst) {
typedef typename iterator_traits<_ForwardIterator>::value_type _ValueType;
auto __result = _VSTD::__uninitialized_copy<_ValueType>(_VSTD::move(__ifirst), _VSTD::move(__ilast),
_VSTD::move(__ofirst), __unreachable_sentinel());
auto __result = std::__uninitialized_copy<_ValueType>(
std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __always_false());
return _VSTD::move(__result.second);
}

// uninitialized_copy_n

template <class _ValueType, class _InputIterator, class _Size, class _ForwardIterator, class _Sentinel>
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator>
__uninitialized_copy_n(_InputIterator __ifirst, _Size __n,
_ForwardIterator __ofirst, _Sentinel __olast) {
template <class _ValueType, class _InputIterator, class _Size, class _ForwardIterator, class _EndPredicate>
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitialized_copy_n(
_InputIterator __ifirst, _Size __n, _ForwardIterator __ofirst, _EndPredicate __stop_copying) {
_ForwardIterator __idx = __ofirst;
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
try {
#endif
for (; __n > 0 && __idx != __olast; ++__ifirst, (void)++__idx, (void)--__n)
for (; __n > 0 && !__stop_copying(__idx); ++__ifirst, (void)++__idx, (void)--__n)
::new (_VSTD::__voidify(*__idx)) _ValueType(*__ifirst);
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
Expand All @@ -111,8 +107,8 @@ template <class _InputIterator, class _Size, class _ForwardIterator>
inline _LIBCPP_HIDE_FROM_ABI _ForwardIterator uninitialized_copy_n(_InputIterator __ifirst, _Size __n,
_ForwardIterator __ofirst) {
typedef typename iterator_traits<_ForwardIterator>::value_type _ValueType;
auto __result = _VSTD::__uninitialized_copy_n<_ValueType>(_VSTD::move(__ifirst), __n, _VSTD::move(__ofirst),
__unreachable_sentinel());
auto __result =
std::__uninitialized_copy_n<_ValueType>(std::move(__ifirst), __n, std::move(__ofirst), __always_false());
return _VSTD::move(__result.second);
}

Expand Down Expand Up @@ -300,16 +296,23 @@ _ForwardIterator uninitialized_value_construct_n(_ForwardIterator __first, _Size

// uninitialized_move

template <class _ValueType, class _InputIterator, class _Sentinel1, class _ForwardIterator, class _Sentinel2,
template <class _ValueType,
class _InputIterator,
class _Sentinel1,
class _ForwardIterator,
class _EndPredicate,
class _IterMove>
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator>
__uninitialized_move(_InputIterator __ifirst, _Sentinel1 __ilast,
_ForwardIterator __ofirst, _Sentinel2 __olast, _IterMove __iter_move) {
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitialized_move(
_InputIterator __ifirst,
_Sentinel1 __ilast,
_ForwardIterator __ofirst,
_EndPredicate __stop_moving,
_IterMove __iter_move) {
auto __idx = __ofirst;
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
try {
#endif
for (; __ifirst != __ilast && __idx != __olast; ++__idx, (void)++__ifirst) {
for (; __ifirst != __ilast && !__stop_moving(__idx); ++__idx, (void)++__ifirst) {
::new (_VSTD::__voidify(*__idx)) _ValueType(__iter_move(__ifirst));
}
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
Expand All @@ -328,22 +331,26 @@ inline _LIBCPP_HIDE_FROM_ABI _ForwardIterator uninitialized_move(_InputIterator
using _ValueType = typename iterator_traits<_ForwardIterator>::value_type;
auto __iter_move = [](auto&& __iter) -> decltype(auto) { return _VSTD::move(*__iter); };

auto __result = _VSTD::__uninitialized_move<_ValueType>(_VSTD::move(__ifirst), _VSTD::move(__ilast),
_VSTD::move(__ofirst), __unreachable_sentinel(), __iter_move);
auto __result = std::__uninitialized_move<_ValueType>(
std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __always_false(), __iter_move);
return _VSTD::move(__result.second);
}

// uninitialized_move_n

template <class _ValueType, class _InputIterator, class _Size, class _ForwardIterator, class _Sentinel, class _IterMove>
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator>
__uninitialized_move_n(_InputIterator __ifirst, _Size __n,
_ForwardIterator __ofirst, _Sentinel __olast, _IterMove __iter_move) {
template <class _ValueType,
class _InputIterator,
class _Size,
class _ForwardIterator,
class _EndPredicate,
class _IterMove>
inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitialized_move_n(
_InputIterator __ifirst, _Size __n, _ForwardIterator __ofirst, _EndPredicate __stop_moving, _IterMove __iter_move) {
auto __idx = __ofirst;
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
try {
#endif
for (; __n > 0 && __idx != __olast; ++__idx, (void)++__ifirst, --__n)
for (; __n > 0 && !__stop_moving(__idx); ++__idx, (void)++__ifirst, --__n)
::new (_VSTD::__voidify(*__idx)) _ValueType(__iter_move(__ifirst));
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
Expand All @@ -361,8 +368,8 @@ uninitialized_move_n(_InputIterator __ifirst, _Size __n, _ForwardIterator __ofir
using _ValueType = typename iterator_traits<_ForwardIterator>::value_type;
auto __iter_move = [](auto&& __iter) -> decltype(auto) { return _VSTD::move(*__iter); };

return _VSTD::__uninitialized_move_n<_ValueType>(_VSTD::move(__ifirst), __n, _VSTD::move(__ofirst),
__unreachable_sentinel(), __iter_move);
return std::__uninitialized_move_n<_ValueType>(
std::move(__ifirst), __n, std::move(__ofirst), __always_false(), __iter_move);
}

// TODO: Rewrite this to iterate left to right and use reverse_iterators when calling
Expand Down
73 changes: 73 additions & 0 deletions libcxx/test/std/utilities/memory/specialized.algorithms/overload_compare_iterator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// -*- C++ -*-
//===----------------------------------------------------------------------===//
//
// 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 LIBCPP_TEST_STD_UTILITIES_MEMORY_SPECIALIZED_ALGORITHMS_OVERLOAD_COMPARE_ITERATOR_H
#define LIBCPP_TEST_STD_UTILITIES_MEMORY_SPECIALIZED_ALGORITHMS_OVERLOAD_COMPARE_ITERATOR_H

#include <iterator>
#include <memory>

#include "test_macros.h"

// An iterator type that overloads operator== and operator!= without any constraints, which
// can trip up some algorithms if we compare iterators against types that we're not allowed to.
//
// See https://github.com/llvm/llvm-project/issues/69334 for details.
template <class Iterator>
struct overload_compare_iterator {
using value_type = typename std::iterator_traits<Iterator>::value_type;
using difference_type = typename std::iterator_traits<Iterator>::difference_type;
using reference = typename std::iterator_traits<Iterator>::reference;
using pointer = typename std::iterator_traits<Iterator>::pointer;
using iterator_category = typename std::iterator_traits<Iterator>::iterator_category;

overload_compare_iterator() = default;

explicit overload_compare_iterator(Iterator it) : it_(it) {}

overload_compare_iterator(overload_compare_iterator const&) = default;
overload_compare_iterator(overload_compare_iterator&&) = default;
overload_compare_iterator& operator=(overload_compare_iterator const&) = default;
overload_compare_iterator& operator=(overload_compare_iterator&&) = default;

reference operator*() const TEST_NOEXCEPT { return *it_; }

pointer operator->() const TEST_NOEXCEPT { return std::addressof(*it_); }

overload_compare_iterator& operator++() TEST_NOEXCEPT {
++it_;
return *this;
}

overload_compare_iterator operator++(int) const TEST_NOEXCEPT {
overload_compare_iterator old(*this);
++(*this);
return old;
}

bool operator==(overload_compare_iterator const& other) const TEST_NOEXCEPT { return this->it_ == other.it_; }

bool operator!=(overload_compare_iterator const& other) const TEST_NOEXCEPT { return !this->operator==(other); }

// Hostile overloads
template <class Sentinel>
friend bool operator==(overload_compare_iterator const& lhs, Sentinel const& rhs) TEST_NOEXCEPT {
return static_cast<Iterator const&>(lhs) == rhs;
}

template <class Sentinel>
friend bool operator!=(overload_compare_iterator const& lhs, Sentinel const& rhs) TEST_NOEXCEPT {
return static_cast<Iterator const&>(lhs) != rhs;
}

private:
Iterator it_;
};

#endif // LIBCPP_TEST_STD_UTILITIES_MEMORY_SPECIALIZED_ALGORITHMS_OVERLOAD_COMPARE_ITERATOR_H
32 changes: 32 additions & 0 deletions ...ities/memory/specialized.algorithms/uninitialized.copy/ranges_uninitialized_copy.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "../buffer.h"
#include "../counted.h"
#include "../overload_compare_iterator.h"
#include "test_macros.h"
#include "test_iterators.h"

Expand Down Expand Up @@ -396,5 +397,36 @@ int main(int, char**) {
}
}

// Test with an iterator that overloads operator== and operator!= as the input and output iterators
{
using T = int;
using Iterator = overload_compare_iterator<T*>;
const int N = 5;

// input
{
char pool[sizeof(T) * N] = {0};
T* p = reinterpret_cast<T*>(pool);
T* p_end = reinterpret_cast<T*>(pool) + N;
T array[N] = {1, 2, 3, 4, 5};
std::ranges::uninitialized_copy(Iterator(array), Iterator(array + N), p, p_end);
for (int i = 0; i != N; ++i) {
assert(array[i] == p[i]);
}
}

// output
{
char pool[sizeof(T) * N] = {0};
T* p = reinterpret_cast<T*>(pool);
T* p_end = reinterpret_cast<T*>(pool) + N;
T array[N] = {1, 2, 3, 4, 5};
std::ranges::uninitialized_copy(array, array + N, Iterator(p), Iterator(p_end));
for (int i = 0; i != N; ++i) {
assert(array[i] == p[i]);
}
}
}

return 0;
}
32 changes: 32 additions & 0 deletions ...ies/memory/specialized.algorithms/uninitialized.copy/ranges_uninitialized_copy_n.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "../buffer.h"
#include "../counted.h"
#include "../overload_compare_iterator.h"
#include "test_macros.h"
#include "test_iterators.h"

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

// Test with an iterator that overloads operator== and operator!= as the input and output iterators
{
using T = int;
using Iterator = overload_compare_iterator<T*>;
const int N = 5;

// input
{
char pool[sizeof(T) * N] = {0};
T* p = reinterpret_cast<T*>(pool);
T* p_end = reinterpret_cast<T*>(pool) + N;
T array[N] = {1, 2, 3, 4, 5};
std::ranges::uninitialized_copy_n(Iterator(array), N, p, p_end);
for (int i = 0; i != N; ++i) {
assert(array[i] == p[i]);
}
}

// output
{
char pool[sizeof(T) * N] = {0};
T* p = reinterpret_cast<T*>(pool);
T* p_end = reinterpret_cast<T*>(pool) + N;
T array[N] = {1, 2, 3, 4, 5};
std::ranges::uninitialized_copy_n(array, N, Iterator(p), Iterator(p_end));
for (int i = 0; i != N; ++i) {
assert(array[i] == p[i]);
}
}
}

return 0;
}
Loading