Skip to content

MaybeUninit::uninit().assume_init() is undefined behavior #536

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

Closed
ExpHP opened this issue Jul 14, 2019 · 3 comments
Closed

MaybeUninit::uninit().assume_init() is undefined behavior #536

ExpHP opened this issue Jul 14, 2019 · 3 comments
Labels

Comments

@ExpHP
Copy link

ExpHP commented Jul 14, 2019

pyo3/src/gil.rs

Lines 303 to 306 in 83d0ac4

if next_idx == 0 {
self.inner
.push_back(unsafe { mem::MaybeUninit::uninit().assume_init() });
}

This is explicitly noted in the MaybeUninit docs as undefined behavior (std::mem:;unintialized was deprecated for a reason!). The correct solution is to store [MaybeUninit<T>; BLOCK_SIZE] in the type, and to only call assume_init() on individual items that are known to be initialized.

(also unrelated: the current implementation of ArrayList<T> is not safe for T that have destructors due to its use of indexed assignment instead of ptr::write. It should have a T: Copy bound to reflect this)

@ExpHP
Copy link
Author

ExpHP commented Jul 14, 2019

To make matters even worse, you're using it for NonZero<*mut T>, a type which has invalid bit patterns!

(not that that makes a difference; mem::MaybeUninit::uninit().assume_init() is already undefined behavior for all types! This fact merely makes it... deadlier)

@kngwyu
Copy link
Member

kngwyu commented Jul 15, 2019

Good catch, thanks.

@kngwyu
Copy link
Member

kngwyu commented Jul 15, 2019

Closed via #537 for now

@kngwyu kngwyu closed this as completed Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants