From 6adb9cb53f5d3ee899398603c6de628ec55f99fe Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Thu, 24 Mar 2016 15:48:38 -0700 Subject: [PATCH 1/3] Added crosspointer transmute error and tests --- src/lib.rs | 2 ++ src/transmute.rs | 58 +++++++++++++++++++++++++++++++++ tests/compile-fail/transmute.rs | 31 ++++++++++++++++-- 3 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ffa66fd58..8a4a84f5d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -195,6 +195,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box no_effect::NoEffectPass); reg.register_late_lint_pass(box map_clone::MapClonePass); reg.register_late_lint_pass(box temporary_assignment::TemporaryAssignmentPass); + reg.register_late_lint_pass(box transmute::CrosspointerTransmute); reg.register_late_lint_pass(box transmute::UselessTransmute); reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(conf.cyclomatic_complexity_threshold)); reg.register_late_lint_pass(box escape::EscapePass); @@ -350,6 +351,7 @@ pub fn plugin_registrar(reg: &mut Registry) { swap::ALMOST_SWAPPED, swap::MANUAL_SWAP, temporary_assignment::TEMPORARY_ASSIGNMENT, + transmute::CROSSPOINTER_TRANSMUTE, transmute::USELESS_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, types::BOX_VEC, diff --git a/src/transmute.rs b/src/transmute.rs index 0dd5b60e7..daefbaf07 100644 --- a/src/transmute.rs +++ b/src/transmute.rs @@ -1,5 +1,7 @@ use rustc::lint::*; use rustc_front::hir::*; +use rustc::middle::ty::TyS; +use rustc::middle::ty::TypeVariants::TyRawPtr; use utils; /// **What it does:** This lint checks for transmutes to the original type of the object. @@ -15,6 +17,19 @@ declare_lint! { "transmutes that have the same to and from types" } +/// **What it does:*** This lint checks for transmutes between a type T and *T. +/// +/// **Why is this bad?** It's easy to mistakenly transmute between a type and a pointer to that type. +/// +/// **Known problems:** None. +/// +/// **Example:** `core::intrinsics::transmute(t)` where the result type is the same as `*t` or `&t`'s. +declare_lint! { + pub CROSSPOINTER_TRANSMUTE, + Warn, + "transmutes that have to or from types that are a pointer to the other" +} + pub struct UselessTransmute; impl LintPass for UselessTransmute { @@ -43,3 +58,46 @@ impl LateLintPass for UselessTransmute { } } } + +pub struct CrosspointerTransmute; + +impl LintPass for CrosspointerTransmute { + fn get_lints(&self) -> LintArray { + lint_array!(CROSSPOINTER_TRANSMUTE) + } +} + +fn is_ptr_to(from: &TyS, to: &TyS) -> bool { + if let TyRawPtr(from_ptr) = from.sty { + from_ptr.ty == to + } else { + false + } +} + +impl LateLintPass for CrosspointerTransmute { + fn check_expr(&mut self, cx: &LateContext, e: &Expr) { + if let ExprCall(ref path_expr, ref args) = e.node { + if let ExprPath(None, _) = path_expr.node { + let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); + + if utils::match_def_path(cx, def_id, &["core", "intrinsics", "transmute"]) { + let from_ty = cx.tcx.expr_ty(&args[0]); + let to_ty = cx.tcx.expr_ty(e); + + if is_ptr_to(to_ty, from_ty) { + cx.span_lint(CROSSPOINTER_TRANSMUTE, + e.span, + &format!("transmute from a type (`{}`) to a pointer to that type (`{}`)", from_ty, to_ty)); + } + + if is_ptr_to(from_ty, to_ty) { + cx.span_lint(CROSSPOINTER_TRANSMUTE, + e.span, + &format!("transmute from a type (`{}`) to the type that it points to (`{}`)", from_ty, to_ty)); + } + } + } + } + } +} diff --git a/tests/compile-fail/transmute.rs b/tests/compile-fail/transmute.rs index 94dd3a185..b87550080 100644 --- a/tests/compile-fail/transmute.rs +++ b/tests/compile-fail/transmute.rs @@ -1,6 +1,5 @@ #![feature(plugin)] #![plugin(clippy)] -#![deny(useless_transmute)] extern crate core; @@ -12,6 +11,7 @@ fn my_vec() -> MyVec { } #[allow(needless_lifetimes)] +#[deny(useless_transmute)] unsafe fn _generic<'a, T, U: 'a>(t: &'a T) { let _: &'a T = core::intrinsics::transmute(t); //~^ ERROR transmute from a type (`&'a T`) to itself @@ -19,7 +19,8 @@ unsafe fn _generic<'a, T, U: 'a>(t: &'a T) { let _: &'a U = core::intrinsics::transmute(t); } -fn main() { +#[deny(useless_transmute)] +fn useless() { unsafe { let _: Vec = core::intrinsics::transmute(my_vec()); //~^ ERROR transmute from a type (`collections::vec::Vec`) to itself @@ -43,3 +44,29 @@ fn main() { let _: Vec = my_transmute(my_vec()); } } + +#[deny(crosspointer_transmute)] +fn crosspointer() { + let mut vec: Vec = vec![]; + let vec_const_ptr: *const Vec = &vec as *const Vec; + let vec_mut_ptr: *mut Vec = &mut vec as *mut Vec; + + unsafe { + let _: Vec = core::intrinsics::transmute(vec_const_ptr); + //~^ ERROR transmute from a type (`*const collections::vec::Vec`) to the type that it points to (`collections::vec::Vec`) + + let _: Vec = core::intrinsics::transmute(vec_mut_ptr); + //~^ ERROR transmute from a type (`*mut collections::vec::Vec`) to the type that it points to (`collections::vec::Vec`) + + let _: *const Vec = core::intrinsics::transmute(my_vec()); + //~^ ERROR transmute from a type (`collections::vec::Vec`) to a pointer to that type (`*const collections::vec::Vec`) + + let _: *mut Vec = core::intrinsics::transmute(my_vec()); + //~^ ERROR transmute from a type (`collections::vec::Vec`) to a pointer to that type (`*mut collections::vec::Vec`) + } +} + +fn main() { + useless(); + crosspointer(); +} From b07360eb2852953fcb07da738da0e2266058b2c0 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Thu, 24 Mar 2016 16:38:16 -0700 Subject: [PATCH 2/3] Cleanup and added transmute to ugly path list --- src/transmute.rs | 11 +++++------ src/utils/mod.rs | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/transmute.rs b/src/transmute.rs index daefbaf07..488d214b2 100644 --- a/src/transmute.rs +++ b/src/transmute.rs @@ -3,6 +3,7 @@ use rustc_front::hir::*; use rustc::middle::ty::TyS; use rustc::middle::ty::TypeVariants::TyRawPtr; use utils; +use utils::TRANSMUTE_PATH; /// **What it does:** This lint checks for transmutes to the original type of the object. /// @@ -17,7 +18,7 @@ declare_lint! { "transmutes that have the same to and from types" } -/// **What it does:*** This lint checks for transmutes between a type T and *T. +/// **What it does:*** This lint checks for transmutes between a type `T` and `*T`. /// /// **Why is this bad?** It's easy to mistakenly transmute between a type and a pointer to that type. /// @@ -44,7 +45,7 @@ impl LateLintPass for UselessTransmute { if let ExprPath(None, _) = path_expr.node { let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); - if utils::match_def_path(cx, def_id, &["core", "intrinsics", "transmute"]) { + if utils::match_def_path(cx, def_id, &TRANSMUTE_PATH) { let from_ty = cx.tcx.expr_ty(&args[0]); let to_ty = cx.tcx.expr_ty(e); @@ -81,7 +82,7 @@ impl LateLintPass for CrosspointerTransmute { if let ExprPath(None, _) = path_expr.node { let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); - if utils::match_def_path(cx, def_id, &["core", "intrinsics", "transmute"]) { + if utils::match_def_path(cx, def_id, &TRANSMUTE_PATH) { let from_ty = cx.tcx.expr_ty(&args[0]); let to_ty = cx.tcx.expr_ty(e); @@ -89,9 +90,7 @@ impl LateLintPass for CrosspointerTransmute { cx.span_lint(CROSSPOINTER_TRANSMUTE, e.span, &format!("transmute from a type (`{}`) to a pointer to that type (`{}`)", from_ty, to_ty)); - } - - if is_ptr_to(from_ty, to_ty) { + } else if is_ptr_to(from_ty, to_ty) { cx.span_lint(CROSSPOINTER_TRANSMUTE, e.span, &format!("transmute from a type (`{}`) to the type that it points to (`{}`)", from_ty, to_ty)); diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 050aec0e4..e7b2a9d52 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -51,6 +51,7 @@ pub const RANGE_TO_PATH: [&'static str; 3] = ["std", "ops", "RangeTo"]; pub const REGEX_NEW_PATH: [&'static str; 3] = ["regex", "Regex", "new"]; pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"]; pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"]; +pub const TRANSMUTE_PATH: [&'static str; 3] = ["core", "intrinsics", "transmute"]; pub const VEC_FROM_ELEM_PATH: [&'static str; 3] = ["std", "vec", "from_elem"]; pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"]; From bafffbd624c25bdec62df6655ee6b1786da0e11d Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Thu, 24 Mar 2016 16:39:22 -0700 Subject: [PATCH 3/3] Ran python lint updater --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b3e693760..0ad56769c 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Table of contents: * [License](#license) ##Lints -There are 134 lints included in this crate: +There are 135 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -39,6 +39,7 @@ name [cmp_nan](https://github.com/Manishearth/rust-clippy/wiki#cmp_nan) | deny | comparisons to NAN (which will always return false, which is probably not intended) [cmp_owned](https://github.com/Manishearth/rust-clippy/wiki#cmp_owned) | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()` [collapsible_if](https://github.com/Manishearth/rust-clippy/wiki#collapsible_if) | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }` and an `else { if .. } expression can be collapsed to `else if` +[crosspointer_transmute](https://github.com/Manishearth/rust-clippy/wiki#crosspointer_transmute) | warn | transmutes that have to or from types that are a pointer to the other [cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | finds functions that should be split up into multiple functions [deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | `Warn` on `#[deprecated(since = "x")]` where x is not semver [derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly