Arc::get_mut_unchecked does not mention restrictions on drop

[orlp]

Location

Arc::get_mut_unchecked

Summary

The safety comment on Arc::get_mut_unchecked states:

If any other Arc or Weak pointers to the same allocation exist, then they must not be dereferenced or have active borrows for the duration of the returned borrow, and their inner type must be exactly the same as the inner type of this Rc (including lifetimes).

But it mentions nothing about dropping Arcs, which triggers unsoundness in MIRI:

#![feature(get_mut_unchecked)]

use std::sync::Arc;

fn main() {
    let mut x = Arc::new(0);
    let y = x.clone();
    let x_ref = unsafe { Arc::get_mut_unchecked(&mut x) }; // Create unchecked reference.
    drop(y); // Drop a different Arc pointing to the same value while reference exists.
    *x_ref = 42; // Use unchecked reference.
}

error: Undefined Behavior: attempting a write access using <1715> at alloc838[0x10], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:10:5
   |
10 |     *x_ref = 42;
   |     ^^^^^^^^^^^
   |     |
   |     attempting a write access using <1715> at alloc838[0x10], but that tag does not exist in the borrow stack for this location
   |     this error occurs as part of an access at alloc838[0x10..0x14]
   |
   = 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: <1715> was created by a Unique retag at offsets [0x10..0x14]
  --> src/main.rs:8:26
   |
8  |     let x_ref = unsafe { Arc::get_mut_unchecked(&mut x) };
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <1715> was later invalidated at offsets [0x0..0x18] by a SharedReadOnly retag
  --> src/main.rs:9:5
   |
9  |     drop(y);
   |     ^^^^^^^

[zachs18]

Note that this passes under MIRIFLAGS=-Zmiri-tree-borrows

[zachs18]

I think this is because Arc::inner (which is used in Arc::drop and elsewhere), returns a &ArcInner<T>, which covers both the counts that Arc::drop needs access to (as SharedReadWrite), as well as the T (as SharedReadOnly, assuming T: Freeze), so under stacked borrows (and where T: Freeze), calling Arc::inner invalidates any previous Unique tag to the value because of the SharedReadOnly tag that is produced.

Miri details

MIRIFLAGS="-Zmiri-track-pointer-tag=1633,1658" cargo +nightly miri run -Zbuild-std

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
     Running `/home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner /home/zachary/opt_mount/zachary/cargo-target/miri/x86_64-unknown-linux-gnu/debug/miri-thing-2`
note: tracking was triggered
 --> src/main.rs:8:26
  |
8 |     let x_ref = unsafe { Arc::get_mut_unchecked(&mut x) }; // Create unchecked reference.
  |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ created tag 1633 with Unique permission (pointee type i32) at alloc778[0x10..0x14] derived from <1632>
  |
  = note: BACKTRACE:
  = note: inside `main` at src/main.rs:8:26: 8:56

note: tracking was triggered
   --> /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:429:18
    |
429 |         unsafe { &*self.as_ptr().cast_const() }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ created tag 1658 with SharedReadOnly/SharedReadWrite permission for frozen/non-frozen parts (pointee type alloc::sync::ArcInner<i32>) at alloc778[0x0..0x18] derived from <1563>
    |
    = note: BACKTRACE:
    = note: inside `std::ptr::NonNull::<alloc::sync::ArcInner<i32>>::as_ref::<'_>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:429:18: 429:46
    = note: inside `std::sync::Arc::<i32>::inner` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1894:18: 1894:35
    = note: inside `<std::sync::Arc<i32> as std::ops::Drop>::drop` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:2554:12: 2554:24
    = note: inside `std::ptr::drop_in_place::<std::sync::Arc<i32>> - shim(Some(std::sync::Arc<i32>))` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:523:1: 523:56
    = note: inside `std::mem::drop::<std::sync::Arc<i32>>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:940:24: 940:25
note: inside `main`
   --> src/main.rs:9:5
    |
9   |     drop(y); // Drop a different Arc pointing to the same value while reference exists.
    |     ^^^^^^^

note: tracking was triggered
   --> /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:429:18
    |
429 |         unsafe { &*self.as_ptr().cast_const() }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ popped tracked tag for item [Unique for <1633>] due to SharedReadOnly retag from <1563> (that retag created <1658>)
    |
    = note: BACKTRACE:
    = note: inside `std::ptr::NonNull::<alloc::sync::ArcInner<i32>>::as_ref::<'_>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:429:18: 429:46
    = note: inside `std::sync::Arc::<i32>::inner` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1894:18: 1894:35
    = note: inside `<std::sync::Arc<i32> as std::ops::Drop>::drop` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:2554:12: 2554:24
    = note: inside `std::ptr::drop_in_place::<std::sync::Arc<i32>> - shim(Some(std::sync::Arc<i32>))` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:523:1: 523:56
    = note: inside `std::mem::drop::<std::sync::Arc<i32>>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:940:24: 940:25
note: inside `main`
   --> src/main.rs:9:5
    |
9   |     drop(y); // Drop a different Arc pointing to the same value while reference exists.
    |     ^^^^^^^

error: Undefined Behavior: attempting a write access using <1633> at alloc778[0x10], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:10:5
   |
10 |     *x_ref = 42; // Use unchecked reference.
   |     ^^^^^^^^^^^
   |     |
   |     attempting a write access using <1633> at alloc778[0x10], but that tag does not exist in the borrow stack for this location
   |     this error occurs as part of an access at alloc778[0x10..0x14]
   |
   = 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: <1633> was created by a Unique retag at offsets [0x10..0x14]
  --> src/main.rs:8:26
   |
8  |     let x_ref = unsafe { Arc::get_mut_unchecked(&mut x) }; // Create unchecked reference.
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <1633> was later invalidated at offsets [0x0..0x18] by a SharedReadOnly retag
  --> src/main.rs:9:5
   |
9  |     drop(y); // Drop a different Arc pointing to the same value while reference exists.
   |     ^^^^^^^
   = note: BACKTRACE (of the first span):
   = note: inside `main` at src/main.rs:10:5: 10:16

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

One possible fix (other than restricting Arc::get_mut_unchecked further) would be to put the refcounts into a struct as a field of ArcInner and only take a reference to that field in places where the full &ArcInner<T> is not needed.