diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 187b1fcee..a9ebd4dfd 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -641,12 +641,14 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { transmute::TRANSMUTE_INT_TO_CHAR, transmute::TRANSMUTE_INT_TO_FLOAT, transmute::TRANSMUTE_PTR_TO_REF, + transmute::TRANSMUTE_PTR_TO_PTR, transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, types::BORROWED_BOX, types::BOX_VEC, types::CAST_LOSSLESS, + types::CAST_PTR_ALIGNMENT, types::CHAR_LIT_AS_U8, types::IMPLICIT_HASHER, types::LET_UNIT_VALUE, @@ -791,6 +793,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { transmute::TRANSMUTE_INT_TO_CHAR, transmute::TRANSMUTE_INT_TO_FLOAT, transmute::TRANSMUTE_PTR_TO_REF, + transmute::TRANSMUTE_PTR_TO_PTR, transmute::USELESS_TRANSMUTE, types::BORROWED_BOX, types::CAST_LOSSLESS, @@ -848,6 +851,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { swap::ALMOST_SWAPPED, transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, + types::CAST_PTR_ALIGNMENT, types::UNIT_CMP, unicode::ZERO_WIDTH_SPACE, unused_io_amount::UNUSED_IO_AMOUNT, diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs index 00d6c310c..7b591a583 100644 --- a/clippy_lints/src/transmute.rs +++ b/clippy_lints/src/transmute.rs @@ -186,6 +186,33 @@ declare_clippy_lint! { "transmutes to a potentially less-aligned type" } +/// **What it does:** Checks for transmutes from a pointer to a pointer, or +/// from a reference to a reference. +/// +/// **Why is this bad?** Transmutes are dangerous, and these can instead be +/// written as casts. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// let ptr = &1u32 as *const u32; +/// unsafe { +/// // pointer-to-pointer transmute +/// let _: *const f32 = std::mem::transmute(ptr); +/// // ref-ref transmute +/// let _: &f32 = std::mem::transmute(&1u32); +/// } +/// // These can be respectively written: +/// let _ = ptr as *const f32 +/// let _ = unsafe{ &*(&1u32 as *const u32 as *const f32) }; +/// ``` +declare_clippy_lint! { + pub TRANSMUTE_PTR_TO_PTR, + complexity, + "transmutes from a pointer to a reference type" +} + pub struct Transmute; impl LintPass for Transmute { @@ -193,6 +220,7 @@ impl LintPass for Transmute { lint_array!( CROSSPOINTER_TRANSMUTE, TRANSMUTE_PTR_TO_REF, + TRANSMUTE_PTR_TO_PTR, USELESS_TRANSMUTE, WRONG_TRANSMUTE, TRANSMUTE_INT_TO_CHAR, @@ -363,9 +391,35 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute { ); } ) + } else { + span_lint_and_then( + cx, + TRANSMUTE_PTR_TO_PTR, + e.span, + "transmute from a reference to a reference", + |db| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + let sugg_paren = arg.as_ty(cx.tcx.mk_ptr(*ref_from)).as_ty(cx.tcx.mk_ptr(*ref_to)); + let sugg = if ref_to.mutbl == Mutability::MutMutable { + sugg_paren.mut_addr_deref() + } else { + sugg_paren.addr_deref() + }; + db.span_suggestion(e.span, "try", sugg.to_string()); + }, + ) } } }, + (&ty::TyRawPtr(_), &ty::TyRawPtr(to_ty)) => span_lint_and_then( + cx, + TRANSMUTE_PTR_TO_PTR, + e.span, + "transmute from a pointer to a pointer", + |db| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + let sugg = arg.as_ty(cx.tcx.mk_ptr(to_ty)); + db.span_suggestion(e.span, "try", sugg.to_string()); + }, + ), (&ty::TyInt(ast::IntTy::I8), &ty::TyBool) | (&ty::TyUint(ast::UintTy::U8), &ty::TyBool) => { span_lint_and_then( cx, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 16a1142ef..9034badd5 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -679,6 +679,25 @@ declare_clippy_lint! { "cast to the same type, e.g. `x as i32` where `x: i32`" } +/// **What it does:** Checks for casts from a more-strictly-aligned pointer to a +/// less-strictly-aligned pointer +/// +/// **Why is this bad?** Dereferencing the resulting pointer is undefined +/// behavior. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// let _ = (&1u8 as *const u8) as *const u16; +/// let _ = (&mut 1u8 as *mut u8) as *mut u16; +/// ``` +declare_clippy_lint! { + pub CAST_PTR_ALIGNMENT, + correctness, + "cast from a pointer to a less-strictly-aligned pointer" +} + /// Returns the size in bits of an integral type. /// Will return 0 if the type is not an int or uint variant fn int_ty_to_nbits(typ: Ty, tcx: TyCtxt) -> u64 { @@ -871,7 +890,8 @@ impl LintPass for CastPass { CAST_POSSIBLE_TRUNCATION, CAST_POSSIBLE_WRAP, CAST_LOSSLESS, - UNNECESSARY_CAST + UNNECESSARY_CAST, + CAST_PTR_ALIGNMENT ) } } @@ -955,6 +975,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { }, } } + if_chain!{ + if let ty::TyRawPtr(from_ptr_ty) = &cast_from.sty; + if let ty::TyRawPtr(to_ptr_ty) = &cast_to.sty; + if let Some(from_align) = cx.layout_of(from_ptr_ty.ty).ok().map(|a| a.align.abi()); + if let Some(to_align) = cx.layout_of(to_ptr_ty.ty).ok().map(|a| a.align.abi()); + if from_align < to_align; + then { + span_lint( + cx, + CAST_PTR_ALIGNMENT, + expr.span, + &format!("casting from `{}` to a less-strictly-aligned pointer (`{}`)", cast_from, cast_to) + ); + } + } } } } diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 4ca6e5cbf..bd48774a5 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -161,6 +161,20 @@ impl<'a> Sugg<'a> { make_unop("*", self) } + /// Convenience method to create the `&*` suggestion. Currently this + /// is needed because `sugg.deref().addr()` produces an unnecessary set of + /// parentheses around the deref. + pub fn addr_deref(self) -> Sugg<'static> { + make_unop("&*", self) + } + + /// Convenience method to create the `&mut *` suggestion. Currently + /// this is needed because `sugg.deref().mut_addr()` produces an unnecessary + /// set of parentheses around the deref. + pub fn mut_addr_deref(self) -> Sugg<'static> { + make_unop("&mut *", self) + } + /// Convenience method to create the `..` or `...` /// suggestion. pub fn range(self, end: &Self, limit: ast::RangeLimits) -> Sugg<'static> { diff --git a/tests/ui/cast_alignment.rs b/tests/ui/cast_alignment.rs new file mode 100644 index 000000000..4985a90bd --- /dev/null +++ b/tests/ui/cast_alignment.rs @@ -0,0 +1,19 @@ +//! Test casts for alignment issues + +#[warn(cast_ptr_alignment)] +#[allow(no_effect, unnecessary_operation, cast_lossless)] +fn main() { + /* These should be warned against */ + + // cast to more-strictly-aligned type + (&1u8 as *const u8) as *const u16; + (&mut 1u8 as *mut u8) as *mut u16; + + /* These should be okay */ + + // not a pointer type + 1u8 as u16; + // cast to less-strictly-aligned type + (&1u16 as *const u16) as *const u8; + (&mut 1u16 as *mut u16) as *mut u8; +} diff --git a/tests/ui/cast_alignment.stderr b/tests/ui/cast_alignment.stderr new file mode 100644 index 000000000..d4fdb5bec --- /dev/null +++ b/tests/ui/cast_alignment.stderr @@ -0,0 +1,16 @@ +error: casting from `*const u8` to a less-strictly-aligned pointer (`*const u16`) + --> $DIR/cast_alignment.rs:9:5 + | +9 | (&1u8 as *const u8) as *const u16; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D cast-ptr-alignment` implied by `-D warnings` + +error: casting from `*mut u8` to a less-strictly-aligned pointer (`*mut u16`) + --> $DIR/cast_alignment.rs:10:5 + | +10 | (&mut 1u8 as *mut u8) as *mut u16; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/transmute.rs b/tests/ui/transmute.rs index 48b89c33a..3ff444b38 100644 --- a/tests/ui/transmute.rs +++ b/tests/ui/transmute.rs @@ -16,7 +16,7 @@ fn my_vec() -> MyVec { vec![] } -#[allow(needless_lifetimes)] +#[allow(needless_lifetimes, transmute_ptr_to_ptr)] #[warn(useless_transmute)] unsafe fn _generic<'a, T, U: 'a>(t: &'a T) { let _: &'a T = core::intrinsics::transmute(t); @@ -147,4 +147,23 @@ fn misaligned_transmute() { let _: [u8; 4] = unsafe { std::mem::transmute(0u32) }; // ok (alignment-wise) } +#[warn(transmute_ptr_to_ptr)] +fn transmute_ptr_to_ptr() { + let ptr = &1u32 as *const u32; + let mut_ptr = &mut 1u32 as *mut u32; + unsafe { + // pointer-to-pointer transmutes; bad + let _: *const f32 = std::mem::transmute(ptr); + let _: *mut f32 = std::mem::transmute(mut_ptr); + // ref-ref transmutes; bad + let _: &f32 = std::mem::transmute(&1u32); + let _: &mut f32 = std::mem::transmute(&mut 1u32); + } + // These should be fine + let _ = ptr as *const f32; + let _ = mut_ptr as *mut f32; + let _ = unsafe { &*(&1u32 as *const u32 as *const f32) }; + let _ = unsafe { &mut *(&mut 1u32 as *mut u32 as *mut f32) }; +} + fn main() { } diff --git a/tests/ui/transmute.stderr b/tests/ui/transmute.stderr index 74bbf95d5..f7090f0dc 100644 --- a/tests/ui/transmute.stderr +++ b/tests/ui/transmute.stderr @@ -212,5 +212,31 @@ error: transmute from `[u8; 4]` to a less-aligned type (`u32`) | = note: `-D misaligned-transmute` implied by `-D warnings` -error: aborting due to 33 previous errors +error: transmute from a pointer to a pointer + --> $DIR/transmute.rs:156:29 + | +156 | let _: *const f32 = std::mem::transmute(ptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr as *const f32` + | + = note: `-D transmute-ptr-to-ptr` implied by `-D warnings` + +error: transmute from a pointer to a pointer + --> $DIR/transmute.rs:157:27 + | +157 | let _: *mut f32 = std::mem::transmute(mut_ptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `mut_ptr as *mut f32` + +error: transmute from a reference to a reference + --> $DIR/transmute.rs:159:23 + | +159 | let _: &f32 = std::mem::transmute(&1u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(&1u32 as *const u32 as *const f32)` + +error: transmute from a reference to a reference + --> $DIR/transmute.rs:160:27 + | +160 | let _: &mut f32 = std::mem::transmute(&mut 1u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut *(&mut 1u32 as *mut u32 as *mut f32)` + +error: aborting due to 37 previous errors