Skip to content

std::iter::Skip::count can overflow even if the iterator would return less that usize::max_value() items #68139

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.

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

Closed
ollie27 opened this issue Jan 11, 2020 · 3 comments · Fixed by #68469
Labels
A-iterators Area: Iterators C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@ollie27
Copy link
Member

ollie27 commented Jan 11, 2020

std::iter::Skip::count currently calls count on the inner iterator but that can overflow even if the Skip itself doesn't return more that usize::max_value() items.

For example:

(0..usize::max_value()).chain(0..10).skip(usize::max_value()).count()

This should return 10 but currently triggers an attempt to add with overflow (or returns 0 if overflow checks are disabled).

This is caused by

#[inline]
fn count(self) -> usize {
self.iter.count().saturating_sub(self.n)
}
which was added in #25035 which was merged between versions 1.0 and 1.1.

@ollie27 ollie27 added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. C-bug Category: This is a bug. A-iterators Area: Iterators labels Jan 11, 2020
@clarfonthey
Copy link
Contributor

clarfonthey commented Jan 12, 2020

This could probably be fixed by calling self.iter.nth(self.n.saturating_sub(1)) and then relying on self.iter.count().

@JohnTitor
Copy link
Member

@clarfon I think it doesn't fix because nth(usize::max_value() - 1) is very slow, right?

@clarfonthey
Copy link
Contributor

Should be overridden for ranges though.

tmandry added a commit to tmandry/rust that referenced this issue Jan 24, 2020
Avoid overflow in `std::iter::Skip::count`

The call to `count` on the inner iterator can overflow even if `Skip` itself would return less that `usize::max_value()` items.

Fixes rust-lang#68139
@bors bors closed this as completed in e7752ae Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-iterators Area: Iterators C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants