VecDeque implementation creates a slice pointing to possibly uninitialized memory

[ecstatic-morse]

VecDeque has an internal method called buffer_as_slice, which returns an &[T] containing the entire capacity of the VecDeque. This is undefined behavior if the VecDeque is not full, since some elements of the backing RawVec may be uninitialized. However, this invariant is not documented on buffer_as_slice and is not respected in practice. For example, VecDeque::iter calls buffer_as_slice unconditionally:

rust/src/liballoc/collections/vec_deque.rs

Lines 959 to 962 in 34c5cd9

	#[stable(feature = "rust1", since = "1.0.0")]
	pub fn iter(&self) -> Iter<'_, T> {
	Iter { tail: self.tail, head: self.head, ring: unsafe { self.buffer_as_slice() } }
	}

This one seems so obvious that I'm wondering if I've overlooked something. cc @rust-lang/wg-unsafe-code-guidelines

Found while doing #74172.

[RalfJung]

This one seems so obvious that I'm wondering if I've overlooked something.

This code was probably written before we had a clear understanding of our invariants, and before we had MaybeUninit to properly work with uninitialized data.

Cc rust-lang/unsafe-code-guidelines#77

[ecstatic-morse]

This one seems so obvious that I'm wondering if I've overlooked something.

I need to stop hedging 😄.

I also had assumed that the question of whether &T required T to be initialized was more settled than the discussion in rust-lang/unsafe-code-guidelines#77 suggests. With that in mind, how do we want to prioritize this? I think there's precedent for the standard library to rely on behavior that is technically undefined when there is active discussion about the rules and we aren't relying on that property for optimizations. That seems to be the case here?

[RalfJung]

I think there's precedent for the standard library to rely on behavior that is technically undefined when there is active discussion about the rules and we aren't relying on that property for optimizations. That seems to be the case here?

Yeah, I think we can treat it like that.

[JmPotato]

@rustbot claim

[JmPotato]

Hi, @ecstatic-morse @RalfJung. After reading the code, it seems that safety is guaranteed by the check of the head and tail index, so there is no implementation that will access these uninitialized memories currently. However, considering the possibility of these codes being modified in the future, we do need to do something to make it be aware. Should I change &[T] to something like &[MaybeUninit<T>]. or just add some document? Any guidance would be helpful and appreciated!

[RalfJung]

The main concern is that this code is violating the rules that are explicitly stated at https://doc.rust-lang.org/reference/behavior-considered-undefined.html. So we have two options, both of which seem reasonable to me:

• Adjust the code to follow the rules.
• Since this is the standard library, we could alternatively document that we rely on privileged compiler-internal knowledge saying that violating these particular rules in this particular way is actually fine. If the compiler changes in the future in a way that makes this no longer fine, we will have to adjust that code, but since the standard library and the compiler are developed together, that kind of coupling is acceptable. (This would not be an option if the exact same code was written in a regular crate.)

[JmPotato]

@ecstatic-morse Hi, I think we can close this issue now since it has been fixed.