From 48a044b35132894de4ea7bea3243cc47ddd7837c Mon Sep 17 00:00:00 2001 From: Waffle Date: Sun, 5 Apr 2020 22:15:41 +0300 Subject: [PATCH] remove unsound `MaybeUninitSlice::from_init_mut` and useless `...::from_init` Well, that's weird. I've had a safety comment: ```rust // `MaybeUninit` is guaranteed to have the same ABI as `T`, so // it's safe to cast `&mut [T]` to `&mut [MaybeUninit]` ``` But it's wrong. `&mut T` and `T` isn't the same thing. While it's true that `T => MaybeUninit` 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` 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`: https://github.com/rust-lang/rust/issues/66699 --- src/ext/slice_ext.rs | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/ext/slice_ext.rs b/src/ext/slice_ext.rs index 0806633..f493955 100644 --- a/src/ext/slice_ext.rs +++ b/src/ext/slice_ext.rs @@ -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 Slice for [T] { @@ -223,22 +217,4 @@ impl MaybeUninitSlice for [MaybeUninit] { // `uninit` state &mut *(self as *mut [MaybeUninit] as *mut [T]) } - - #[inline] - fn from_init(init: &[Self::InitItem]) -> &Self { - // ## Safety - // - // `MaybeUninit` is guaranteed to have the same ABI as `T`, so - // it's safe to cast `&[T]` to `&[MaybeUninit]` - unsafe { &*(init as *const [T] as *const [MaybeUninit]) } - } - - #[inline] - fn from_init_mut(init: &mut [Self::InitItem]) -> &mut Self { - // ## Safety - // - // `MaybeUninit` is guaranteed to have the same ABI as `T`, so - // it's safe to cast `&mut [T]` to `&mut [MaybeUninit]` - unsafe { &mut *(init as *mut [T] as *mut [MaybeUninit]) } - } }