diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 3 | ||||
| -rw-r--r-- | clippy_lints/src/transmute.rs | 105 | ||||
| -rw-r--r-- | src/lintlist/mod.rs | 7 | ||||
| -rw-r--r-- | tests/ui/transmutes_expressible_as_ptr_casts.fixed | 90 | ||||
| -rw-r--r-- | tests/ui/transmutes_expressible_as_ptr_casts.rs | 90 | ||||
| -rw-r--r-- | tests/ui/transmutes_expressible_as_ptr_casts.stderr | 56 |
7 files changed, 350 insertions, 2 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 776b04295f9..a6e69491017 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1730,6 +1730,7 @@ Released 2018-09-13 [`transmute_int_to_float`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_float [`transmute_ptr_to_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr [`transmute_ptr_to_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref +[`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts [`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null [`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex [`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f371942dbee..828ee910596 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -788,6 +788,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &to_digit_is_some::TO_DIGIT_IS_SOME, &trait_bounds::TYPE_REPETITION_IN_BOUNDS, &transmute::CROSSPOINTER_TRANSMUTE, + &transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, &transmute::TRANSMUTE_BYTES_TO_STR, &transmute::TRANSMUTE_FLOAT_TO_INT, &transmute::TRANSMUTE_INT_TO_BOOL, @@ -1417,6 +1418,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT), LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME), LintId::of(&transmute::CROSSPOINTER_TRANSMUTE), + LintId::of(&transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS), LintId::of(&transmute::TRANSMUTE_BYTES_TO_STR), LintId::of(&transmute::TRANSMUTE_FLOAT_TO_INT), LintId::of(&transmute::TRANSMUTE_INT_TO_BOOL), @@ -1617,6 +1619,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&swap::MANUAL_SWAP), LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT), LintId::of(&transmute::CROSSPOINTER_TRANSMUTE), + LintId::of(&transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS), LintId::of(&transmute::TRANSMUTE_BYTES_TO_STR), LintId::of(&transmute::TRANSMUTE_FLOAT_TO_INT), LintId::of(&transmute::TRANSMUTE_INT_TO_BOOL), diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs index d55eb1a0c93..f077c146183 100644 --- a/clippy_lints/src/transmute.rs +++ b/clippy_lints/src/transmute.rs @@ -7,8 +7,10 @@ use rustc_ast::ast; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, GenericArg, Mutability, QPath, TyKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, cast::CastKind, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::DUMMY_SP; +use rustc_typeck::check::{cast::CastCheck, FnCtxt, Inherited}; use std::borrow::Cow; declare_clippy_lint! { @@ -48,6 +50,29 @@ declare_clippy_lint! { "transmutes that have the same to and from types or could be a cast/coercion" } +// FIXME: Merge this lint with USELESS_TRANSMUTE once that is out of the nursery. +declare_clippy_lint! { + /// **What it does:**Checks for transmutes that could be a pointer cast. + /// + /// **Why is this bad?** Readability. The code tricks people into thinking that + /// something complex is going on. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,ignore + /// core::intrinsics::transmute::<*const [i32], *const [u16]>(p) + /// ``` + /// Use instead: + /// ```rust + /// p as *const [u16] + /// ``` + pub TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, + complexity, + "transmutes that could be a pointer cast" +} + declare_clippy_lint! { /// **What it does:** Checks for transmutes between a type `T` and `*T`. /// @@ -269,6 +294,7 @@ declare_clippy_lint! { correctness, "transmute between collections of layout-incompatible types" } + declare_lint_pass!(Transmute => [ CROSSPOINTER_TRANSMUTE, TRANSMUTE_PTR_TO_REF, @@ -281,6 +307,7 @@ declare_lint_pass!(Transmute => [ TRANSMUTE_INT_TO_FLOAT, TRANSMUTE_FLOAT_TO_INT, UNSOUND_COLLECTION_TRANSMUTE, + TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, ]); // used to check for UNSOUND_COLLECTION_TRANSMUTE @@ -601,7 +628,25 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { ); } }, - _ => return, + (_, _) if can_be_expressed_as_pointer_cast(cx, e, from_ty, to_ty) => span_lint_and_then( + cx, + TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, + e.span, + &format!( + "transmute from `{}` to `{}` which could be expressed as a pointer cast instead", + from_ty, + to_ty + ), + |diag| { + if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + let sugg = arg.as_ty(&to_ty.to_string()).to_string(); + diag.span_suggestion(e.span, "try", sugg, Applicability::MachineApplicable); + } + } + ), + _ => { + return; + }, } } } @@ -648,3 +693,59 @@ fn is_layout_incompatible<'tcx>(cx: &LateContext<'tcx>, from: Ty<'tcx>, to: Ty<' false } } + +/// Check if the type conversion can be expressed as a pointer cast, instead of +/// a transmute. In certain cases, including some invalid casts from array +/// references to pointers, this may cause additional errors to be emitted and/or +/// ICE error messages. This function will panic if that occurs. +fn can_be_expressed_as_pointer_cast<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + from_ty: Ty<'tcx>, + to_ty: Ty<'tcx>, +) -> bool { + use CastKind::*; + matches!( + check_cast(cx, e, from_ty, to_ty), + Some(PtrPtrCast | PtrAddrCast | AddrPtrCast | ArrayPtrCast | FnPtrPtrCast | FnPtrAddrCast) + ) +} + +/// If a cast from from_ty to to_ty is valid, returns an Ok containing the kind of +/// the cast. In certain cases, including some invalid casts from array references +/// to pointers, this may cause additional errors to be emitted and/or ICE error +/// messages. This function will panic if that occurs. +fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>) -> Option<CastKind> { + let hir_id = e.hir_id; + let local_def_id = hir_id.owner; + + Inherited::build(cx.tcx, local_def_id).enter(|inherited| { + let fn_ctxt = FnCtxt::new(&inherited, cx.param_env, hir_id); + + // If we already have errors, we can't be sure we can pointer cast. + assert!( + !fn_ctxt.errors_reported_since_creation(), + "Newly created FnCtxt contained errors" + ); + + if let Ok(check) = CastCheck::new( + &fn_ctxt, e, from_ty, to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, DUMMY_SP, + ) { + let res = check.do_check(&fn_ctxt); + + // do_check's documentation says that it might return Ok and create + // errors in the fcx instead of returing Err in some cases. Those cases + // should be filtered out before getting here. + assert!( + !fn_ctxt.errors_reported_since_creation(), + "`fn_ctxt` contained errors after cast check!" + ); + + res.ok() + } else { + None + } + }) +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 1879aae77fb..1f3f70631fb 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2216,6 +2216,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![ module: "transmute", }, Lint { + name: "transmutes_expressible_as_ptr_casts", + group: "complexity", + desc: "transmutes that could be a pointer cast", + deprecation: None, + module: "transmute", + }, + Lint { name: "transmuting_null", group: "correctness", desc: "transmutes from a null pointer to a reference, which is undefined behavior", diff --git a/tests/ui/transmutes_expressible_as_ptr_casts.fixed b/tests/ui/transmutes_expressible_as_ptr_casts.fixed new file mode 100644 index 00000000000..98288dde6d8 --- /dev/null +++ b/tests/ui/transmutes_expressible_as_ptr_casts.fixed @@ -0,0 +1,90 @@ +// run-rustfix +#![warn(clippy::transmutes_expressible_as_ptr_casts)] +// These two warnings currrently cover the cases transmutes_expressible_as_ptr_casts +// would otherwise be responsible for +#![warn(clippy::useless_transmute)] +#![warn(clippy::transmute_ptr_to_ptr)] +#![allow(unused_unsafe)] +#![allow(dead_code)] + +use std::mem::{size_of, transmute}; + +// rustc_typeck::check::cast contains documentation about when a cast `e as U` is +// valid, which we quote from below. +fn main() { + // We should see an error message for each transmute, and no error messages for + // the casts, since the casts are the recommended fixes. + + // e is an integer and U is *U_0, while U_0: Sized; addr-ptr-cast + let _ptr_i32_transmute = unsafe { + usize::MAX as *const i32 + }; + let ptr_i32 = usize::MAX as *const i32; + + // e has type *T, U is *U_0, and either U_0: Sized ... + let _ptr_i8_transmute = unsafe { + ptr_i32 as *const i8 + }; + let _ptr_i8 = ptr_i32 as *const i8; + + let slice_ptr = &[0,1,2,3] as *const [i32]; + + // ... or pointer_kind(T) = pointer_kind(U_0); ptr-ptr-cast + let _ptr_to_unsized_transmute = unsafe { + slice_ptr as *const [u16] + }; + let _ptr_to_unsized = slice_ptr as *const [u16]; + // TODO: We could try testing vtable casts here too, but maybe + // we should wait until std::raw::TraitObject is stabilized? + + // e has type *T and U is a numeric type, while T: Sized; ptr-addr-cast + let _usize_from_int_ptr_transmute = unsafe { + ptr_i32 as usize + }; + let _usize_from_int_ptr = ptr_i32 as usize; + + let array_ref: &[i32; 4] = &[1,2,3,4]; + + // e has type &[T; n] and U is *const T; array-ptr-cast + let _array_ptr_transmute = unsafe { + array_ref as *const [i32; 4] + }; + let _array_ptr = array_ref as *const [i32; 4]; + + fn foo(_: usize) -> u8 { 42 } + + // e is a function pointer type and U has type *T, while T: Sized; fptr-ptr-cast + let _usize_ptr_transmute = unsafe { + foo as *const usize + }; + let _usize_ptr_transmute = foo as *const usize; + + // e is a function pointer type and U is an integer; fptr-addr-cast + let _usize_from_fn_ptr_transmute = unsafe { + foo as usize + }; + let _usize_from_fn_ptr = foo as *const usize; +} + +// If a ref-to-ptr cast of this form where the pointer type points to a type other +// than the referenced type, calling `CastCheck::do_check` has been observed to +// cause an ICE error message. `do_check` is currently called inside the +// `transmutes_expressible_as_ptr_casts` check, but other, more specific lints +// currently prevent it from being called in these cases. This test is meant to +// fail if the ordering of the checks ever changes enough to cause these cases to +// fall through into `do_check`. +fn trigger_do_check_to_emit_error(in_param: &[i32; 1]) -> *const u8 { + unsafe { in_param as *const [i32; 1] as *const u8 } +} + +#[repr(C)] +struct Single(u64); + +#[repr(C)] +struct Pair(u32, u32); + +fn cannot_be_expressed_as_pointer_cast(in_param: Single) -> Pair { + assert_eq!(size_of::<Single>(), size_of::<Pair>()); + + unsafe { transmute::<Single, Pair>(in_param) } +} diff --git a/tests/ui/transmutes_expressible_as_ptr_casts.rs b/tests/ui/transmutes_expressible_as_ptr_casts.rs new file mode 100644 index 00000000000..fd5055c08f6 --- /dev/null +++ b/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -0,0 +1,90 @@ +// run-rustfix +#![warn(clippy::transmutes_expressible_as_ptr_casts)] +// These two warnings currrently cover the cases transmutes_expressible_as_ptr_casts +// would otherwise be responsible for +#![warn(clippy::useless_transmute)] +#![warn(clippy::transmute_ptr_to_ptr)] +#![allow(unused_unsafe)] +#![allow(dead_code)] + +use std::mem::{size_of, transmute}; + +// rustc_typeck::check::cast contains documentation about when a cast `e as U` is +// valid, which we quote from below. +fn main() { + // We should see an error message for each transmute, and no error messages for + // the casts, since the casts are the recommended fixes. + + // e is an integer and U is *U_0, while U_0: Sized; addr-ptr-cast + let _ptr_i32_transmute = unsafe { + transmute::<usize, *const i32>(usize::MAX) + }; + let ptr_i32 = usize::MAX as *const i32; + + // e has type *T, U is *U_0, and either U_0: Sized ... + let _ptr_i8_transmute = unsafe { + transmute::<*const i32, *const i8>(ptr_i32) + }; + let _ptr_i8 = ptr_i32 as *const i8; + + let slice_ptr = &[0,1,2,3] as *const [i32]; + + // ... or pointer_kind(T) = pointer_kind(U_0); ptr-ptr-cast + let _ptr_to_unsized_transmute = unsafe { + transmute::<*const [i32], *const [u16]>(slice_ptr) + }; + let _ptr_to_unsized = slice_ptr as *const [u16]; + // TODO: We could try testing vtable casts here too, but maybe + // we should wait until std::raw::TraitObject is stabilized? + + // e has type *T and U is a numeric type, while T: Sized; ptr-addr-cast + let _usize_from_int_ptr_transmute = unsafe { + transmute::<*const i32, usize>(ptr_i32) + }; + let _usize_from_int_ptr = ptr_i32 as usize; + + let array_ref: &[i32; 4] = &[1,2,3,4]; + + // e has type &[T; n] and U is *const T; array-ptr-cast + let _array_ptr_transmute = unsafe { + transmute::<&[i32; 4], *const [i32; 4]>(array_ref) + }; + let _array_ptr = array_ref as *const [i32; 4]; + + fn foo(_: usize) -> u8 { 42 } + + // e is a function pointer type and U has type *T, while T: Sized; fptr-ptr-cast + let _usize_ptr_transmute = unsafe { + transmute::<fn(usize) -> u8, *const usize>(foo) + }; + let _usize_ptr_transmute = foo as *const usize; + + // e is a function pointer type and U is an integer; fptr-addr-cast + let _usize_from_fn_ptr_transmute = unsafe { + transmute::<fn(usize) -> u8, usize>(foo) + }; + let _usize_from_fn_ptr = foo as *const usize; +} + +// If a ref-to-ptr cast of this form where the pointer type points to a type other +// than the referenced type, calling `CastCheck::do_check` has been observed to +// cause an ICE error message. `do_check` is currently called inside the +// `transmutes_expressible_as_ptr_casts` check, but other, more specific lints +// currently prevent it from being called in these cases. This test is meant to +// fail if the ordering of the checks ever changes enough to cause these cases to +// fall through into `do_check`. +fn trigger_do_check_to_emit_error(in_param: &[i32; 1]) -> *const u8 { + unsafe { transmute::<&[i32; 1], *const u8>(in_param) } +} + +#[repr(C)] +struct Single(u64); + +#[repr(C)] +struct Pair(u32, u32); + +fn cannot_be_expressed_as_pointer_cast(in_param: Single) -> Pair { + assert_eq!(size_of::<Single>(), size_of::<Pair>()); + + unsafe { transmute::<Single, Pair>(in_param) } +} diff --git a/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/tests/ui/transmutes_expressible_as_ptr_casts.stderr new file mode 100644 index 00000000000..46597acc6c0 --- /dev/null +++ b/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -0,0 +1,56 @@ +error: transmute from an integer to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:20:9 + | +LL | transmute::<usize, *const i32>(usize::MAX) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `usize::MAX as *const i32` + | + = note: `-D clippy::useless-transmute` implied by `-D warnings` + +error: transmute from a pointer to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:26:9 + | +LL | transmute::<*const i32, *const i8>(ptr_i32) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr_i32 as *const i8` + | + = note: `-D clippy::transmute-ptr-to-ptr` implied by `-D warnings` + +error: transmute from a pointer to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:34:9 + | +LL | transmute::<*const [i32], *const [u16]>(slice_ptr) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice_ptr as *const [u16]` + +error: transmute from `*const i32` to `usize` which could be expressed as a pointer cast instead + --> $DIR/transmutes_expressible_as_ptr_casts.rs:42:9 + | +LL | transmute::<*const i32, usize>(ptr_i32) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr_i32 as usize` + | + = note: `-D clippy::transmutes-expressible-as-ptr-casts` implied by `-D warnings` + +error: transmute from a reference to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:50:9 + | +LL | transmute::<&[i32; 4], *const [i32; 4]>(array_ref) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `array_ref as *const [i32; 4]` + +error: transmute from `fn(usize) -> u8 {main::foo}` to `*const usize` which could be expressed as a pointer cast instead + --> $DIR/transmutes_expressible_as_ptr_casts.rs:58:9 + | +LL | transmute::<fn(usize) -> u8, *const usize>(foo) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as *const usize` + +error: transmute from `fn(usize) -> u8 {main::foo}` to `usize` which could be expressed as a pointer cast instead + --> $DIR/transmutes_expressible_as_ptr_casts.rs:64:9 + | +LL | transmute::<fn(usize) -> u8, usize>(foo) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as usize` + +error: transmute from a reference to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:77:14 + | +LL | unsafe { transmute::<&[i32; 1], *const u8>(in_param) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `in_param as *const [i32; 1] as *const u8` + +error: aborting due to 8 previous errors + |