search

rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.65k stars 348 forks source link

Detecting if a MutexGuard is sent to another thread? #3988

Closed gendx closed 1 month ago

gendx commented 1 month ago

The std::sync::MutexGuard<T: Sync> type is famous for being Sync but !Send [1, 2]. Therefore, an idea would be to use it as a canary under Miri tests to confirm that one got the Send and Sync bounds right in complicated unsafe code (that would of course not be a proof, but Miri tests are still very valuable). Similarly to how std::cell::Cell<T: Send> can be used as an example of Send but !Sync type.

However, Miri currently doesn't report any error when one sends (via unsafe code) a MutexGuard and drops it on another thread: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c274542f30abf037266e7e3cae8c8c45

Would it be possible for Miri to detect such illegal sends? As far as I understand it, whether sending a MutexGuard is problematic or not depends on the underlying platform and implementation, but shouldn't Miri model the general API contract and confirm that it hasn't been violated?

[1] https://users.rust-lang.org/t/why-mutexguard-impls-sync-but-not-send/99801 [2] https://users.rust-lang.org/t/example-of-a-type-that-is-not-send/59835

RalfJung commented 1 month ago

Thanks for the report!

shouldn't Miri model the general API contract

No, that's not the intention.

Miri generally aims to detect language UB, not library UB. We emulate the behavior of the underlying platform primitives (like the pthread or futex APIs); std is just normal code to Miri. The underlying implementation of Mutex, at least on most platforms, is actually fine with an unlock on a different thread, so it is expected behavior for Miri not to detect this.

Detecting library UB would require the library to somehow explain to Miri what is and is not UB -- I don't think we want to special case std::sync::Mutex in Miri. But that's way off currently and a much broader problem than specifically MutexGuard, so I am going to close this issue as currently being out of scope for what Miri intends to do.

RalfJung commented 1 month ago

For MutexGuard specifically, one could imagine patching std to keep track of the owner, and report an error if the unlock happens on the wrong thread. That doesn't even need any special Miri support. It could be done under #[cfg(any(miri, debug_assertions))] if we want it to be always-on in Miri. But this would be entirely on the libs side, so it'd be up to t-libs whether they want to do that or not.