-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
Implement AnyBitPattern
for MaybeUninit<T>
#152
Conversation
Note about
This is true of Edit: Thinking about this more, I'm less sure, but still pretty confident this PR is sound. I don't think interior mutable types that are |
So it turns out that TODO: Maybe |
This might still be sound if // Assume we are in a hypothetical world where `Cell<T>: Copy where T: Copy`
// https://github.com/zachs18/rust/tree/cell_copy
use std::cell::Cell;
fn main() {
let a: u32 = 0;
let b: &MaybeUninit<Cell<u32>> = bytemuck::cast_ref(&a);
let c: &Cell<u32> = unsafe { b.assume_init_ref() }; // the only unsafe code is here
c.set(4); // UB: writing to a readonly location
dbg!(a);
} Actually, under current miri Stacked Borrows (with library modifications to make Miri output on above codeerror: Undefined Behavior: trying to retag from <14149> for SharedReadWrite permission at alloc5677[0x0], but that tag only grants SharedReadOnly permission for this location
--> /home/zachary/.cargo/git/checkouts/bytemuck-b67854ce243a90d2/9cdc314/src/internal.rs:291:17
|
291 | Ok(unsafe { &*(a as *const A as *const B) })
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| trying to retag from <14149> for SharedReadWrite permission at alloc5677[0x0], but that tag only grants SharedReadOnly permission for this location
| this error occurs as part of retag at alloc5677[0x0..0x4]
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <14149> was created by a SharedReadOnly retag at offsets [0x0..0x4]
--> src/main.rs:43:38
|
43 | let b: &MaybeUninit<Cell<u32>> = bytemuck::cast_ref(&a);
| ^^^^^^^^^^^^^^^^^^^^^^
= note: BACKTRACE:
= note: inside `bytemuck::internal::try_cast_ref::<u32, std::mem::MaybeUninit<std::cell::Cell<u32>>>` at /home/zachary/.cargo/git/checkouts/bytemuck-b67854ce243a90d2/9cdc314/src/internal.rs:291:17
= note: inside `bytemuck::internal::cast_ref::<u32, std::mem::MaybeUninit<std::cell::Cell<u32>>>` at /home/zachary/.cargo/git/checkouts/bytemuck-b67854ce243a90d2/9cdc314/src/internal.rs:215:11
= note: inside `bytemuck::cast_ref::<u32, std::mem::MaybeUninit<std::cell::Cell<u32>>>` at /home/zachary/.cargo/git/checkouts/bytemuck-b67854ce243a90d2/9cdc314/src/lib.rs:262:12
note: inside `main` at src/main.rs:43:38
--> src/main.rs:43:38
|
43 | let b: &MaybeUninit<Cell<u32>> = bytemuck::cast_ref(&a);
| ^^^^^^^^^^^^^^^^^^^^^^
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
error: aborting due to previous error |
Pod itself also assumes Freeze without saying so. Really, Freeze needs to be a trait that code can talk about directly |
...
where MaybeUninit<T>: Copy + 'static
.Resolves #108 .
A feature flag is required because
MaybeUninit
was unstable in bytemuck's MSRV. I just used the#[cfg(feature = "zeroable_maybe_uninit")]
flag since that's required anyway, and it didn't seem reasonable to add another feature flag for this.The
where MaybeUninit<T>: Copy + 'static
bound is required because ofAnyBitPattern
's supertrait boundsCopy + 'static
. A simpler bound would bewhere T: Copy + 'static
(i.e. have the bound onT
instead ofMaybeUninit<T>
). These are currently (and probably always will be) equivalent, but ifstd
ever relaxes theCopy
impl ofMaybeUninit
, this bound will reflect that, whereaswhere T: Copy + 'static
would not.