UB in `ArcBorrow` implementation due to provenance
steffahn opened this issue · 1 comments
steffahn commented
ArcBorrow<'_, T>
contains an actual &T
reference. Such a reference cannot have provenance over the reference count though, which leads to miri detecting UB if you run cargo +nightly miri test
on #64.
git fetch origin bf35d79324665bd4886a4319216a0964a958dda5
git checkout bf35d79324665bd4886a4319216a0964a958dda5
cargo +miri test
running 33 tests
test arc::tests::arc_eq_and_cmp ... ok
test arc::tests::arc_eq_and_partial_cmp ... ok
test arc::tests::from_iterator_exact_size ... ok
test arc::tests::from_iterator_unknown_size ... ok
test arc::tests::maybeuninit ... ok
test arc::tests::maybeuninit_array ... ok
test arc::tests::maybeuninit_slice_ub_to_proceed - should panic ... ok
test arc::tests::maybeuninit_ub_to_proceed - should panic ... ok
test arc::tests::roundtrip ... ok
test arc::tests::roundtrip_slice ... ok
test arc::tests::try_unwrap ... ok
test arc_borrow::clone_arc_borrow ... error: Undefined Behavior: trying to retag from <346713> for SharedReadWrite permission at alloc90475[0x0], but that tag does not exist in the borrow stack for this location
--> src/arc.rs:230:18
|
230 | unsafe { &*self.ptr() }
| ^^^^^^^^^^^^
| |
| trying to retag from <346713> for SharedReadWrite permission at alloc90475[0x0], but that tag does not exist in the borrow stack for this location
| this error occurs as part of retag at alloc90475[0x0..0x10]
|
= 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: <346713> was created by a SharedReadOnly retag at offsets [0x8..0xc]
--> src/arc_borrow.rs:39:42
|
39 | let arc = unsafe { Arc::from_raw(self.0) };
| ^^^^^^
= note: BACKTRACE (of the first span):
= note: inside `arc::Arc::<i32>::inner` at src/arc.rs:230:18: 230:30
note: inside `<arc::Arc<i32> as std::clone::Clone>::clone`
--> src/arc.rs:423:24
|
423 | let old_size = self.inner().count.fetch_add(1, Relaxed);
| ^^^^^^^^^^^^
note: inside `arc_borrow::ArcBorrow::<'_, i32>::clone_arc`
--> src/arc_borrow.rs:41:21
|
41 | mem::forget(arc.clone());
| ^^^^^^^^^^^
note: inside `arc_borrow::clone_arc_borrow`
--> src/arc_borrow.rs:122:13
|
122 | let y = b.clone_arc();
| ^^^^^^^^^^^^^
note: inside closure
--> src/arc_borrow.rs:119:22
|
118 | #[test]
| ------- in this procedural macro expansion
119 | fn clone_arc_borrow() {
| ^
= note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
error: aborting due to previous error
error: test failed, to rerun pass `--lib`
Caused by:
process didn't exit successfully: `/home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner /home/frank/forks/just-a-clone/triomphe/target/miri/x86_64-unknown-linux-gnu/debug/deps/triomphe-70e77d022ca4bab7` (exit status: 1)
I should have time tomorrow to work on a fix, probably comparable to the implementation of rc_borrow::ArcBorrow
which uses a raw (NonNull
) pointer instead of a reference.
Manishearth commented
Ah, thanks for going through this. This crate was written ages ago and has some rough edges, very much appreciate your attempts to review and fix it!