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

Infinite loop when trying to fill a Vector of MaybeUninit<SomeZeroSizedType> #80747

Open
PaulGrandperrin opened this issue Jan 6, 2021 · 2 comments
Labels
A-collections Area: std::collections. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@PaulGrandperrin
Copy link

I tried to reduce the demonstration to the minimum possible.

I wrote a function that fills a vector of MaybeUninit<T> values that I think is safe (free of UB).
The code works well with cargo miri run.

But when calling this function with a zero-sized type, the execution goes into an infinite loop.
The exact same code without the MaybeUninit logic works as expected.

Here is the code:

#![feature(maybe_uninit_extra)] // not important, works also without
use core::mem::{self, MaybeUninit};

struct A(u8);
impl Default for A {
    fn default() -> Self {
        Self(0)
    }
}
impl Drop for A {
    fn drop(&mut self) {
        println!("dropping A");
    }
}

struct B;
impl Default for B {
    fn default() -> Self {
        Self
    }
}
impl Drop for B {
    fn drop(&mut self) {
        println!("dropping B");
    }
}

// Fills a vector of uninitialized T with their default values
fn fill_default_uninit<T: Default>(mut v: Vec<MaybeUninit<T>>) -> Vec<T> {
    v.iter_mut().for_each(|i|{
        i.write(T::default()); // without the nightly feature: unsafe{i.as_mut_ptr().write(T::default())};
    });
    unsafe { mem::transmute::<_, _>(v) } // this is safe because the Vec is now fully initialized
}

// Fills a vector of T with their default values, overriding the previous ones
fn fill_default_init<T: Default>(mut v: Vec<T>) -> Vec<T> {
    v.iter_mut().for_each(|i|{
        *i = T::default();
    });
    v
}

fn main() {
    // create an uninitialized vector of 3 As
    let mut v: Vec<MaybeUninit<A>> = Vec::with_capacity(3);
    unsafe{v.set_len(v.capacity())}; // this is safe because the elements are MaybeUninit
    let v = fill_default_uninit(v); // no problem
    drop(v); // all 3 As are dropped
    
    
    // create an uninitialized vector of 3 Bs
    let mut v: Vec<MaybeUninit<B>> = Vec::with_capacity(3);
    unsafe{v.set_len(v.capacity())}; // this is safe because the elements are MaybeUninit
    //let v = init_default(v); //  <------------------------------------------------------ INFINITE LOOP HERE
    //drop(v);
    
    // now let's try the same code for B without MaybeUninit
    let mut v = Vec::with_capacity(3);
    for _ in 0..3 { v.push(B::default()) }

    let v = fill_default_init(v); // no problem, here the 3 old Bs will be dropped
    drop(v); // no problem, the 3 new Bs will be dropped
}

(Playground)

Output:

dropping A
dropping A
dropping A
dropping B
dropping B
dropping B
dropping B
dropping B
dropping B

@SkiFire13
Copy link
Contributor

unsafe{v.set_len(v.capacity())};

Note that since v is a Vec of ZST v.capacity() will always be usize::MAX, even if you created it by using with_capacity. This will result in a really long loop when trying to drop it

I wonder if Vec's and in particular Vec::with_capacity's docs may be a bit more specific on this behaviour.

@PaulGrandperrin
Copy link
Author

PaulGrandperrin commented Jan 6, 2021

@SkiFire13 oh yes, if I change

unsafe{v.set_len(v.capacity())};

to

unsafe{v.set_len(3)};

then the code works fine, so I guess it's fair to say that there are no bugs!

However, it is true that it's surprising that v.capacity() is not equal to the one specified at construction.

I'm willing to do a PR adding that to the doc.
Are there other small surprising behaviors related to Vec (or other std containers) and ZST that I should include?

@camelid camelid added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-collections Area: std::collections. labels Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: std::collections. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants