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

remove unsound MaybeUninitSlice::from_init_mut and useless `...::fr… #21

Merged
merged 1 commit into from
Apr 5, 2020

Conversation

WaffleLapkin
Copy link
Owner

…om_init`

Well, that's weird. I've had a safety comment:

// `MaybeUninit<T>` is guaranteed to have the same ABI as `T`, so
// it's safe to cast `&mut [T]` to `&mut [MaybeUninit<T>]`

But it's wrong. &mut T and T isn't the same thing. While it's true that T => MaybeUninit<T>
or the same for an owned container (array, box, etc) should be fine, for an unique borrowed
container (&mut _, &mut [_]) it is definitely not fine, because the original owned value
remains T. Example of such a UB in safe code:

let mut a = ["string"];
<_>::from_init_mut(&mut a[..])[0] = MaybeUninit::uninit();
println!("{}", a); // segfault

You can also think of MaybeUninit<T> as a supertype of T and
then note that &mut T is invariant over T: https://doc.rust-lang.org/nomicon/subtyping.html#variance

The weirdest part of all of this is that I haven't tested those functions.
There aren't any tests. There aren't any test for safe function with unsafe in it!

I am ashamed...

A related issue in rust-lang repo, about documenting unsoundness
of &mut T => &mut MaybeUninit<T>: rust-lang/rust#66699

…om_init`

Well, that's weird. I've had a safety comment:
```rust
// `MaybeUninit<T>` is guaranteed to have the same ABI as `T`, so
// it's safe to cast `&mut [T]` to `&mut [MaybeUninit<T>]`
```
But it's wrong. `&mut T` and `T` isn't the same thing. While it's true that `T => MaybeUninit<T>`
or the same for an owned container (array, box, etc) should be fine, for an unique borrowed
container (`&mut _`, `&mut [_]`) it is definitely **not fine**, because the original owned value
remains `T`. Example of such a UB in safe code:

```rust
let mut a = ["string"];
<_>::from_init_mut(&mut a[..])[0] = MaybeUninit::uninit();
println!("{}", a); // segfault
```

You can also think of `MaybeUninit<T>` as a supertype of `T` and
then note that `&mut T` is invariant over `T`: https://doc.rust-lang.org/nomicon/subtyping.html#variance

The weirdest part of all of this is that I haven't tested those functions.
There aren't any tests. There aren't any test for safe function with unsafe in it!

I am ashamed...

A related issue in `rust-lang` repo, about documenting unsoundness
of `&mut T => &mut MaybeUninit<T>`: rust-lang/rust#66699
@WaffleLapkin WaffleLapkin merged commit b0a695e into master Apr 5, 2020
@WaffleLapkin WaffleLapkin deleted the remove_unsoundness branch April 5, 2020 19:18
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 this pull request may close these issues.

1 participant