Skip to content
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

fix pointer aliasing in in_parallel.rs #1115

Merged
merged 1 commit into from Nov 16, 2023

Conversation

martinvonz
Copy link
Contributor

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.

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.
@Byron
Copy link
Owner

Byron commented Nov 16, 2023

Thanks so much! I don't even know how I ended up with keeping a slice instead of a pointer, and I am very thankful for any improvements to the (few) unsafe spots in this codebase.

I really, really, don't like writing it and am not very familiar with it either, so please do keep this PRs coming :).

@Byron Byron merged commit c65b80b into Byron:main Nov 16, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants