Open Manishearth opened 2 years ago
Ran this lint the top 1000 (more or less) crates from crates.io. Here are the warnings I found.
I'm unable to say which are false positives.
Hope this helps.
Crate | Number |
---|---|
cgmath-0.18.0 | 32 |
criterion-0.3.5 | 1 |
iovec-0.1.4 | 3 |
nalgebra-0.30.1 | 2 |
parity-scale-codec-3.0.0 | 20 |
prettytable-rs-0.8.0 | 1 |
state-0.5.2 | 1 |
wasm-bindgen-0.2.79 | 40 |
The criterion error seems to indicate that the lint fails to apply type adjustments – in this case from &Vec<T>
to &[T]
as indicated by the first generic argument of the transmute
call. If I replace v
(which is of type &Vec<_>
by &v[..]
, the lint no longer fires.
sounds like we should at the very least be using the inferred or provided generic arguments of transmute
instead of reading the expression types
I opened #8501 to track this.
Just went through all them.
cgmath
is relying on the order of tuple fields.criterion
is a bug. Probably used expr_ty
instead of expr_ty_adjusted
.iovec
is relying on the order of slice pointers.nalgebra
is fine, it's checking the TypeId
of the generic types first.parity-scale-codec
is the same as nalgebra
.prettytable
is between two non-repr(C)
types.state
is fine. It doesn't read either of the fields, but the names for the fields are useless.wasm-bindgen
looks to be the same as state
, but using tuples instead of a struct.For both wasm-bindgen
and state
would not have triggered if they used [usize; 2]
. (usize, usize)
could be allowed here as well. state
uses a struct so there's nothing to really do there.
parity-scale-codec
really should be using a trait instead of transmuting. (this would need specialization to work, so not practical)nalgebra
could probably do that, but would be more complicated.
With the recent changes both nalgebra
and state
are still false positives.
nalgebra
still needs to be fixed not to lint.state
I don't think should be fixed given that the fields are named. Were the struct to be declared as #[repr(C)] struct AnyObject([usize;2])
then it would not lint.Results on running on more crates https://gist.github.com/Jarcho/6890efe9c9f051aef943f1dedbbbb02d
vsdb-0.45.2: Transmute between different Vec
instances
sxd-document-0.3.2: Relies on the order of &str
utf8-cstr-0.1.6: Relies on the order of &str
hyper-old-types-0.11.0: Relies on the order of &dyn Trait
serde_v8-0.56.0: Relies on the order of non-repr(C)
structs
query_interface-0.3.5: Relies on the order of &dyn Trait
solana_rbpf-0.2.31: Relies on the order of &dyn Trait
and the vtable layout
fontdue-0.7.2: Relies on the layout of tuples (f32, f32, f32, f32)
str-buf-3.0.1: Relies on the &[u8]
, but it at least has a test for it. Not guarenteed on the target platform, but better than nothing
rusty_v8-0.32.1: Relies on the layout of &dyn Trait
. Has a runtime assert checking the layout.
iovec-0.1.4: Relies on the layout of &[u8]
libpulse-binding-2.26.0: Relies on the layout of a non-repr(C)
struct.
adapton-0.3.31: Relies on the layout of &dyn Trait
. It also manages to leak memory by transmuting Box<&dyn Trait>
into &Box<U>
, with no way to deallocate the original box.
v8-0.48.0: same as rusty_v8
parity-scale-codec-3.1.5: Type parameter -> concrete type
lodepng-3.7.0: Horrible &mut *mut u8
-> &mut &mut Vec
which is un-erasing the type. (Won't lint with #9287)
skia-safe: Transmutes between &dyn Trait
and a struct, but doesn't read the fields.
state-0.5.3: Same as skia-safe
. Uses a custom struct but doesn't read the fields.
*fluent-bundle-0.15.2: Cast NonNull<u8>
from alloc
to `NonNull
Most of those are related to fat pointer layouts, which as far as I know are still formally undefined. The Unsafe Code Guidelines Working Group has them defined well enough for most of these transmutes, but should that be considered enough to ignore these cases?
See discussion in https://github.com/rust-lang/rust-clippy/pull/8432
The lint is currently in the nursery and has had multiple problems in the past. We should do a deeper investigation: probably running it on a lot of crates and looking for false positives, as well as trying to structure it to be more conservative by default.
Subissues: