Skip to content

core::slice code may overflow with slices of zero-sized elements #25016

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
lilyball opened this issue May 1, 2015 · 3 comments · Fixed by #25300
Closed

core::slice code may overflow with slices of zero-sized elements #25016

lilyball opened this issue May 1, 2015 · 3 comments · Fixed by #25300

Comments

@lilyball
Copy link
Contributor

lilyball commented May 1, 2015

The core::slice code appears to have been written to work correctly for slices of zero-sized elements where s.as_ptr() + s.len() overflows. Specifically, the iterators calculate an end value like that, but doesn't rely on end > ptr (it does equality tests, and for size_hint it calculates the delta), and the iterators also explicitly always yield &mut *(1 as *mut _) as the pointer value for zero-sized types to avoid the case of yielding null (which would be interpreted as None instead of Some(_)).

But all this code was written before we had overflow checks. With overflow checks enabled, there's a number of places that can now inadvertently overflow. As such, the whole module needs to be looked over and converted to use wrapping arithmetic where it makes sense.

Also see #24997.

@lilyball
Copy link
Contributor Author

Working on a patch for this I ran across a related bug:

fn main() {
    let v = [(); 10];
    let mut it = v.iter();
    let _ = it.nth(4);
    assert_eq!(it.count(), 5);
}

This fails the assertion because it.count() returns 9.

lilyball added a commit to lilyball/rust that referenced this issue May 11, 2015
core::slice was originally written to tolerate overflow (notably, with
slices of zero-sized elements), but it was never updated to use wrapping
arithmetic when overflow traps were added.

Also correctly handle the case of calling .nth() on an Iter with a
zero-sized element type. The iterator was assuming that the pointer
value of the returned reference was meaningful, but that's not true for
zero-sized elements.

Fixes rust-lang#25016.
@lilyball
Copy link
Contributor Author

#25300 fixes the aforementioned related bug as well.

@lilyball
Copy link
Contributor Author

Another bug:

#![feature(core)]

use std::slice;

fn foo<T>(v: &[T]) -> Option<&[T]> {
    let mut it = v.iter();
    for _ in 0..5 {
        let _ = it.next();
    }
    Some(it.as_slice())
}

fn main() {
    let v: &[()] = unsafe { slice::from_raw_parts(-5isize as *const (), 10) };
    assert!(foo(v).is_some());
}

bors added a commit that referenced this issue May 12, 2015
core::slice was originally written to tolerate overflow (notably, with
slices of zero-sized elements), but it was never updated to use wrapping
arithmetic when overflow traps were added.

Also correctly handle the case of calling .nth() on an Iter with a
zero-sized element type. The iterator was assuming that the pointer
value of the returned reference was meaningful, but that's not true for
zero-sized elements.

Fixes #25016.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants