The implementation of InPlaceIterable for Peekable is unsound

[SkiFire13]

This code leads to an out of bounds write, makes the assert fail and triggers miri:

fn main() {
    let v = vec![0; 5];
    let mut i = v.into_iter().peekable();
    i.peek();
    let v = i.clone().collect::<Vec<_>>();
    assert!(v.len() <= v.capacity());
}

playground

The problem is that after cloning a Peekable the inner iterator can no longer be considered advanced, but the Peekable may still yield the peeked element without advancing it, thus breaking the contract of InPlaceIterable.

[the8472]

This can be fixed by either removing InPlaceIterable from Peekable or by changing the Clone impl of IntoIter to clone the exact state of IntoIter rather than allocating a smaller one. The latter may be unnecessarily costly when cloning a mostly-exhausted iterator, but on the other hand it would preserve the optimization.
So I'd try the first one first and do a perf run to see what the impact is (when it was introduced it mostly helped rustdoc) and try the latter if there's a significant regression.

[the8472]

It may also be possible to fix this by specializing Clone on Peekable to look into its SourceIter (if available) and do a custom clone of its source.
I'll see if specialization is powerful enough to pull that off.

[the8472]

Hrm, I think it would be possible to do that but it would require some additional helper traits or methods tailored to just this use-case since core doesn't know about alloc. That's probably too much hackery for this optimization.

[aDotInTheVoid]

This also effects stable

https://godbolt.org/z/4P5sbP6xf

@rustbot modify labels: regression-from-stable-to-stable

[hameerabbasi]

Is it a regression though?

Marking as P-critical according to the discussion in WG-prio.

@rustbot modify labels -I-prioritize +P-critical

[SkiFire13]

Is it a regression though?

Technically when #70793 was merged it broke any crate that used code like this (which is not that unexpected like exploting panics and stuff like that). In fact I found this bug due to a project written in only safe rust code that was segfaulting