From 34daf09aa41e24fada5bf10e4193bf85b910a3ac Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Sat, 29 Dec 2018 19:25:27 +0100 Subject: [PATCH 1/6] cast_ref_to_mut lint --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/lib.rs | 2 ++ clippy_lints/src/types.rs | 61 +++++++++++++++++++++++++++++++++ tests/ui/cast_ref_to_mut.rs | 31 +++++++++++++++++ tests/ui/cast_ref_to_mut.stderr | 22 ++++++++++++ 6 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 tests/ui/cast_ref_to_mut.rs create mode 100644 tests/ui/cast_ref_to_mut.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index efa637b185cd..7acf9bc1eaa8 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 be54424dc381..8ca10da416dd 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 2e515cc8aea6..84e201118971 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, @@ -1026,6 +1027,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { mutex_atomic::MUTEX_INTEGER, needless_borrow::NEEDLESS_BORROW, redundant_clone::REDUNDANT_CLONE, + types::CAST_REF_TO_MUT, unwrap::PANICKING_UNWRAP, unwrap::UNNECESSARY_UNWRAP, ]); diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index f4ca8209d67e..5091662e3d98 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, + nursery, + "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::TyKind::Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; + then { + span_lint( + cx, + CAST_REF_TO_MUT, + expr.span, + "casting immutable reference to a mutable reference" + ); + } + } + } +} diff --git a/tests/ui/cast_ref_to_mut.rs b/tests/ui/cast_ref_to_mut.rs new file mode 100644 index 000000000000..967e8e467396 --- /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 000000000000..a3e4fc5412cf --- /dev/null +++ b/tests/ui/cast_ref_to_mut.stderr @@ -0,0 +1,22 @@ +error: casting immutable reference to a mutable reference + --> $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 immutable reference to a mutable reference + --> $DIR/cast_ref_to_mut.rs:19:9 + | +LL | *(a as *const _ as *mut _) = String::from("Replaced"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: casting immutable reference to a mutable reference + --> $DIR/cast_ref_to_mut.rs:20:9 + | +LL | *(a as *const _ as *mut String) += " world"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + From fd5787410625cdea037a70bc85fc657453e2aeb3 Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Sun, 30 Dec 2018 13:02:58 +0100 Subject: [PATCH 2/6] Use ty::Ref instead of ty::TyKind::Ref --- clippy_lints/src/types.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 5091662e3d98..43603c287029 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -25,7 +25,7 @@ use rustc::hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisito use rustc::hir::*; use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; use rustc::ty::layout::LayoutOf; -use rustc::ty::{self, Ty, TyCtxt, TypeckTables}; +use rustc::ty::{self, Ref, Ty, TyCtxt, TypeckTables}; use rustc::{declare_tool_lint, lint_array}; use rustc_errors::Applicability; use rustc_target::spec::abi::Abi; @@ -2289,7 +2289,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RefToMut { 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::TyKind::Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; + if let Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; then { span_lint( cx, From 1cab4d15a2e95223ad8d812e5f0932e31242f665 Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Sun, 30 Dec 2018 13:06:37 +0100 Subject: [PATCH 3/6] Add a note to cast_ref_to_mut lint --- clippy_lints/src/types.rs | 10 ++++++---- tests/ui/cast_ref_to_mut.stderr | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 43603c287029..097ff1fecc08 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -16,8 +16,8 @@ use crate::utils::paths; use crate::utils::{ clip, comparisons, differing_macro_contexts, higher, in_constant, in_macro, int_bits, last_path_segment, match_def_path, match_path, multispan_sugg, opt_def_id, same_tys, sext, snippet, snippet_opt, - snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext, - AbsolutePathBuffer, + snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, + span_note_and_lint, unsext, AbsolutePathBuffer, }; use if_chain::if_chain; use rustc::hir; @@ -2291,11 +2291,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RefToMut { if let TyKind::Ptr(MutTy { mutbl: Mutability::MutImmutable, .. }) = t.node; if let Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; then { - span_lint( + span_note_and_lint( cx, CAST_REF_TO_MUT, expr.span, - "casting immutable reference to a mutable reference" + "casting immutable reference to a mutable reference", + expr.span, + "consider implementing `UnsafeCell` instead", ); } } diff --git a/tests/ui/cast_ref_to_mut.stderr b/tests/ui/cast_ref_to_mut.stderr index a3e4fc5412cf..23f03f0113d4 100644 --- a/tests/ui/cast_ref_to_mut.stderr +++ b/tests/ui/cast_ref_to_mut.stderr @@ -5,18 +5,23 @@ LL | (*(a as *const _ as *mut String)).push_str(" world"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::cast-ref-to-mut` implied by `-D warnings` + = note: consider implementing `UnsafeCell` instead error: casting immutable reference to a mutable reference --> $DIR/cast_ref_to_mut.rs:19:9 | LL | *(a as *const _ as *mut _) = String::from("Replaced"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: consider implementing `UnsafeCell` instead error: casting immutable reference to a mutable reference --> $DIR/cast_ref_to_mut.rs:20:9 | LL | *(a as *const _ as *mut String) += " world"; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: consider implementing `UnsafeCell` instead error: aborting due to 3 previous errors From 6faf1330aaa42f9f524b7f84a881111a536ca10d Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Sun, 30 Dec 2018 13:11:53 +0100 Subject: [PATCH 4/6] Move a hint to an error message in cast_ref_to_mut lint This matches mem::transmute::<&T, &mut T> lint in rustc. --- clippy_lints/src/types.rs | 10 ++++------ tests/ui/cast_ref_to_mut.stderr | 11 +++-------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 097ff1fecc08..a9fb595e708c 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -16,8 +16,8 @@ use crate::utils::paths; use crate::utils::{ clip, comparisons, differing_macro_contexts, higher, in_constant, in_macro, int_bits, last_path_segment, match_def_path, match_path, multispan_sugg, opt_def_id, same_tys, sext, snippet, snippet_opt, - snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, - span_note_and_lint, unsext, AbsolutePathBuffer, + snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext, + AbsolutePathBuffer, }; use if_chain::if_chain; use rustc::hir; @@ -2291,13 +2291,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RefToMut { if let TyKind::Ptr(MutTy { mutbl: Mutability::MutImmutable, .. }) = t.node; if let Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; then { - span_note_and_lint( + span_lint( cx, CAST_REF_TO_MUT, expr.span, - "casting immutable reference to a mutable reference", - expr.span, - "consider implementing `UnsafeCell` instead", + "casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell", ); } } diff --git a/tests/ui/cast_ref_to_mut.stderr b/tests/ui/cast_ref_to_mut.stderr index 23f03f0113d4..448a66cfcce0 100644 --- a/tests/ui/cast_ref_to_mut.stderr +++ b/tests/ui/cast_ref_to_mut.stderr @@ -1,27 +1,22 @@ -error: casting immutable reference to a mutable reference +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` - = note: consider implementing `UnsafeCell` instead -error: casting immutable reference to a mutable reference +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"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: consider implementing `UnsafeCell` instead -error: casting immutable reference to a mutable reference +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"; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: consider implementing `UnsafeCell` instead error: aborting due to 3 previous errors From 21d30450b51adf206626aacf6fe0abf52a7b4ec5 Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Sun, 30 Dec 2018 13:12:28 +0100 Subject: [PATCH 5/6] Don't import ty::Ref in cast_ref_to_mut lint --- clippy_lints/src/types.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index a9fb595e708c..3acc82edf291 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -25,7 +25,7 @@ use rustc::hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisito use rustc::hir::*; use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; use rustc::ty::layout::LayoutOf; -use rustc::ty::{self, Ref, Ty, TyCtxt, TypeckTables}; +use rustc::ty::{self, Ty, TyCtxt, TypeckTables}; use rustc::{declare_tool_lint, lint_array}; use rustc_errors::Applicability; use rustc_target::spec::abi::Abi; @@ -2289,7 +2289,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RefToMut { 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 Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; + if let ty::Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; then { span_lint( cx, From 27ea638a15b5d71e06a2dcc3cc996f35b4ab3bd1 Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Mon, 7 Jan 2019 14:39:56 +0100 Subject: [PATCH 6/6] Move cast_ref_to_mut list to correctness group --- clippy_lints/src/lib.rs | 3 ++- clippy_lints/src/types.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 84e201118971..35c00fb63284 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -759,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, @@ -990,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, @@ -1027,7 +1029,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { mutex_atomic::MUTEX_INTEGER, needless_borrow::NEEDLESS_BORROW, redundant_clone::REDUNDANT_CLONE, - types::CAST_REF_TO_MUT, unwrap::PANICKING_UNWRAP, unwrap::UNNECESSARY_UNWRAP, ]); diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 3acc82edf291..f9ed38e52a02 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -2269,7 +2269,7 @@ impl<'a, 'b, 'tcx: 'a + 'b> Visitor<'tcx> for ImplicitHasherConstructorVisitor<' /// ``` declare_clippy_lint! { pub CAST_REF_TO_MUT, - nursery, + correctness, "a cast of reference to a mutable pointer" }