Skip to content

Commit 41b17c4

Browse files
author
Arthur O'Dwyer
committed
[libc++] Fix signed overflow inside ranges::advance.
See LWG reflector thread of 2021-07-23 titled 'Question on ranges::advance and "past-the-sentinel iterators"'. Test case heavily based on one graciously provided by Casey Carter. Differential Revision: https://reviews.llvm.org/D106735
1 parent 25666a7 commit 41b17c4

File tree

2 files changed

+35
-4
lines changed

2 files changed

+35
-4
lines changed

libcxx/include/__iterator/advance.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ struct __advance_fn final : private __function_like {
7676
private:
7777
template <class _Tp>
7878
_LIBCPP_HIDE_FROM_ABI
79-
static constexpr _Tp __abs(_Tp __n) noexcept {
80-
return __n < 0 ? -__n : __n;
79+
static constexpr _Tp __magnitude_geq(_Tp __a, _Tp __b) {
80+
return __a < 0 ? (__a <= __b) : (__a >= __b);
8181
}
8282

8383
template <class _Ip>
@@ -153,12 +153,12 @@ struct __advance_fn final : private __function_like {
153153
template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
154154
_LIBCPP_HIDE_FROM_ABI
155155
constexpr iter_difference_t<_Ip> operator()(_Ip& __i, iter_difference_t<_Ip> __n, _Sp __bound) const {
156-
_LIBCPP_ASSERT(__n >= 0 || (bidirectional_iterator<_Ip> && same_as<_Ip, _Sp>),
156+
_LIBCPP_ASSERT((bidirectional_iterator<_Ip> && same_as<_Ip, _Sp>) || (__n >= 0),
157157
"If `n < 0`, then `bidirectional_iterator<I> && same_as<I, S>` must be true.");
158158
// If `S` and `I` model `sized_sentinel_for<S, I>`:
159159
if constexpr (sized_sentinel_for<_Sp, _Ip>) {
160160
// If |n| >= |bound - i|, equivalent to `ranges::advance(i, bound)`.
161-
if (const auto __M = __bound - __i; __abs(__n) >= __abs(__M)) {
161+
if (auto __M = __bound - __i; __magnitude_geq(__n, __M)) {
162162
(*this)(__i, __bound);
163163
return __n - __M;
164164
}

libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include <array>
1818
#include <cassert>
19+
#include <climits>
1920

2021
#include "test_iterators.h"
2122

@@ -100,6 +101,23 @@ constexpr void check_backward(std::ptrdiff_t n, expected_t expected, range_t& ra
100101
assert(current.stride_count() == -current.stride_displacement());
101102
}
102103

104+
struct iota_iterator {
105+
using difference_type = int;
106+
using value_type = int;
107+
108+
constexpr int operator*() const { return x; }
109+
constexpr iota_iterator& operator++() { ++x; return *this; }
110+
constexpr iota_iterator operator++(int) { ++x; return iota_iterator{x - 1}; }
111+
constexpr bool operator==(const iota_iterator&) const = default;
112+
constexpr int operator-(const iota_iterator& that) const { return x - that.x; }
113+
constexpr iota_iterator& operator--() { --x; return *this; }
114+
constexpr iota_iterator operator--(int) { --x; return iota_iterator{x + 1}; }
115+
116+
int x;
117+
};
118+
static_assert(std::bidirectional_iterator<iota_iterator>);
119+
static_assert(std::sized_sentinel_for<iota_iterator, iota_iterator>);
120+
103121
constexpr bool test() {
104122
auto range = range_t{0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
105123
check_forward_sized_sentinel<cpp17_input_iterator<range_t::const_iterator> >(1, {range.begin() + 1, 0}, range);
@@ -136,6 +154,19 @@ constexpr bool test() {
136154
check_forward<forward_iterator<range_t::const_iterator> >(1000, {range.end(), 990}, range);
137155
check_backward<bidirectional_iterator<range_t::const_iterator> >(1000, {range.begin(), -990}, range);
138156

157+
// regression-test that INT_MIN doesn't cause any undefined behavior
158+
{
159+
auto i = iota_iterator{+1};
160+
assert(std::ranges::advance(i, INT_MIN, iota_iterator{-2}) == INT_MIN+3);
161+
assert(i == iota_iterator{-2});
162+
i = iota_iterator{+1};
163+
assert(std::ranges::advance(i, -2, iota_iterator{INT_MIN+1}) == 0);
164+
assert(i == iota_iterator{-1});
165+
i = iota_iterator{+1};
166+
assert(std::ranges::advance(i, INT_MIN, iota_iterator{INT_MIN+1}) == 0);
167+
assert(i == iota_iterator{INT_MIN+1});
168+
}
169+
139170
return true;
140171
}
141172

0 commit comments

Comments
 (0)