Dropping SmallVecN is unsound

[SimonSapin]

For example, this segfaults:

SmallVec4::<Box<u32>>::new();

SmallVec4<T> contains a [T; 4] field directly, which is initialized in new with std::mem::zeroed(). When the vector is dropped, the destructor for T is run for each of the 4 Ts, even if there isn’t actually a T there (i.e. if the vector’s length is less than 4 by the time it is dropped).

https://github.com/bluss/arrayvec works around this issue by having (simplified):

enum Array<T> {
    Alive([T, 4]),
    Dropped,
}

with a destructor that resets to Dropped before the recursive destructors are run implicitly.

In SmallVecN, the second variant could instead contain the pointer and capacity for a spilled vector (reset to null/zero during destruction).

[SimonSapin]

CC @pcwalton

[SimonSapin]

I think this used to be sound when Rust had both drop flags and zeroing-on-drop, but that changed in rust-lang/rust#23535.

We could probably go back to the old behavior by initializing with std::mem::POST_DROP_U8 instead of zeroes, but I think that’s not a good idea since drop flags are probably going away entirely.

rust-lang/rfcs#197 is also relevant.

[bluss]

The replacement for zeroed is mem::dropped with filling drop.

[SimonSapin]

Using dropped instead of zeroed could be a short-term fix, but it requires #![feature(filling_drop)] whereas the library currently runs (although brokenly) on Rust stable. I’d prefer using NoDrop from arrayvec.