`Arc::<MaybeUninit<T>>::write` is UB
WaffleLapkin opened this issue · 1 comments
WaffleLapkin commented
write
is defined like this:
impl<T> Arc<MaybeUninit<T>> {
pub fn write(&mut self, val: T) -> &mut T {
unsafe {
let ptr = (*self.ptr()).data.as_mut_ptr();
ptr.write(val);
&mut *ptr
}
}
}
It writes to the inner value of the Arc
even though unique access to Arc
does not guarantee unique access to the inner value (b/c of clones). Here is a simple test case that fails under miri:
let mut uninit = Arc::new_uninit();
let clone = uninit.clone();
let x: &MaybeUninit<String> = &*clone;
// This write invalidates `x` reference
uninit.write(String::from("nonononono"));
// Read invalidated reference to trigger UB
let _ = &*x;
error: Undefined Behavior: trying to reborrow <205779> for SharedReadOnly permission at alloc89625[0x8], but that tag does not exist in the borrow stack for this location
--> src/lib.rs:107:5
|
107 | &*_x;
| ^^^^
| |
| trying to reborrow <205779> for SharedReadOnly permission at alloc89625[0x8], but that tag does not exist in the borrow stack for this location
| this error occurs as part of a reborrow at alloc89625[0x8..0x20]
|
= 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: <205779> was created by a retag at offsets [0x8..0x20]
--> src/lib.rs:103:36
|
103 | let _x: &MaybeUninit<String> = &*clone;
| ^^^^^^^
help: <205779> was later invalidated at offsets [0x8..0x20]
--> src/arc.rs:247:23
|
247 | let ptr = (*self.ptr()).data.as_mut_ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: inside `ub` at src/lib.rs:107:5
note: inside closure at src/lib.rs:97:1
--> src/lib.rs:97:1
|
96 | #[test]
| ------- in this procedural macro expansion
97 | / fn ub() {
98 | | use core::mem::MaybeUninit;
99 | |
100 | | let mut uninit = Arc::new_uninit();
... |
107 | | &*_x;
108 | | }
| |_^
= note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)
Manishearth commented
Good catch, thanks! That's an older API that should have just been removed, yeah, deprecating it and making it panicky is fine for now.