diff --git a/CHANGELOG.md b/CHANGELOG.md index b05fa25fb1b..7837b899b53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1022,6 +1022,7 @@ All notable changes to this project will be documented in this file. [`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 +[`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 [`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity diff --git a/README.md b/README.md index 0fd7c557dd6..43b477d2cef 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 297 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 298 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/consts.rs b/clippy_lints/src/consts.rs index 61bfc78b3de..66fbf18f3dd 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -41,6 +41,8 @@ pub enum Constant { Repeat(Box, u64), /// A tuple of constants. Tuple(Vec), + /// A raw pointer. + RawPtr(u128), /// A literal with syntax error. Err(Symbol), } @@ -109,6 +111,9 @@ impl Hash for Constant { c.hash(state); l.hash(state); }, + Constant::RawPtr(u) => { + u.hash(state); + }, Constant::Err(ref s) => { s.hash(state); }, @@ -192,7 +197,7 @@ pub fn constant_simple<'c, 'cc>( constant(lcx, tables, e).and_then(|(cst, res)| if res { None } else { Some(cst) }) } -/// Creates a `ConstEvalLateContext` from the given `LateContext` and `TypeckTables` +/// Creates a `ConstEvalLateContext` from the given `LateContext` and `TypeckTables`. pub fn constant_context<'c, 'cc>( lcx: &LateContext<'c, 'cc>, tables: &'c ty::TypeckTables<'cc>, @@ -215,7 +220,7 @@ pub struct ConstEvalLateContext<'a, 'tcx: 'a> { } impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { - /// simple constant folding: Insert an expression, get a constant or none. + /// Simple constant folding: Insert an expression, get a constant or none. pub fn expr(&mut self, e: &Expr) -> Option { match e.node { ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id), @@ -238,7 +243,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { }), ExprKind::Binary(op, ref left, ref right) => self.binop(op, left, right), ExprKind::Call(ref callee, ref args) => { - // We only handle a few const functions for now + // We only handle a few const functions for now. if_chain! { if args.is_empty(); if let ExprKind::Path(qpath) = &callee.node; @@ -262,7 +267,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { } } }, - // TODO: add other expressions + // TODO: add other expressions. _ => None, } } @@ -304,13 +309,13 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { } } - /// create `Some(Vec![..])` of all constants, unless there is any - /// non-constant part + /// Create `Some(Vec![..])` of all constants, unless there is any + /// non-constant part. fn multi(&mut self, vec: &[Expr]) -> Option> { vec.iter().map(|elem| self.expr(elem)).collect::>() } - /// lookup a possibly constant expression from a ExprKind::Path + /// Lookup a possibly constant expression from a ExprKind::Path. fn fetch_path(&mut self, qpath: &QPath, id: HirId) -> Option { use rustc::mir::interpret::GlobalId; @@ -334,14 +339,14 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { if ret.is_some() { self.needed_resolution = true; } - return ret; + ret }, - _ => {}, + // FIXME: cover all useable cases. + _ => None, } - None } - /// A block can only yield a constant if it only has one constant expression + /// A block can only yield a constant if it only has one constant expression. fn block(&mut self, block: &Block) -> Option { if block.stmts.is_empty() { block.expr.as_ref().and_then(|b| self.expr(b)) @@ -467,7 +472,13 @@ pub fn miri_to_const<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, result: &ty::Const<' ty::Float(FloatTy::F64) => Some(Constant::F64(f64::from_bits( b.try_into().expect("invalid f64 bit representation"), ))), - // FIXME: implement other conversion + ty::RawPtr(type_and_mut) => { + if let ty::Uint(_) = type_and_mut.ty.sty { + return Some(Constant::RawPtr(b)); + } + None + }, + // FIXME: implement other conversions. _ => None, }, ConstValue::Slice(Scalar::Ptr(ptr), n) => match result.ty.sty { @@ -484,7 +495,7 @@ pub fn miri_to_const<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, result: &ty::Const<' }, _ => None, }, - // FIXME: implement other conversions + // FIXME: implement other conversions. _ => None, } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index bf98aa7e2b5..f7d9524c6a2 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -255,6 +255,7 @@ pub mod suspicious_trait_impl; pub mod swap; pub mod temporary_assignment; pub mod transmute; +pub mod transmuting_null; pub mod trivially_copy_pass_by_ref; pub mod types; pub mod unicode; @@ -570,6 +571,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box types::RefToMut); reg.register_late_lint_pass(box assertions_on_constants::AssertionsOnConstants); reg.register_late_lint_pass(box missing_const_for_fn::MissingConstForFn); + reg.register_late_lint_pass(box transmuting_null::Pass); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -841,6 +843,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { transmute::TRANSMUTE_PTR_TO_REF, transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, + transmuting_null::TRANSMUTING_NULL, trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, types::ABSURD_EXTREME_COMPARISONS, types::BORROWED_BOX, @@ -1078,6 +1081,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL, swap::ALMOST_SWAPPED, transmute::WRONG_TRANSMUTE, + transmuting_null::TRANSMUTING_NULL, types::ABSURD_EXTREME_COMPARISONS, types::CAST_PTR_ALIGNMENT, types::CAST_REF_TO_MUT, diff --git a/clippy_lints/src/transmuting_null.rs b/clippy_lints/src/transmuting_null.rs new file mode 100644 index 00000000000..445abc4832a --- /dev/null +++ b/clippy_lints/src/transmuting_null.rs @@ -0,0 +1,112 @@ +use crate::consts::{constant_context, Constant}; +use crate::utils::{match_qpath, span_lint}; +use if_chain::if_chain; +use rustc::hir::{Expr, ExprKind}; +use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; +use rustc::{declare_tool_lint, lint_array}; +use syntax::ast::LitKind; + +declare_clippy_lint! { + /// **What it does:** Checks for transmute calls which would receive a null pointer. + /// + /// **Why is this bad?** Transmuting a null pointer is undefined behavior. + /// + /// **Known problems:** Not all cases can be detected at the moment of this writing. + /// For example, variables which hold a null pointer and are then fed to a `transmute` + /// call, aren't detectable yet. + /// + /// **Example:** + /// ```rust + /// let null_ref: &u64 = unsafe { std::mem::transmute(0 as *const u64) }; + /// ``` + pub TRANSMUTING_NULL, + correctness, + "transmutes from a null pointer to a reference, which is undefined behavior" +} + +#[derive(Copy, Clone)] +pub struct Pass; + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array!(TRANSMUTING_NULL,) + } + + fn name(&self) -> &'static str { + "TransmutingNull" + } +} + +const LINT_MSG: &str = "transmuting a known null pointer into a reference."; + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { + if in_external_macro(cx.sess(), expr.span) { + return; + } + + if_chain! { + if let ExprKind::Call(ref func, ref args) = expr.node; + if let ExprKind::Path(ref path) = func.node; + if match_qpath(path, &["std", "mem", "transmute"]); + if args.len() == 1; + + then { + + // Catching transmute over constants that resolve to `null`. + let mut const_eval_context = constant_context(cx, cx.tables); + if_chain! { + if let ExprKind::Path(ref _qpath) = args[0].node; + let x = const_eval_context.expr(&args[0]); + if let Some(constant) = x; + if let Constant::RawPtr(ptr_value) = constant; + if ptr_value == 0; + then { + span_lint( + cx, + TRANSMUTING_NULL, + expr.span, + LINT_MSG) + } + } + + // Catching: + // `std::mem::transmute(0 as *const i32)` + if_chain! { + if let ExprKind::Cast(ref inner_expr, ref _cast_ty) = args[0].node; + if let ExprKind::Lit(ref lit) = inner_expr.node; + if let LitKind::Int(0, _) = lit.node; + then { + span_lint( + cx, + TRANSMUTING_NULL, + expr.span, + LINT_MSG) + } + } + + // Catching: + // `std::mem::transmute(std::ptr::null::())` + if_chain! { + if let ExprKind::Call(ref func1, ref args1) = args[0].node; + if let ExprKind::Path(ref path1) = func1.node; + if match_qpath(path1, &["std", "ptr", "null"]); + if args1.len() == 0; + then { + span_lint( + cx, + TRANSMUTING_NULL, + expr.span, + LINT_MSG) + } + } + + // FIXME: + // Also catch transmutations of variables which are known nulls. + // To do this, MIR const propagation seems to be the better tool. + // Whenever MIR const prop routines are more developed, this will + // become available. As of this writing (25/03/19) it is not yet. + } + } + } +} diff --git a/tests/ui/issue_3849.rs b/tests/ui/issue_3849.rs index 33462b0ea77..bae4570e539 100644 --- a/tests/ui/issue_3849.rs +++ b/tests/ui/issue_3849.rs @@ -1,6 +1,7 @@ #![allow(dead_code)] #![allow(clippy::zero_ptr)] #![allow(clippy::transmute_ptr_to_ref)] +#![allow(clippy::transmuting_null)] pub const ZPTR: *const usize = 0 as *const _; diff --git a/tests/ui/transmuting_null.rs b/tests/ui/transmuting_null.rs new file mode 100644 index 00000000000..ea3ee8edc81 --- /dev/null +++ b/tests/ui/transmuting_null.rs @@ -0,0 +1,30 @@ +#![allow(dead_code)] +#![warn(clippy::transmuting_null)] +#![allow(clippy::zero_ptr)] +#![allow(clippy::transmute_ptr_to_ref)] +#![allow(clippy::eq_op)] + +// Easy to lint because these only span one line. +fn one_liners() { + unsafe { + let _: &u64 = std::mem::transmute(0 as *const u64); + let _: &u64 = std::mem::transmute(std::ptr::null::()); + } +} + +pub const ZPTR: *const usize = 0 as *const _; +pub const NOT_ZPTR: *const usize = 1 as *const _; + +fn transmute_const() { + unsafe { + // Should raise a lint. + let _: &u64 = std::mem::transmute(ZPTR); + // Should NOT raise a lint. + let _: &u64 = std::mem::transmute(NOT_ZPTR); + } +} + +fn main() { + one_liners(); + transmute_const(); +} diff --git a/tests/ui/transmuting_null.stderr b/tests/ui/transmuting_null.stderr new file mode 100644 index 00000000000..05f91ee2ada --- /dev/null +++ b/tests/ui/transmuting_null.stderr @@ -0,0 +1,22 @@ +error: transmuting a known null pointer into a reference. + --> $DIR/transmuting_null.rs:10:23 + | +LL | let _: &u64 = std::mem::transmute(0 as *const u64); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::transmuting-null` implied by `-D warnings` + +error: transmuting a known null pointer into a reference. + --> $DIR/transmuting_null.rs:11:23 + | +LL | let _: &u64 = std::mem::transmute(std::ptr::null::()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: transmuting a known null pointer into a reference. + --> $DIR/transmuting_null.rs:21:23 + | +LL | let _: &u64 = std::mem::transmute(ZPTR); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors +