-
Notifications
You must be signed in to change notification settings - Fork 28
unwind safety fixes #140
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
base: main
Are you sure you want to change the base?
unwind safety fixes #140
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -278,6 +278,108 @@ impl<T: ?Sized> Drop for DropGuard<T> { | |
| } | ||
| } | ||
|
|
||
| /// Allows safe (pinned and non-pinned) initialization of an array. | ||
| /// | ||
| /// Drops the already initialized elements of the array if an error or panic occurs | ||
| /// partway through the initialization process. | ||
| /// | ||
| /// # Invariants | ||
| /// | ||
| /// - `ptr` is null: | ||
| /// - before `__init` or `__pinned_init` is called | ||
| /// - after the array is successfully initialized | ||
| /// - `ptr[0..num_init]` contains initialized elements of type `T` | ||
| /// - `ptr[num_init..N]` (where N is the size of the array) contains uninitialized memory | ||
| pub(crate) struct ArrayInit<T, F> { | ||
| /// A pointer to the first element of the array. | ||
| ptr: *mut T, | ||
| /// The number of initialized elements in the array. | ||
| num_init: usize, | ||
| /// Initialization function factory. | ||
| make_init: F, | ||
| } | ||
|
|
||
| impl<T, F> ArrayInit<T, F> { | ||
| pub(crate) fn new(make_init: F) -> Self { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs ``#[inline]` |
||
| Self { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need |
||
| ptr: core::ptr::null_mut(), | ||
| num_init: 0, | ||
| make_init, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// SAFETY: On success, all `N` elements of the array have been initialized through | ||
| /// `I: Init`. On error or panic, the elements that have been initialized so far are | ||
| /// dropped, thus leaving the array uninitialized and ready to deallocate. The `Init` | ||
| /// implementation executes the same code as that of `PinInit`. | ||
| unsafe impl<T, F, I, E, const N: usize> Init<[T; N], E> for ArrayInit<T, F> | ||
| where | ||
| F: FnMut(usize) -> I, | ||
| I: Init<T, E>, | ||
| { | ||
| unsafe fn __init(mut self, slot: *mut [T; N]) -> Result<(), E> { | ||
| self.ptr = slot.cast::<T>(); | ||
| for i in 0..N { | ||
| let init = (self.make_init)(i); | ||
| // SAFETY: Since `0 <= i < N`, `self.ptr.add(i)` is in bounds and | ||
| // valid for writes by the safety contract of `__init`. | ||
|
Comment on lines
+325
to
+326
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not calling |
||
| let ptr = unsafe { self.ptr.add(i) }; | ||
| // SAFETY: The pointer is derived from `slot` and thus satisfies the | ||
| // `__init` requirements. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be more explicit and refers to the safety requirements of |
||
| unsafe { init.__init(ptr) }?; | ||
| self.num_init += 1; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd actually either put |
||
| } | ||
| self.ptr = core::ptr::null_mut(); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| /// SAFETY: On success, all `N` elements of the array have been initialized through | ||
| /// `I`. Since `I: PinInit` guarantees that the pinning invariants of `T` are upheld, | ||
| /// the guarantees of `[T; N]` are also upheld. On error or panic, the elements that | ||
| /// have been initialized so far are dropped, thus leaving the array uninitialized | ||
| /// and ready to deallocate. | ||
| unsafe impl<T, F, I, E, const N: usize> PinInit<[T; N], E> for ArrayInit<T, F> | ||
| where | ||
| F: FnMut(usize) -> I, | ||
| I: PinInit<T, E>, | ||
| { | ||
| unsafe fn __pinned_init(mut self, slot: *mut [T; N]) -> Result<(), E> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto for this function |
||
| self.ptr = slot.cast::<T>(); | ||
| for i in 0..N { | ||
| let init = (self.make_init)(i); | ||
| // SAFETY: Since `0 <= i < N`, `self.ptr.add(i)` is in bounds and | ||
| // valid for writes by the safety contract of `__pinned_init`. | ||
| let ptr = unsafe { self.ptr.add(i) }; | ||
| // SAFETY: The pointer is derived from `slot` and thus satisfies the | ||
| // `__pinned_init` requirements. | ||
| unsafe { init.__pinned_init(ptr) }?; | ||
| self.num_init += 1; | ||
| } | ||
| self.ptr = core::ptr::null_mut(); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| impl<T, F> Drop for ArrayInit<T, F> { | ||
| fn drop(&mut self) { | ||
| if self.ptr.is_null() { | ||
| // Either no initialization has been attempted or the array was | ||
| // successfully initialized. Nothing to drop. | ||
| return; | ||
| } | ||
|
|
||
| // SAFETY: Invariants of `ArrayInit` guarantee that elements | ||
| // `self.ptr[0..self.num_init]` are initialized and contain valid `T` | ||
| // values, so dropping them is safe. | ||
|
Comment on lines
+373
to
+375
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps also mention before the safety comment that "In this code path, the initialization has failed partway and we need to drop elements initialized thus far to uphold the pinning requirement." |
||
| unsafe { | ||
| let slice = core::ptr::slice_from_raw_parts_mut(self.ptr, self.num_init); | ||
| core::ptr::drop_in_place(slice) | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| /// Token used by `PinnedDrop` to prevent calling the function without creating this unsafely | ||
| /// created struct. This is needed, because the `drop` function is safe, but should not be called | ||
| /// manually. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -966,12 +966,17 @@ where | |
| unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> { | ||
| // SAFETY: All requirements fulfilled since this function is `__pinned_init`. | ||
| unsafe { self.0.__pinned_init(slot)? }; | ||
| // SAFETY: The above call initialized `slot` and we still have unique access. | ||
| // SAFETY: The above call initialized `slot`. The guard drops it if `self.1` returns | ||
| // `Err` or panics. | ||
| let guard = unsafe { __internal::DropGuard::new(slot) }; | ||
| // SAFETY: `slot` was initialized above. Excluding the guard (which only drops) we | ||
| // have unique access to it. | ||
| let val = unsafe { &mut *slot }; | ||
| // SAFETY: `slot` is considered pinned. | ||
| let val = unsafe { Pin::new_unchecked(val) }; | ||
| // SAFETY: `slot` was initialized above. | ||
| (self.1)(val).inspect_err(|_| unsafe { core::ptr::drop_in_place(slot) }) | ||
| (self.1)(val)?; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The whole implementation here can probably be simplified to something like
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could rebase this branch to |
||
| core::mem::forget(guard); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1073,10 +1078,14 @@ where | |
| unsafe fn __init(self, slot: *mut T) -> Result<(), E> { | ||
| // SAFETY: All requirements fulfilled since this function is `__init`. | ||
| unsafe { self.0.__pinned_init(slot)? }; | ||
| // SAFETY: The above call initialized `slot` and we still have unique access. | ||
| (self.1)(unsafe { &mut *slot }).inspect_err(|_| | ||
| // SAFETY: `slot` was initialized above. | ||
| unsafe { core::ptr::drop_in_place(slot) }) | ||
| // SAFETY: The above call initialized `slot`. The guard drops it if `self.1` returns | ||
| // `Err` or panics. | ||
| let guard = unsafe { __internal::DropGuard::new(slot) }; | ||
| // SAFETY: `slot` was initialized above. Excluding the guard (which only drops) we | ||
| // have unique access to it. | ||
| (self.1)(unsafe { &mut *slot })?; | ||
| core::mem::forget(guard); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1188,31 +1197,12 @@ pub fn uninit<T, E>() -> impl Init<MaybeUninit<T>, E> { | |
| /// assert_eq!(array.len(), 1_000); | ||
| /// ``` | ||
| pub fn init_array_from_fn<I, const N: usize, T, E>( | ||
| mut make_init: impl FnMut(usize) -> I, | ||
| make_init: impl FnMut(usize) -> I, | ||
| ) -> impl Init<[T; N], E> | ||
| where | ||
| I: Init<T, E>, | ||
| { | ||
| let init = move |slot: *mut [T; N]| { | ||
| let slot = slot.cast::<T>(); | ||
| for i in 0..N { | ||
| let init = make_init(i); | ||
| // SAFETY: Since 0 <= `i` < N, it is still in bounds of `[T; N]`. | ||
| let ptr = unsafe { slot.add(i) }; | ||
| // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init` | ||
| // requirements. | ||
| if let Err(e) = unsafe { init.__init(ptr) } { | ||
| // SAFETY: The loop has initialized the elements `slot[0..i]` and since we return | ||
| // `Err` below, `slot` will be considered uninitialized memory. | ||
| unsafe { ptr::drop_in_place(ptr::slice_from_raw_parts_mut(slot, i)) }; | ||
| return Err(e); | ||
| } | ||
| } | ||
| Ok(()) | ||
| }; | ||
| // SAFETY: The initializer above initializes every element of the array. On failure it drops | ||
| // any initialized elements and returns `Err`. | ||
| unsafe { init_from_closure(init) } | ||
| __internal::ArrayInit::new(make_init) | ||
| } | ||
|
|
||
| /// Initializes an array by initializing each element via the provided initializer. | ||
|
|
@@ -1231,31 +1221,12 @@ where | |
| /// assert_eq!(array.len(), 1_000); | ||
| /// ``` | ||
| pub fn pin_init_array_from_fn<I, const N: usize, T, E>( | ||
| mut make_init: impl FnMut(usize) -> I, | ||
| make_init: impl FnMut(usize) -> I, | ||
| ) -> impl PinInit<[T; N], E> | ||
| where | ||
| I: PinInit<T, E>, | ||
| { | ||
| let init = move |slot: *mut [T; N]| { | ||
| let slot = slot.cast::<T>(); | ||
| for i in 0..N { | ||
| let init = make_init(i); | ||
| // SAFETY: Since 0 <= `i` < N, it is still in bounds of `[T; N]`. | ||
| let ptr = unsafe { slot.add(i) }; | ||
| // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init` | ||
| // requirements. | ||
| if let Err(e) = unsafe { init.__pinned_init(ptr) } { | ||
| // SAFETY: The loop has initialized the elements `slot[0..i]` and since we return | ||
| // `Err` below, `slot` will be considered uninitialized memory. | ||
| unsafe { ptr::drop_in_place(ptr::slice_from_raw_parts_mut(slot, i)) }; | ||
| return Err(e); | ||
| } | ||
| } | ||
| Ok(()) | ||
| }; | ||
| // SAFETY: The initializer above initializes every element of the array. On failure it drops | ||
| // any initialized elements and returns `Err`. | ||
| unsafe { pin_init_from_closure(init) } | ||
| __internal::ArrayInit::new(make_init) | ||
| } | ||
|
|
||
| /// Construct an initializer in a closure and run it. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.