Skip to content

Commit

Permalink
Auto merge of #3848 - felix91gr:null_transmute, r=flip1995
Browse files Browse the repository at this point in the history
Transmuting known null ptr to ref

Working on implementing #628
  • Loading branch information
bors committed Apr 8, 2019
2 parents 42e1cf3 + 069957a commit e91ba8a
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
37 changes: 24 additions & 13 deletions clippy_lints/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down Expand Up @@ -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);
},
Expand Down Expand Up @@ -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>,
Expand All @@ -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),
Expand All @@ -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;
Expand All @@ -262,7 +267,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
}
}
},
// TODO: add other expressions
// TODO: add other expressions.
_ => None,
}
}
Expand Down Expand Up @@ -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;

Expand All @@ -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))
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}
}
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
112 changes: 112 additions & 0 deletions clippy_lints/src/transmuting_null.rs
Original file line number Diff line number Diff line change
@@ -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.
}
}
}
}
1 change: 1 addition & 0 deletions tests/ui/issue_3849.rs
Original file line number Diff line number Diff line change
@@ -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 _;

Expand Down
30 changes: 30 additions & 0 deletions tests/ui/transmuting_null.rs
Original file line number Diff line number Diff line change
@@ -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();
}
22 changes: 22 additions & 0 deletions tests/ui/transmuting_null.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit e91ba8a

Please sign in to comment.