Inline storage types need to use UnsafeCell or a get_mut resolver

[CAD97]

<SingleElement as SingleElementStorage>::get allows you to go from &SingleElement to &mut T, violating &'s mutability restrictions. The same goes for any other inline storage. There are two solutions to this:

• Use UnsafeCell in inline storages (pessimizes Box<T, InlineStorage>), or
• Add a get_mut(&mut self, handle: Handle<T>) -> NonNull<T> resolver, and require that mutable pointee access goes through get_mut unless a marker trait SharedMutabilityStorage is implemented.

(To allow e.g. split_at_mut style use cases, it could be get_mut(&mut self, [Handle<T>; N]) -> [NonNull<T>; N], get_mut(&mut self, Handle<T>) -> (&mut Self, NonNull<T>), or even just get(*mut self, Handle<T>) -> NonNull<T>.)

[matthieu-m]

Is that a problem?

I hesitated adding get_mut, as it essentially duplicates the API and implementation, and after experimentation it seemed unnecessary: cargo miri test did not report any issue, and therefore it appeared to me that the Stacked Borrow model was respected.

Did I miss something at the time? Did something change? Or do you believe this is a shortcoming of MIRI?

[CAD97]

Your test must've not actually used the place mutably, because even with default miri settings, this causes UB:

use storage_poc::{collections::RawBox, inline::SingleElement};

fn main() {
    let storage = SingleElement::<usize>::default();
    let mut boxed = RawBox::new(0usize, storage).unwrap();

    *boxed = 2;
}

D:\repos\cad97\playground> $env:MIRIFLAGS=$null
D:\repos\cad97\playground> cargo miri run   
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `D:\.rust\rustup\toolchains\nightly-x86_64-pc-windows-msvc\bin\cargo-miri.exe D:\.rust\target\miri\x86_64-pc-windows-msvc\debug\playground.exe`
error: Undefined Behavior: attempting a write access using <untagged> at alloc772[0x0], but that tag only grants SharedReadOnly permission for this location
    --> D:\.rust\rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ptr\mod.rs:1162:9
     |
1162 |         copy_nonoverlapping(&src as *const T, dst, 1);
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |         |
     |         attempting a write access using <untagged> at alloc772[0x0], but that tag only grants SharedReadOnly permission for this location
     |         this error occurs as part of an access at alloc772[0x0..0x8]
     |
     = help: this indicates a potential bug in the program: it performed an invalid operation, but the 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

     = note: inside `std::ptr::write::<usize>` at D:\.rust\rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ptr\mod.rs:1162:9
     = note: inside `<storage_poc::inline::SingleElement<usize> as storage_poc::traits::SingleElementStorage>::create::<usize>` at D:\.rust\cargo\git\checkouts\storage-poc-fbbb453f999fd909\018b31a\src\traits.rs:80:22
     = note: inside `storage_poc::collections::RawBox::<usize, storage_poc::inline::SingleElement<usize>>::new` at D:\.rust\cargo\git\checkouts\storage-poc-fbbb453f999fd909\018b31a\src\collections\raw_box.rs:23:15
note: inside `main` at src\main.rs:5:21
    --> src\main.rs:5:21
     |
5    |     let mut boxed = RawBox::new(0usize, storage).unwrap();
     |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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

error: aborting due to previous error

[CAD97]

I'm working on a revised proposal I hope to propose for MCP/in-tree testing (and maybe a foundation grant 👉👈) when I ran into this.

[matthieu-m]

I'm working on a revised proposal I hope to propose for MCP/in-tree testing (and maybe a foundation grant 👉👈) when I ran into this.

That's great to hear! I don't have much time, unfortunately, but I'd be happy to support you.

At the very least, feel free to reuse as much code/ideas from this repository as you wish.

---

Your test must've not actually used the place mutably

I thought they would, but may have missed it :(

In this case, I would guess the cleanest is to add a get_mut resolver. It's a bit of code duplication, but seems preferable to just papering over it with UnsafeCell.

[CAD97]

If you have time to glance over it, my current revision lives at https://github.com/CAD97/storages-api

[matthieu-m]

Should be solved by 08f60c1 .

I also went ahead and switched to resolve/resolve_mut (instead of get/get_mut) for aesthetics reasons; I much prefer the name you picked.

Unlike your API, I did not leave any provision for resolve_mut to invalidate existing resolved references; however I expect that in practice the Stacked Borrow model would disallow it regardless.