Skip to content

Commit

Permalink
remove unsound MaybeUninitSlice::from_init_mut and useless `...::fr…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
WaffleLapkin committed Apr 5, 2020
1 parent 9fa1aa2 commit 48a044b
Showing 1 changed file with 0 additions and 24 deletions.
24 changes: 0 additions & 24 deletions src/ext/slice_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,6 @@ pub trait MaybeUninitSlice:
/// [inv]: core::mem#initialization-invariant
/// [`MaybeUninit::assume_init`]: core::mem::MaybeUninit::assume_init
unsafe fn assume_init_mut(&mut self) -> &mut [Self::InitItem];

/// Create self from initialized slice
fn from_init(init: &[Self::InitItem]) -> &Self;

/// Create self from initialized slice
fn from_init_mut(init: &mut [Self::InitItem]) -> &mut Self;
}

impl<T> Slice for [T] {
Expand Down Expand Up @@ -223,22 +217,4 @@ impl<T> MaybeUninitSlice for [MaybeUninit<T>] {
// `uninit` state
&mut *(self as *mut [MaybeUninit<T>] as *mut [T])
}

#[inline]
fn from_init(init: &[Self::InitItem]) -> &Self {
// ## Safety
//
// `MaybeUninit<T>` is guaranteed to have the same ABI as `T`, so
// it's safe to cast `&[T]` to `&[MaybeUninit<T>]`
unsafe { &*(init as *const [T] as *const [MaybeUninit<T>]) }
}

#[inline]
fn from_init_mut(init: &mut [Self::InitItem]) -> &mut Self {
// ## Safety
//
// `MaybeUninit<T>` is guaranteed to have the same ABI as `T`, so
// it's safe to cast `&mut [T]` to `&mut [MaybeUninit<T>]`
unsafe { &mut *(init as *mut [T] as *mut [MaybeUninit<T>]) }
}
}

0 comments on commit 48a044b

Please sign in to comment.