Skip to content
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

Possible UB from use of uninitialized [&T; N] #126

Closed
mbrubeck opened this issue Sep 25, 2018 · 4 comments · Fixed by #162
Closed

Possible UB from use of uninitialized [&T; N] #126

mbrubeck opened this issue Sep 25, 2018 · 4 comments · Fixed by #162

Comments

@mbrubeck
Copy link
Collaborator

mbrubeck commented Sep 25, 2018

Creating an array with mem::uninitialized may be UB if the array's elements contain non-null types (like &T) or uninhabited types (like !) or other types that have invalid values.

This can result in undefined behavior even if you never read the elements directly while they are uninitialized. For example, this program prints "true" if optimizations are enabled, when built with current rustc:

fn main() {
    let x: Option<[&u8; 1]> = unsafe { Some(std::mem::uninitialized()) };
    println!("{}", x.is_none());
}

SmallVec isn't eligible for null pointer optimization, so it isn't affected by this. However, it's possible that a future Rust compiler could add other optimizations that break if SmallVec uses mem::uninitialized to create arrays of such types.

The ideal solution is to switch from uninitialized to the new MaybeUninit union (rust-lang/rfcs#1892) when it becomes stable.

@mbrubeck
Copy link
Collaborator Author

See also rust-lang/rust#54542.

@bluss
Copy link
Contributor

bluss commented Dec 3, 2018

ManuallyDrop<T> changed to allow enum layout optimization using the interior value, so it regressed this crate in that way. rust-lang/rust/pull/52711

I can't make the test fail, but I think we have a problem hiding in here, as long as an optional smallvec of references is the same size as the plain type:

https://play.rust-lang.org/?version=stable&mode=release&edition=2015&gist=471955af6ec77850bf98446aba013c35

@mbrubeck
Copy link
Collaborator Author

mbrubeck commented Dec 3, 2018

SmallVec contains an enum, so I think the enum layout optimization is using spare bits from that enum's tag to store the Option discriminant. If this is true then we're not seeing incorrect behavior in practice yet.

@llogiq
Copy link
Contributor

llogiq commented Mar 30, 2019

I recently looked into the use of MaybeUninit<T> in arrays. We'd need to extend the Array trait with an associated type of type InternalArray = [MaybeUninit<$ty>; $n] and use that internally. However that would require the heap variant to become (*mut MaybeUninit<A::Item>, usize) to ensure UB-freedom.

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.

3 participants