Skip to content

Commit 0d8def7

Browse files
saethlincuviper
authored andcommitted
Fix provenance issues
The uninitialized region of a Vec cannot be accessed by slicing the Vec first, see rust-lang/rust#92097 Similarly, it is never valid to merge adjacent slices, because any pointer derived from a slice only has provenance over that slice, not anything adjacent. So we pass raw pointers and a length around to avoid narrowing provenance by converting to a reference.
1 parent 1c5277f commit 0d8def7

3 files changed

Lines changed: 79 additions & 47 deletions

File tree

src/iter/collect/consumer.rs

Lines changed: 66 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,34 @@ use std::mem::MaybeUninit;
44
use std::ptr;
55
use std::slice;
66

7+
/// We need to store raw pointers in a `Send` struct to remember
8+
/// provenance (see `CollectResult`).
9+
#[derive(Clone, Copy)]
10+
struct SendPtr<T>(*mut T);
11+
12+
// SAFETY: !Send for raw pointers is not for safety, just as a lint
13+
unsafe impl<T> Send for SendPtr<T> {}
14+
715
pub(super) struct CollectConsumer<'c, T: Send> {
8-
/// A slice covering the target memory, not yet initialized!
9-
target: &'c mut [MaybeUninit<T>],
16+
/// See `CollectConsumer` for explanation of why this is not a slice
17+
start: SendPtr<MaybeUninit<T>>,
18+
len: usize,
19+
marker: PhantomData<&'c mut T>,
1020
}
1121

1222
impl<'c, T: Send + 'c> CollectConsumer<'c, T> {
1323
/// The target memory is considered uninitialized, and will be
1424
/// overwritten without reading or dropping existing values.
15-
pub(super) fn new(target: &'c mut [MaybeUninit<T>]) -> Self {
16-
CollectConsumer { target }
25+
pub(super) fn new(
26+
start: *mut MaybeUninit<T>,
27+
len: usize,
28+
marker: PhantomData<&'c mut T>,
29+
) -> Self {
30+
CollectConsumer {
31+
start: SendPtr(start),
32+
len,
33+
marker,
34+
}
1735
}
1836
}
1937

@@ -23,10 +41,14 @@ impl<'c, T: Send + 'c> CollectConsumer<'c, T> {
2341
/// the elements will be dropped, unless its ownership is released before then.
2442
#[must_use]
2543
pub(super) struct CollectResult<'c, T> {
26-
/// A slice covering the target memory, initialized up to our separate `len`.
27-
target: &'c mut [MaybeUninit<T>],
28-
/// The current initialized length in `target`
29-
len: usize,
44+
/// This pointer and length has the same representation as a slice,
45+
/// but retains the provenance of the entire array so that we can merge
46+
/// these regions together in `CollectReducer`.
47+
/// Constructing a slice from this start + start_l
48+
start: SendPtr<MaybeUninit<T>>,
49+
total_len: usize,
50+
/// The current initialized length after `start`
51+
initialized_len: usize,
3052
/// Lifetime invariance guarantees that the data flows from consumer to result,
3153
/// especially for the `scope_fn` callback in `Collect::with_consumer`.
3254
invariant_lifetime: PhantomData<&'c mut &'c mut [T]>,
@@ -37,25 +59,26 @@ unsafe impl<'c, T> Send for CollectResult<'c, T> where T: Send {}
3759
impl<'c, T> CollectResult<'c, T> {
3860
/// The current length of the collect result
3961
pub(super) fn len(&self) -> usize {
40-
self.len
62+
self.initialized_len
4163
}
4264

4365
/// Release ownership of the slice of elements, and return the length
4466
pub(super) fn release_ownership(mut self) -> usize {
45-
let ret = self.len;
46-
self.len = 0;
67+
let ret = self.initialized_len;
68+
self.initialized_len = 0;
4769
ret
4870
}
4971
}
5072

5173
impl<'c, T> Drop for CollectResult<'c, T> {
5274
fn drop(&mut self) {
53-
// Drop the first `self.len` elements, which have been recorded
75+
// Drop the first `self.initialized_len` elements, which have been recorded
5476
// to be initialized by the folder.
5577
unsafe {
56-
// TODO: use `MaybeUninit::slice_as_mut_ptr`
57-
let start = self.target.as_mut_ptr() as *mut T;
58-
ptr::drop_in_place(slice::from_raw_parts_mut(start, self.len));
78+
ptr::drop_in_place(slice::from_raw_parts_mut(
79+
self.start.0 as *mut T,
80+
self.initialized_len,
81+
));
5982
}
6083
}
6184
}
@@ -66,24 +89,27 @@ impl<'c, T: Send + 'c> Consumer<T> for CollectConsumer<'c, T> {
6689
type Result = CollectResult<'c, T>;
6790

6891
fn split_at(self, index: usize) -> (Self, Self, CollectReducer) {
69-
let CollectConsumer { target } = self;
70-
71-
// Produce new consumers. Normal slicing ensures that the
72-
// memory range given to each consumer is disjoint.
73-
let (left, right) = target.split_at_mut(index);
74-
(
75-
CollectConsumer::new(left),
76-
CollectConsumer::new(right),
77-
CollectReducer,
78-
)
92+
let CollectConsumer { start, len, marker } = self;
93+
94+
// Produce new consumers.
95+
// SAFETY: This assert checks that `index` is a valid offset for `start`
96+
unsafe {
97+
assert!(index <= len);
98+
(
99+
CollectConsumer::new(start.0, index, marker),
100+
CollectConsumer::new(start.0.add(index), len - index, marker),
101+
CollectReducer,
102+
)
103+
}
79104
}
80105

81106
fn into_folder(self) -> Self::Folder {
82107
// Create a result/folder that consumes values and writes them
83-
// into target. The initial result has length 0.
108+
// into the region after start. The initial result has length 0.
84109
CollectResult {
85-
target: self.target,
86-
len: 0,
110+
start: self.start,
111+
total_len: self.len,
112+
initialized_len: 0,
87113
invariant_lifetime: PhantomData,
88114
}
89115
}
@@ -97,15 +123,15 @@ impl<'c, T: Send + 'c> Folder<T> for CollectResult<'c, T> {
97123
type Result = Self;
98124

99125
fn consume(mut self, item: T) -> Self {
100-
let dest = self
101-
.target
102-
.get_mut(self.len)
103-
.expect("too many values pushed to consumer");
126+
assert!(
127+
self.initialized_len < self.total_len,
128+
"too many values pushed to consumer"
129+
);
104130

105131
// Write item and increase the initialized length
106132
unsafe {
107-
dest.as_mut_ptr().write(item);
108-
self.len += 1;
133+
(*self.start.0.add(self.initialized_len)).write(item);
134+
self.initialized_len += 1;
109135
}
110136

111137
self
@@ -146,14 +172,13 @@ impl<'c, T> Reducer<CollectResult<'c, T>> for CollectReducer {
146172
// Merge if the CollectResults are adjacent and in left to right order
147173
// else: drop the right piece now and total length will end up short in the end,
148174
// when the correctness of the collected result is asserted.
149-
let left_end = left.target[left.len..].as_ptr();
150-
if left_end == right.target.as_ptr() {
151-
let len = left.len + right.release_ownership();
152-
unsafe {
153-
left.target = slice::from_raw_parts_mut(left.target.as_mut_ptr(), len);
175+
unsafe {
176+
let left_end = left.start.0.add(left.initialized_len);
177+
if left_end == right.start.0 {
178+
left.total_len += right.total_len;
179+
left.initialized_len += right.release_ownership();
154180
}
155-
left.len = len;
181+
left
156182
}
157-
left
158183
}
159184
}

src/iter/collect/mod.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,11 @@ impl<'c, T: Send + 'c> Collect<'c, T> {
9090
F: FnOnce(CollectConsumer<'_, T>) -> CollectResult<'_, T>,
9191
{
9292
let slice = Self::reserve_get_tail_slice(&mut self.vec, self.len);
93-
let result = scope_fn(CollectConsumer::new(slice));
93+
let result = scope_fn(CollectConsumer::new(
94+
slice.as_mut_ptr(),
95+
slice.len(),
96+
std::marker::PhantomData,
97+
));
9498

9599
// The CollectResult represents a contiguous part of the
96100
// slice, that has been written to.
@@ -136,8 +140,12 @@ impl<'c, T: Send + 'c> Collect<'c, T> {
136140
// TODO: use `Vec::spare_capacity_mut` instead
137141
// SAFETY: `MaybeUninit<T>` is guaranteed to have the same layout
138142
// as `T`, and we already made sure to have the additional space.
143+
// This pointer is derived from `Vec` directly, not through a `Deref`,
144+
// so it has provenance over the whole allocation.
139145
let start = vec.len();
140-
let tail_ptr = vec[start..].as_mut_ptr() as *mut MaybeUninit<T>;
141-
unsafe { slice::from_raw_parts_mut(tail_ptr, len) }
146+
unsafe {
147+
let tail_ptr = vec.as_mut_ptr() as *mut MaybeUninit<T>;
148+
slice::from_raw_parts_mut(tail_ptr.add(start), len)
149+
}
142150
}
143151
}

src/vec.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,8 @@ impl<'data, T: Send> IndexedParallelIterator for Drain<'data, T> {
143143

144144
// Create the producer as the exclusive "owner" of the slice.
145145
let producer = {
146-
// Get a correct borrow lifetime, then extend it to the original length.
147-
let mut slice = &mut self.vec[start..];
148-
slice = slice::from_raw_parts_mut(slice.as_mut_ptr(), self.range.len());
146+
let ptr = self.vec.as_mut_ptr().add(start);
147+
let slice = slice::from_raw_parts_mut(ptr, self.range.len());
149148
DrainProducer::new(slice)
150149
};
151150

0 commit comments

Comments
 (0)