Auto merge of #3848 - felix91gr:null_transmute, r=<try>

Transmuting known null ptr to ref

Working on implementing #628

Author: bors

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

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:
 

clippy_lints/src/consts.rs

@@ -41,6 +41,8 @@ pub enum Constant {
     Repeat(Box<Constant>, u64),
     /// A tuple of constants.
     Tuple(Vec<Constant>),
+    /// 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<Constant> {
         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<Constant>> {
         vec.iter().map(|elem| self.expr(elem)).collect::<Option<_>>()
     }
 
-    /// 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<Constant> {
         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<Constant> {
         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,
     }
 }

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,

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::<i32>())`
+                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.
+            }
+        }
+    }
+}

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 _;
 

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::<u64>());
+    }
+}
+
+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();
+}

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::<u64>());
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+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
+