Skip to content

Commit

Permalink
fix pointer aliasing in in_parallel.rs
Browse files Browse the repository at this point in the history
In `in_parallel_with_slice()`, there's a `&mut *input.0` that creates
a `&mut [I]` of the whole slice in `Input`. The same value is created
in multiple threads, thus potentially resulting in multiple threads
having mutable references to the same value.

This patch fixes that by instead only getting mutable pointers to
individual values. It does so by storing a `*mut T` in `Input` instead
of the current `*mut [I]`, and then using `pointer::add()` to get
pointers to individual elements.

Thanks to @Ralith for bringing this up and proposing the solution in
our review of unsafe code at Google.
  • Loading branch information
martinvonz committed Nov 16, 2023
1 parent d75159c commit 1383b0d
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions gix-features/src/parallel/in_parallel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,12 @@ where
.expect("valid name");

let input_len = input.len();
struct Input<I>(*mut [I])
struct Input<I>(*mut I)
where
I: Send;

// SAFETY: I is Send + Sync, so is a *mut [I]
// SAFETY: I is Send, and we only use the pointer for creating new
// pointers (within the input slice) from the threads.
#[allow(unsafe_code)]
unsafe impl<I> Send for Input<I> where I: Send {}

Expand All @@ -245,7 +246,7 @@ where
let new_thread_state = new_thread_state.clone();
let state_to_rval = state_to_rval.clone();
let mut consume = consume.clone();
let input = Input(input as *mut [I]);
let input = Input(input.as_mut_ptr());
move || {
let _ = &input;
threads_left.fetch_sub(1, Ordering::SeqCst);
Expand All @@ -264,7 +265,7 @@ where
let item = {
#[allow(unsafe_code)]
unsafe {
&mut (&mut *input.0)[input_index]
&mut *input.0.add(input_index)
}
};
if let Err(err) = consume(item, &mut state, threads_left, stop_everything) {
Expand Down

0 comments on commit 1383b0d

Please sign in to comment.