Soundness issue in Zip::next() specialization

[Qwaz]

rust/library/core/src/iter/adapters/zip.rs

Lines 191 to 211 in e708cbd

	#[inline]
	fn next(&mut self) -> Option<(A::Item, B::Item)> {
	if self.index < self.len {
	let i = self.index;
	self.index += 1;
	// SAFETY: `i` is smaller than `self.len`, thus smaller than `self.a.len()` and `self.b.len()`
	unsafe {
	Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i)))
	}
	} else if A::may_have_side_effect() && self.index < self.a.size() {
	// match the base implementation's potential side effects
	// SAFETY: we just checked that `self.index` < `self.a.len()`
	unsafe {
	self.a.__iterator_get_unchecked(self.index);
	}
	self.index += 1;
	None
	} else {
	None
	}
	}

rust/library/core/src/iter/adapters/zip.rs

Lines 395 to 396 in e708cbd

	/// 2. If `self: !Clone`, then `get_unchecked` is never called with the same
	/// index on `self` more than once.

There is a panic safety issue in Zip::next() that allows to call __iterator_get_unchecked() to the same index twice. __iterator_get_unchecked() is called at line 204 and the index is updated at line 206. If line 204 panics, the index is not updated and the subsequent next() call will use the same index for __iterator_get_unchecked(). This violates the second safety requirement of TrustedRandomAccess.

Here is a playground link that demonstrates creating two mutable references to the same memory location without using unsafe Rust.

[Qwaz]

The bug fixed by this PR seems to have the same consequence, but it didn't get the unsound label.
#80670

[sdroege]

Seems like this would be fixed by simply doing the same as in the first branch of the if and incrementing the self.index beforehand. Are you creating a PR for this?

[Qwaz]

Seems like this would be fixed by simply doing the same as in the first branch of the if and incrementing the self.index beforehand.

Agreed :)

Are you creating a PR for this?

Not in the meantime. Fill free to work on a PR if you are interested.

[sdroege]

Not in the meantime. Fill free to work on a PR if you are interested.

Sure, why not :)

[apiraino]

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.