Detailed analysis of crater results for rust-lang/rust#128351
Summary: all crates that the lint triggered on contained actual UB (both in Stacked Borrows and in Tree Borrows). Not all instances are UB (some are only reads), though, but this clearly show the pattern is dangerous. However, there are some popular crates that do that.
Most are conversions from &NonAtomic to &Atomic.
Lint fired: mutable_transmutes
Usage status: no dependents, archived.
Status: does & to &mut transmutes, which are definitely UB (but are hidden behind Pin, and so weren't caught before).
It also violates the uniqueness of &mut, since those &mut are aliased with other & references created from the original &.
The & originated from &mut, and also the second &mut is &mut UnsafeCell. One of them might be why the author thought
this is OK, but we know &mut UnsafeCell is not special in any way.
A more interesting question would be whether transmuting to &UnsafeCell instead (which is denied by the other lint)
will be fine. The answer is that it can work, but it isn't needed: the crate never actually mutates values which weren't
originally behind UnsafeCell, so just transmuting to & will work (but will require changes to public API).
Lint fired: unsafe_cell_reference_casting
Usage status: not published on crates.io, demo code.
This crates is created to demonstrate UB in Rust (and in fact the test that fails to compile fails Miri under Stacked Borrows,
like all &->&UnsafeCell casts), so the fact the lints fire here means we are catching more UB at compile time, as intended.
No real use-case here.
Lint fired: unsafe_cell_reference_casting
Usage status: as far as I can tell not published on crates.io. Last commit 4 years ago.
Status: the crate has macro that can transmute between arbitrary references, and it's exported, which seems like a major soundness hole.
However, the macro is named as_atomic and it's usage is constrained to transmuting to atomics, so maybe the export was a mistake.
The lint is fired on almost every invocation of the macro, except one that is cfg'd out.
What matters for us, however, is how it's used. Out of the seven times this macro is used, it is followed 4 times by a load().
But one time it is followed by a store, and another two times by compare_exchange_weak(). This is clearly UB.
The store and the loads are possible to fix since they are working on a &&mut [usize], so it is possible to read the &mut [usize]
without going through the shared reference. But the compare_exchange_weak() is not fixable without changing the type. Furthermore, it comes
from a &u8 function argument (where we pass LLVM noalias), so it may be possible to trigger an actual miscompilation here.
Lint fired: unsafe_cell_reference_casting
Usage status: 57,474 downloads, two dependents, last commit 3 years ago.
Running the crate's tests under Miri quickly revealed another violation of Stacked Borrows. Running it with Tree Borrows
ignores other violations and focuses on this one: the UnsafeCell gotten from the shared reference is indeed being written
into. We have UB here. The lint has saved us once again.
This crate also had much more UB; I fixed a bunch of those a sent a PR (jonhoo/arccstr#7) - now merged and released.
https://github.com/playXE/cgc (https://crates.io/crates/cgc-single-threaded, https://crates.io/crates/cgc)
Lint fired: unsafe_cell_reference_casting
Usage status: Last commit 4 years ago. No dependents.
Status: lint fired once in a method (but for some reason this was reported twice?). A conversion from &*mut T to &AtomicPtr<T>. In the callers,
the AtomicPtr is being loaded, stored, and compare-and-swapped. So, clearly UB.
It looks possible to make this AtomicPtr to begin with - an easy fix.
Lint fired: unsafe_cell_reference_casting
Usage status: this is a popular project. 2,939,140 downloads, 131 dependents.
Crater reports on an old version of it, but the problems seem to stay in the new version. I don't know why crater isn't reporting it.
The lint was firing in two places of conversions to atomics. In both it is correct. In one modification immediately follows, and the other is a public API returning a modifiable atomic (wrapper).
I have not tried to fiddle with that, but as it seems, in the case of the public function it is easily possible to make it sound by
using addr_of!() instead of references as the object is FFI. I believe this is also possible in the second case, but I'm not sure.
https://github.com/zzau13/v_escape (https://crates.io/crates/v_htmlescape, https://crates.io/crates/v_jsonescape, https://crates.io/crates/v_latexescape)
Lint fired: unsafe_cell_reference_casting
Usage status: v_htmlescape is a popular crate - 2,425,606 downloads, 32 dependents. The rest are less, but their situation in the same.
Status: the lint was correct. This was UB (a store to a transmuted atomic). But this was already fixed on master (probably due to another lint,
creating a shared reference to mutable static is discouraged), to use addr_of!(), which is not UB and does not lint. The fix is not published
yet.
Lint fired: unsafe_cell_reference_casting
Usage status: 106,549 downloads, 10 dependents.
Status: the same as v_escape (same author). Was UB, but replaced with addr_of!(). The replacement is not yet released.
Lint fired: unsafe_cell_transmutes
Usage status: no dependents.
Status: definitely UB. One case exposes the transmuted atomic to the public. The other two also exposes it, but also store to it in the project's internal code.
Two cases can just be made into atomics in the first place, it isn't clear to me why they weren't. The other seems to be non-salvageable (it is a general API
for the conversion &u64->&AtomicU64Wrapper).
Lint fired: unsafe_cell_transmutes
Usage status: one dependent from the same project with no dependents.
Usage: UB. Stores into the transmuted atomic.
This code also relies on the internal layout of Arc, but nevermind.
This seems possible to save (but keep the dependency on the layout of Arc) by storing AtomicPtr instead of Arc, and converting it to Arc when needed
instead of the other way around (or alternatively, store UnsafeCell<Arc>).
https://github.com/05storm26/wayland-raw-protocol-bindings (https://crates.io/crates/wayland-raw-protocol-bindings)
Usage status: no dependents. Last commit 3 years ago.
Status: could not compile locally even with nightly. Crater does not show the error. Skipped.
Usage status: not published to crates.io as far as I can tell.
Status: a &u64 transmuted into reference to #[repr(C)] struct CompCount { val: i32, comp_count: Cell<u32> }. The Cell is then mutated. So we have UB here.
But it seems possible to just use a &CompCount in the first place (another piece of code in the same file does that).