Don't call assume_init on Deferred's Data by saethlin · Pull Request #779 · crossbeam-rs/crossbeam · GitHub
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

Don't call assume_init on Deferred's Data #779

Merged
merged 1 commit into from
Feb 5, 2022

Conversation

saethlin
Copy link
Contributor

@saethlin saethlin commented Feb 5, 2022

This situation can be observed by running MIRIFLAGS="-Zmiri-check-number-validity" cargo miri test in crossbeam/crossbeam-deque:

test is_empty ... error: Undefined Behavior: type validation failed at .value[0]: encountered uninitialized bytes
  --> /home/ben/crossbeam/crossbeam-epoch/src/deferred.rs:49:27a
   |
49 |                     data: data.assume_init(),
   |                           ^^^^^^^^^^^^^^^^^^ type validation failed at .value[0]: encountered uninitialized bytes
   |

In the crossbeam-deque test suite, a Deferred was created from a FnOnce which is smaller than the Data. This makes the call to MaybeUninit::assume_init() immediate UB (the reference to it created upon call is probably UB too). The docs for MaybeUninit::assume_init() say this:

It is up to the caller to guarantee that the MaybeUninit really is in an initialized state. Calling this when the content is not yet fully initialized causes immediate undefined behavior. The type-level documentation contains more information about this initialization invariant.

Since Data doesn't have a Drop impl, we can just leave it in the MaybeUninit wrapper. This removes all issues about type validity.

In the crossbeam-deque test suite, a Deferred is created from a FnOnce
which is smaller than the Data. This makes the call to assume_init()
immediate UB (the reference to it created upon call is probably UB too).
Since Data doesn't have a Drop impl, we can just leave it in the MaybeUninit
wrapper.
Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 5, 2022

Build succeeded:

@bors bors bot merged commit be6ff29 into crossbeam-rs:master Feb 5, 2022
bors bot added a commit that referenced this pull request Feb 5, 2022
786: Prepare for the next release r=taiki-e a=taiki-e

- crossbeam-epoch 0.9.6 -> 0.9.7
  - Fix Miri error when `-Zmiri-check-number-validity` is enabled. (#779)
- crossbeam-queue 0.3.3 -> 0.3.4
  - Implement `IntoIterator` for `ArrayQueue` and `SegQueue`. (#772)
- crossbeam-utils 0.8.6 -> 0.8.7
  - Add `AtomicCell<i*,u*>::{fetch_max,fetch_min}`. (#785)
  - Add `AtomicCell<i*,u*,bool>::fetch_nand`. (#785)
  - Fix unsoundness of `AtomicCell<{i,u}64>` arithmetics on 32-bit targets that support `Atomic{I,U}64` (#781)


Co-authored-by: Taiki Endo <te316e89@gmail.com>
bors bot added a commit that referenced this pull request Feb 5, 2022
786: Prepare for the next release r=taiki-e a=taiki-e

- crossbeam-epoch 0.9.6 -> 0.9.7
  - Fix Miri error when `-Zmiri-check-number-validity` is enabled. (#779)
- crossbeam-queue 0.3.3 -> 0.3.4
  - Implement `IntoIterator` for `ArrayQueue` and `SegQueue`. (#772)
- crossbeam-utils 0.8.6 -> 0.8.7
  - Add `AtomicCell<{i*,u*}>::{fetch_max,fetch_min}`. (#785)
  - Add `AtomicCell<{i*,u*,bool}>::fetch_nand`. (#785)
  - Fix unsoundness of `AtomicCell<{i,u}64>` arithmetics on 32-bit targets that support `Atomic{I,U}64` (#781)


Co-authored-by: Taiki Endo <te316e89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants