Merge pull request #1116 from martinvonz/push-rlxsnxnlnkmp

fix pointer aliasing an other issues in ItemSliceSend

Sebastian Thiel committed 2 years ago

Commit 613f0182 parents c65b80b 4f5ab363 files · +33 −16Browse files

Unified · Split

3 files

gix-pack/src/cache/delta/traverse/mod.rs +1 −5
gix-pack/src/cache/delta/traverse/resolve.rs +3 −3
gix-pack/src/cache/delta/traverse/util.rs +29 −8

gix-pack/src/cache/delta/traverse/mod.rs+1−5

@@ -130,7 +130,6 @@ where · 129 unmodified lines
130	130	};
131	131	size_progress.init(None, progress::bytes());
132	132	let size_counter = size_progress.counter();
133		− let child_items = self.child_items.as_mut_slice();
134	133	let object_progress = OwnShared::new(Mutable::new(object_progress));
135	134
136	135	let start = std::time::Instant::now();
@@ -138,10 +137,7 @@ where · 1 unmodified line
138	137	&mut self.root_items,
139	138	thread_limit,
140	139	{
141		− let child_items = ItemSliceSend(std::ptr::slice_from_raw_parts_mut(
142		− child_items.as_mut_ptr(),
143		− child_items.len(),
144		− ));
	140	+ let child_items = ItemSliceSend::new(&mut self.child_items);
145	141	{
146	142	let object_progress = object_progress.clone();
147	143	move \|thread_index\| {

gix-pack/src/cache/delta/traverse/resolve.rs+3−3

@@ -17,13 +17,13 @@ use crate::{ · 16 unmodified lines
17	17	data::EntryRange,
18	18	};
19	19
20		−pub(crate) struct State<F, MBFN, T: Send> {
	20	+pub(crate) struct State<'items, F, MBFN, T: Send> {
21	21	pub delta_bytes: Vec<u8>,
22	22	pub fully_resolved_delta_bytes: Vec<u8>,
23	23	pub progress: Box<dyn Progress>,
24	24	pub resolve: F,
25	25	pub modify_base: MBFN,
26		− pub child_items: ItemSliceSend<Item<T>>,
	26	+ pub child_items: ItemSliceSend<'items, Item<T>>,
27	27	}
28	28
29	29	#[allow(clippy::too_many_arguments)]
@@ -38,7 +38,7 @@ pub(crate) fn deltas<T, F, MBFN, E, R>( · 8 unmodified lines
38	38	resolve,
39	39	modify_base,
40	40	child_items,
41		− }: &mut State<F, MBFN, T>,
	41	+ }: &mut State<'_, F, MBFN, T>,
42	42	resolve_data: &R,
43	43	hash_len: usize,
44	44	threads_left: &AtomicIsize,

gix-pack/src/cache/delta/traverse/util.rs+29−8

@@ -1,28 +1,49 @@
	1	+use std::marker::PhantomData;
	2	+
1	3	use crate::cache::delta::Item;
2	4
3		−pub struct ItemSliceSend<T>(pub *mut [T])
	5	+pub(crate) struct ItemSliceSend<'a, T>
	6	+where
	7	+ T: Send,
	8	+{
	9	+ items: *mut T,
	10	+ phantom: PhantomData<&'a T>,
	11	+}
	12	+
	13	+impl<'a, T> ItemSliceSend<'a, T>
4	14	where
5		− T: Send;
	15	+ T: Send,
	16	+{
	17	+ pub fn new(items: &'a mut [T]) -> Self {
	18	+ ItemSliceSend {
	19	+ items: items.as_mut_ptr(),
	20	+ phantom: PhantomData,
	21	+ }
	22	+ }
	23	+}
6	24
7	25	/// 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
8	26	/// don't violate aliasing rules.
9		−impl<T> Clone for ItemSliceSend<T>
	27	+impl<T> Clone for ItemSliceSend<'_, T>
10	28	where
11	29	T: Send,
12	30	{
13	31	fn clone(&self) -> Self {
14		− ItemSliceSend(self.0)
	32	+ ItemSliceSend {
	33	+ items: self.items,
	34	+ phantom: self.phantom,
	35	+ }
15	36	}
16	37	}
17	38
18	39	// 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.
19	40	#[allow(unsafe_code)]
20		−unsafe impl<T> Send for ItemSliceSend<T> where T: Send {}
	41	+unsafe impl<T> Send for ItemSliceSend<'_, T> where T: Send {}
21	42
22	43	/// An item returned by `iter_root_chunks`, allowing access to the `data` stored alongside nodes in a [`Tree`].
23		−pub struct Node<'a, T: Send> {
	44	+pub(crate) struct Node<'a, T: Send> {
24	45	pub item: &'a mut Item<T>,
25		− pub child_items: ItemSliceSend<Item<T>>,
	46	+ pub child_items: ItemSliceSend<'a, Item<T>>,
26	47	}
27	48
28	49	impl<'a, T: Send> Node<'a, T> {
@@ -57,7 +78,7 @@ impl<'a, T: Send> Node<'a, T> { · 28 unmodified lines
57	78	// SAFETY: The resulting mutable pointer cannot be yielded by any other node.
58	79	#[allow(unsafe_code)]
59	80	Node {
60		− item: &mut unsafe { &mut *children.0 }[index as usize],
	81	+ item: unsafe { &mut *children.items.add(index as usize) },
61	82	child_items: children.clone(),
62	83	}
63	84	})