diff --git a/CHANGELOG.md b/CHANGELOG.md index efa637b185c..7acf9bc1eaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -634,6 +634,7 @@ All notable changes to this project will be documented in this file. [`cast_possible_wrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_wrap [`cast_precision_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_precision_loss [`cast_ptr_alignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment +[`cast_ref_to_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ref_to_mut [`cast_sign_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_sign_loss [`char_lit_as_u8`]: https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8 [`chars_last_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_last_cmp diff --git a/README.md b/README.md index be54424dc38..8ca10da416d 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2e515cc8aea..35c00fb6328 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -486,6 +486,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box ptr_offset_with_cast::Pass); reg.register_late_lint_pass(box redundant_clone::RedundantClone); reg.register_late_lint_pass(box slow_vector_initialization::Pass); + reg.register_late_lint_pass(box types::RefToMut); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -758,6 +759,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { types::BOX_VEC, types::CAST_LOSSLESS, types::CAST_PTR_ALIGNMENT, + types::CAST_REF_TO_MUT, types::CHAR_LIT_AS_U8, types::FN_TO_NUMERIC_CAST, types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION, @@ -989,6 +991,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, types::CAST_PTR_ALIGNMENT, + types::CAST_REF_TO_MUT, types::UNIT_CMP, unicode::ZERO_WIDTH_SPACE, unused_io_amount::UNUSED_IO_AMOUNT, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index f4ca8209d67..f9ed38e52a0 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -2240,3 +2240,64 @@ impl<'a, 'b, 'tcx: 'a + 'b> Visitor<'tcx> for ImplicitHasherConstructorVisitor<' NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir()) } } + +/// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code. +/// +/// **Why is this bad?** It’s basically guaranteed to be undefined behaviour. +/// `UnsafeCell` is the only way to obtain aliasable data that is considered +/// mutable. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// fn x(r: &i32) { +/// unsafe { +/// *(r as *const _ as *mut _) += 1; +/// } +/// } +/// ``` +/// +/// Instead consider using interior mutability types. +/// +/// ```rust +/// fn x(r: &UnsafeCell) { +/// unsafe { +/// *r.get() += 1; +/// } +/// } +/// ``` +declare_clippy_lint! { + pub CAST_REF_TO_MUT, + correctness, + "a cast of reference to a mutable pointer" +} + +pub struct RefToMut; + +impl LintPass for RefToMut { + fn get_lints(&self) -> LintArray { + lint_array!(CAST_REF_TO_MUT) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RefToMut { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { + if_chain! { + if let ExprKind::Unary(UnOp::UnDeref, e) = &expr.node; + if let ExprKind::Cast(e, t) = &e.node; + if let TyKind::Ptr(MutTy { mutbl: Mutability::MutMutable, .. }) = t.node; + if let ExprKind::Cast(e, t) = &e.node; + if let TyKind::Ptr(MutTy { mutbl: Mutability::MutImmutable, .. }) = t.node; + if let ty::Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; + then { + span_lint( + cx, + CAST_REF_TO_MUT, + expr.span, + "casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell", + ); + } + } + } +} diff --git a/tests/ui/cast_ref_to_mut.rs b/tests/ui/cast_ref_to_mut.rs new file mode 100644 index 00000000000..967e8e46739 --- /dev/null +++ b/tests/ui/cast_ref_to_mut.rs @@ -0,0 +1,31 @@ +#![warn(clippy::cast_ref_to_mut)] +#![allow(clippy::no_effect)] + +extern "C" { + // NB. Mutability can be easily incorrect in FFI calls, as + // in C, the default are mutable pointers. + fn ffi(c: *mut u8); + fn int_ffi(c: *mut i32); +} + +fn main() { + let s = String::from("Hello"); + let a = &s; + unsafe { + let num = &3i32; + let mut_num = &mut 3i32; + // Should be warned against + (*(a as *const _ as *mut String)).push_str(" world"); + *(a as *const _ as *mut _) = String::from("Replaced"); + *(a as *const _ as *mut String) += " world"; + // Shouldn't be warned against + println!("{}", *(num as *const _ as *const i16)); + println!("{}", *(mut_num as *mut _ as *mut i16)); + ffi(a.as_ptr() as *mut _); + int_ffi(num as *const _ as *mut _); + int_ffi(&3 as *const _ as *mut _); + let mut value = 3; + let value: *const i32 = &mut value; + *(value as *const i16 as *mut i16) = 42; + } +} diff --git a/tests/ui/cast_ref_to_mut.stderr b/tests/ui/cast_ref_to_mut.stderr new file mode 100644 index 00000000000..448a66cfcce --- /dev/null +++ b/tests/ui/cast_ref_to_mut.stderr @@ -0,0 +1,22 @@ +error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell + --> $DIR/cast_ref_to_mut.rs:18:9 + | +LL | (*(a as *const _ as *mut String)).push_str(" world"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::cast-ref-to-mut` implied by `-D warnings` + +error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell + --> $DIR/cast_ref_to_mut.rs:19:9 + | +LL | *(a as *const _ as *mut _) = String::from("Replaced"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell + --> $DIR/cast_ref_to_mut.rs:20:9 + | +LL | *(a as *const _ as *mut String) += " world"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors +