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 UB reported by miri in AtomicBitSet #61

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

Imberflur
Copy link
Contributor

  • Also modify some spots that have &mut self to use get_mut instead of atomic ops.

Miri reports UB for atom::AtomSetOnce::get (used internally in AtomicBitSet) since it gets a pointer from a Box via dereference and then mem::forgets the Box which moves the Box (invalidating the pointer since the Box pointer is "unique").

Since we aren't using anything else from the atom crate (and it has several other open soundness issues) it is far simpler to provide a minimal implementation of what we need here. once_cell::race::OnceBox is close to what we need here but doesn't provide get_mut and due to its current MSRV has stronger than necessary ordering for the compare_exchange failure case.

* Also modify some spots that have `&mut self` to use `get_mut` instead
  of atomic ops.

Miri reports UB for `atom::AtomSetOnce::get` (used internally in
`AtomicBitSet`) since it gets a pointer from a Box via dereference and
then `mem::forget`s the Box which moves the Box (invalidating the
pointer since the Box pointer is "unique").

Since we aren't using anything else from the `atom` crate (and it has
several other open soundness issues) it is far simpler to provide a
minimal implementation of what we need here. `once_cell::race::OnceBox`
is close to what we need here but doesn't provide `get_mut` and due to
its current MSRV has stronger than necessary ordering for the
compare_exchange failure case.
@zesterer
Copy link
Contributor

Thanks!

@zesterer zesterer merged commit 058e13c into amethyst:master Feb 27, 2023
@Imberflur Imberflur mentioned this pull request Mar 1, 2023
13 tasks
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.

2 participants