Merge pull request #1116 from martinvonz/push-rlxsnxnlnkmp

fix pointer aliasing an other issues in ItemSliceSend

Author: Byron

gix-pack/src/cache/delta/traverse/mod.rs

@@ -130,18 +130,14 @@ where
         };
         size_progress.init(None, progress::bytes());
         let size_counter = size_progress.counter();
-        let child_items = self.child_items.as_mut_slice();
         let object_progress = OwnShared::new(Mutable::new(object_progress));
 
         let start = std::time::Instant::now();
         in_parallel_with_slice(
             &mut self.root_items,
             thread_limit,
             {
-                let child_items = ItemSliceSend(std::ptr::slice_from_raw_parts_mut(
-                    child_items.as_mut_ptr(),
-                    child_items.len(),
-                ));
+                let child_items = ItemSliceSend::new(&mut self.child_items);
                 {
                     let object_progress = object_progress.clone();
                     move |thread_index| {

gix-pack/src/cache/delta/traverse/resolve.rs

@@ -17,13 +17,13 @@ use crate::{
     data::EntryRange,
 };
 
-pub(crate) struct State<F, MBFN, T: Send> {
+pub(crate) struct State<'items, F, MBFN, T: Send> {
     pub delta_bytes: Vec<u8>,
     pub fully_resolved_delta_bytes: Vec<u8>,
     pub progress: Box<dyn Progress>,
     pub resolve: F,
     pub modify_base: MBFN,
-    pub child_items: ItemSliceSend<Item<T>>,
+    pub child_items: ItemSliceSend<'items, Item<T>>,
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -38,7 +38,7 @@ pub(crate) fn deltas<T, F, MBFN, E, R>(
         resolve,
         modify_base,
         child_items,
-    }: &mut State<F, MBFN, T>,
+    }: &mut State<'_, F, MBFN, T>,
     resolve_data: &R,
     hash_len: usize,
     threads_left: &AtomicIsize,

gix-pack/src/cache/delta/traverse/util.rs

@@ -1,28 +1,49 @@
+use std::marker::PhantomData;
+
 use crate::cache::delta::Item;
 
-pub struct ItemSliceSend<T>(pub *mut [T])
+pub(crate) struct ItemSliceSend<'a, T>
+where
+    T: Send,
+{
+    items: *mut T,
+    phantom: PhantomData<&'a T>,
+}
+
+impl<'a, T> ItemSliceSend<'a, T>
 where
-    T: Send;
+    T: Send,
+{
+    pub fn new(items: &'a mut [T]) -> Self {
+        ItemSliceSend {
+            items: items.as_mut_ptr(),
+            phantom: PhantomData,
+        }
+    }
+}
 
 /// SAFETY: This would be unsafe if this would ever be abused, but it's used internally and only in a way that assure that the pointers
 ///         don't violate aliasing rules.
-impl<T> Clone for ItemSliceSend<T>
+impl<T> Clone for ItemSliceSend<'_, T>
 where
     T: Send,
 {
     fn clone(&self) -> Self {
-        ItemSliceSend(self.0)
+        ItemSliceSend {
+            items: self.items,
+            phantom: self.phantom,
+        }
     }
 }
 
 // SAFETY: T is `Send`, and we only ever access one T at a time. And, ptrs need that assurance, I wonder if it's always right.
 #[allow(unsafe_code)]
-unsafe impl<T> Send for ItemSliceSend<T> where T: Send {}
+unsafe impl<T> Send for ItemSliceSend<'_, T> where T: Send {}
 
 /// An item returned by `iter_root_chunks`, allowing access to the `data` stored alongside nodes in a [`Tree`].
-pub struct Node<'a, T: Send> {
+pub(crate) struct Node<'a, T: Send> {
     pub item: &'a mut Item<T>,
-    pub child_items: ItemSliceSend<Item<T>>,
+    pub child_items: ItemSliceSend<'a, Item<T>>,
 }
 
 impl<'a, T: Send> Node<'a, T> {
@@ -57,7 +78,7 @@ impl<'a, T: Send> Node<'a, T> {
             // SAFETY: The resulting mutable pointer cannot be yielded by any other node.
             #[allow(unsafe_code)]
             Node {
-                item: &mut unsafe { &mut *children.0 }[index as usize],
+                item: unsafe { &mut *children.items.add(index as usize) },
                 child_items: children.clone(),
             }
         })