mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #10099 - Niki4tap:null_fn_lints, r=llogiq

Browse Source 
Null fn lints

Adds lints to check for code, that assumes nullable `fn()`.

### Lint examples:

`transmute_null_to_fn`:
```rust
error: transmuting a known null pointer into a function pointer
  --> $DIR/transmute_null_to_fn.rs:9:23
   |
LL |         let _: fn() = std::mem::transmute(std::ptr::null::<()>());
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this transmute results in undefined behavior
   |
   = help: try wrapping your function pointer type in `Option<T>` instead, and using `None` as a null pointer value
```

`fn_null_check`:
```rust
error: function pointer assumed to be nullable, even though it isn't
  --> $DIR/fn_null_check.rs:13:8
   |
LL |     if (fn_ptr as *mut ()).is_null() {}
   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value
```

Closes #1644

---

changelog: Improvement: [`transmuting_null`]: Now detects `const` pointers to all types
[#10099](https://github.com/rust-lang/rust-clippy/pull/10099)
changelog: New lint: [`transmute_null_to_fn`]
[#10099](https://github.com/rust-lang/rust-clippy/pull/10099)
changelog: New lint: [`fn_null_check`]
[#10099](https://github.com/rust-lang/rust-clippy/pull/10099)
<!-- changelog_checked (This is just a flag for me, please don't add it manually) -->
This commit is contained in:
bors 2022-12-19 12:44:23 +00:00
commit b3145fea6a
12 changed files with 328 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -4203,6 +4203,7 @@ Released 2018-09-13
[`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const
[`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_equality_without_abs
[`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_address_comparisons
[`fn_null_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_null_check
[`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools
[`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast
[`fn_to_numeric_cast_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_any
@ -4590,6 +4591,7 @@ Released 2018-09-13
[`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool
[`transmute_int_to_char`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_char
[`transmute_int_to_float`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_float
[`transmute_null_to_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_null_to_fn
[`transmute_num_to_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_num_to_bytes
[`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

2
clippy_lints/src/declared_lints.rs
View File

@ -161,6 +161,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::float_literal::LOSSY_FLOAT_LITERAL_INFO,
crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO,
crate::floating_point_arithmetic::SUBOPTIMAL_FLOPS_INFO,
crate::fn_null_check::FN_NULL_CHECK_INFO,
crate::format::USELESS_FORMAT_INFO,
crate::format_args::FORMAT_IN_FORMAT_ARGS_INFO,
crate::format_args::TO_STRING_IN_FORMAT_ARGS_INFO,
@ -568,6 +569,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::transmute::TRANSMUTE_INT_TO_BOOL_INFO,
crate::transmute::TRANSMUTE_INT_TO_CHAR_INFO,
crate::transmute::TRANSMUTE_INT_TO_FLOAT_INFO,
crate::transmute::TRANSMUTE_NULL_TO_FN_INFO,
crate::transmute::TRANSMUTE_NUM_TO_BYTES_INFO,
crate::transmute::TRANSMUTE_PTR_TO_PTR_INFO,
crate::transmute::TRANSMUTE_PTR_TO_REF_INFO,

106
clippy_lints/src/fn_null_check.rs Normal file
View File

@ -0,0 +1,106 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::{is_integer_literal, is_path_diagnostic_item};
use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
/// Checks for comparing a function pointer to null.
///
/// ### Why is this bad?
/// Function pointers are assumed to not be null.
///
/// ### Example
/// ```rust,ignore
/// let fn_ptr: fn() = /* somehow obtained nullable function pointer */
///
/// if (fn_ptr as *const ()).is_null() { ... }
/// ```
/// Use instead:
/// ```rust,ignore
/// let fn_ptr: Option<fn()> = /* somehow obtained nullable function pointer */
///
/// if fn_ptr.is_none() { ... }
/// ```
#[clippy::version = "1.67.0"]
pub FN_NULL_CHECK,
correctness,
"`fn()` type assumed to be nullable"
}
declare_lint_pass!(FnNullCheck => [FN_NULL_CHECK]);
fn lint_expr(cx: &LateContext<'_>, expr: &Expr<'_>) {
span_lint_and_help(
cx,
FN_NULL_CHECK,
expr.span,
"function pointer assumed to be nullable, even though it isn't",
None,
"try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value",
);
}
fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let ExprKind::Cast(cast_expr, cast_ty) = expr.kind
&& let TyKind::Ptr(_) = cast_ty.kind
{
cx.typeck_results().expr_ty_adjusted(cast_expr).is_fn()
} else {
false
}
}
impl<'tcx> LateLintPass<'tcx> for FnNullCheck {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
match expr.kind {
// Catching:
// (fn_ptr as *<const/mut> <ty>).is_null()
ExprKind::MethodCall(method_name, receiver, _, _)
if method_name.ident.as_str() == "is_null" && is_fn_ptr_cast(cx, receiver) =>
{
lint_expr(cx, expr);
},
ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => {
let to_check: &Expr<'_>;
if is_fn_ptr_cast(cx, left) {
to_check = right;
} else if is_fn_ptr_cast(cx, right) {
to_check = left;
} else {
return;
}
match to_check.kind {
// Catching:
// (fn_ptr as *<const/mut> <ty>) == (0 as <ty>)
ExprKind::Cast(cast_expr, _) if is_integer_literal(cast_expr, 0) => {
lint_expr(cx, expr);
},
// Catching:
// (fn_ptr as *<const/mut> <ty>) == std::ptr::null()
ExprKind::Call(func, []) if is_path_diagnostic_item(cx, func, sym::ptr_null) => {
lint_expr(cx, expr);
},
// Catching:
// (fn_ptr as *<const/mut> <ty>) == <const that evaluates to null_ptr>
_ if matches!(
constant(cx, cx.typeck_results(), to_check),
Some((Constant::RawPtr(0), _))
) =>
{
lint_expr(cx, expr);
},
_ => {},
}
},
_ => {},
}
}
}

2
clippy_lints/src/lib.rs
View File

@ -125,6 +125,7 @@ mod explicit_write;
mod fallible_impl_from;
mod float_literal;
mod floating_point_arithmetic;
mod fn_null_check;
mod format;
mod format_args;
mod format_impl;
@ -902,6 +903,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(suspicious_xor_used_as_pow::ConfusingXorAndPow));
store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv())));
store.register_late_pass(|_| Box::new(semicolon_block::SemicolonBlock));
store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck));
// add lints here, do not remove this comment, it's used in `new_lint`
}

31
clippy_lints/src/transmute/mod.rs
View File

@ -3,6 +3,7 @@ mod transmute_float_to_int;
mod transmute_int_to_bool;
mod transmute_int_to_char;
mod transmute_int_to_float;
mod transmute_null_to_fn;
mod transmute_num_to_bytes;
mod transmute_ptr_to_ptr;
mod transmute_ptr_to_ref;
@ -409,6 +410,34 @@ declare_clippy_lint! {
"transmutes from a null pointer to a reference, which is undefined behavior"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for null function pointer creation through transmute.
///
/// ### Why is this bad?
/// Creating a null function pointer is undefined behavior.
///
/// More info: https://doc.rust-lang.org/nomicon/ffi.html#the-nullable-pointer-optimization
///
/// ### 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_fn: fn() = unsafe { std::mem::transmute( std::ptr::null::<()>() ) };
/// ```
/// Use instead:
/// ```rust
/// let null_fn: Option<fn()> = None;
/// ```
#[clippy::version = "1.67.0"]
pub TRANSMUTE_NULL_TO_FN,
correctness,
"transmute results in a null function pointer, which is undefined behavior"
}
pub struct Transmute {
msrv: Msrv,
}
@ -428,6 +457,7 @@ impl_lint_pass!(Transmute => [
TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS,
TRANSMUTE_UNDEFINED_REPR,
TRANSMUTING_NULL,
TRANSMUTE_NULL_TO_FN,
]);
impl Transmute {
#[must_use]
@ -461,6 +491,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute {
let linted = wrong_transmute::check(cx, e, from_ty, to_ty)
| crosspointer_transmute::check(cx, e, from_ty, to_ty)
| transmuting_null::check(cx, e, arg, to_ty)
| transmute_null_to_fn::check(cx, e, arg, to_ty)
| transmute_ptr_to_ref::check(cx, e, from_ty, to_ty, arg, path, &self.msrv)
| transmute_int_to_char::check(cx, e, from_ty, to_ty, arg, const_context)
| transmute_ref_to_ref::check(cx, e, from_ty, to_ty, arg, const_context)

64
clippy_lints/src/transmute/transmute_null_to_fn.rs Normal file
View File

@ -0,0 +1,64 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{is_integer_literal, is_path_diagnostic_item};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::Ty;
use rustc_span::symbol::sym;
use super::TRANSMUTE_NULL_TO_FN;
fn lint_expr(cx: &LateContext<'_>, expr: &Expr<'_>) {
span_lint_and_then(
cx,
TRANSMUTE_NULL_TO_FN,
expr.span,
"transmuting a known null pointer into a function pointer",
|diag| {
diag.span_label(expr.span, "this transmute results in undefined behavior");
diag.help(
"try wrapping your function pointer type in `Option<T>` instead, and using `None` as a null pointer value"
);
},
);
}
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'tcx Expr<'_>, to_ty: Ty<'tcx>) -> bool {
if !to_ty.is_fn() {
return false;
}
match arg.kind {
// Catching:
// transmute over constants that resolve to `null`.
ExprKind::Path(ref _qpath)
if matches!(constant(cx, cx.typeck_results(), arg), Some((Constant::RawPtr(0), _))) =>
{
lint_expr(cx, expr);
true
},
// Catching:
// `std::mem::transmute(0 as *const i32)`
ExprKind::Cast(inner_expr, _cast_ty) if is_integer_literal(inner_expr, 0) => {
lint_expr(cx, expr);
true
},
// Catching:
// `std::mem::transmute(std::ptr::null::<i32>())`
ExprKind::Call(func1, []) if is_path_diagnostic_item(cx, func1, sym::ptr_null) => {
lint_expr(cx, expr);
true
},
_ => {
// 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.
false
},
}
}

3
clippy_lints/src/transmute/transmuting_null.rs
View File

@ -18,8 +18,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t
// Catching transmute over constants that resolve to `null`.
let mut const_eval_context = constant_context(cx, cx.typeck_results());
if let ExprKind::Path(ref _qpath) = arg.kind &&
let Some(Constant::RawPtr(x)) = const_eval_context.expr(arg) &&
x == 0
let Some(Constant::RawPtr(0)) = const_eval_context.expr(arg)
{
span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG);
return true;

7
clippy_utils/src/consts.rs
View File

@ -620,12 +620,7 @@ pub fn miri_to_const<'tcx>(tcx: TyCtxt<'tcx>, result: mir::ConstantKind<'tcx>) -
ty::Float(FloatTy::F64) => Some(Constant::F64(f64::from_bits(
int.try_into().expect("invalid f64 bit representation"),
))),
ty::RawPtr(type_and_mut) => {
if let ty::Uint(_) = type_and_mut.ty.kind() {
return Some(Constant::RawPtr(int.assert_bits(int.size())));
}
None
},
ty::RawPtr(_) => Some(Constant::RawPtr(int.assert_bits(int.size()))),
// FIXME: implement other conversions.
_ => None,
}

21
tests/ui/fn_null_check.rs Normal file
View File

@ -0,0 +1,21 @@
#![allow(unused)]
#![warn(clippy::fn_null_check)]
#![allow(clippy::cmp_null)]
#![allow(clippy::ptr_eq)]
#![allow(clippy::zero_ptr)]
pub const ZPTR: *const () = 0 as *const _;
pub const NOT_ZPTR: *const () = 1 as *const _;
fn main() {
let fn_ptr = main;
if (fn_ptr as *mut ()).is_null() {}
if (fn_ptr as *const u8).is_null() {}
if (fn_ptr as *const ()) == std::ptr::null() {}
if (fn_ptr as *const ()) == (0 as *const ()) {}
if (fn_ptr as *const ()) == ZPTR {}
// no lint
if (fn_ptr as *const ()) == NOT_ZPTR {}
}

43
tests/ui/fn_null_check.stderr Normal file
View File

@ -0,0 +1,43 @@
error: function pointer assumed to be nullable, even though it isn't
--> $DIR/fn_null_check.rs:13:8
|
LL | if (fn_ptr as *mut ()).is_null() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value
= note: `-D clippy::fn-null-check` implied by `-D warnings`
error: function pointer assumed to be nullable, even though it isn't
--> $DIR/fn_null_check.rs:14:8
|
LL | if (fn_ptr as *const u8).is_null() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value
error: function pointer assumed to be nullable, even though it isn't
--> $DIR/fn_null_check.rs:15:8
|
LL | if (fn_ptr as *const ()) == std::ptr::null() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value
error: function pointer assumed to be nullable, even though it isn't
--> $DIR/fn_null_check.rs:16:8
|
LL | if (fn_ptr as *const ()) == (0 as *const ()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value
error: function pointer assumed to be nullable, even though it isn't
--> $DIR/fn_null_check.rs:17:8
|
LL | if (fn_ptr as *const ()) == ZPTR {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value
error: aborting due to 5 previous errors

28
tests/ui/transmute_null_to_fn.rs Normal file
View File

@ -0,0 +1,28 @@
#![allow(dead_code)]
#![warn(clippy::transmute_null_to_fn)]
#![allow(clippy::zero_ptr)]
// Easy to lint because these only span one line.
fn one_liners() {
unsafe {
let _: fn() = std::mem::transmute(0 as *const ());
let _: fn() = 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 _: fn() = std::mem::transmute(ZPTR);
// Should NOT raise a lint.
let _: fn() = std::mem::transmute(NOT_ZPTR);
}
}
fn main() {
one_liners();
transmute_const();
}

27
tests/ui/transmute_null_to_fn.stderr Normal file
View File

@ -0,0 +1,27 @@
error: transmuting a known null pointer into a function pointer
--> $DIR/transmute_null_to_fn.rs:8:23
|
LL | let _: fn() = std::mem::transmute(0 as *const ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this transmute results in undefined behavior
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `None` as a null pointer value
= note: `-D clippy::transmute-null-to-fn` implied by `-D warnings`
error: transmuting a known null pointer into a function pointer
--> $DIR/transmute_null_to_fn.rs:9:23
|
LL | let _: fn() = std::mem::transmute(std::ptr::null::<()>());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this transmute results in undefined behavior
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `None` as a null pointer value
error: transmuting a known null pointer into a function pointer
--> $DIR/transmute_null_to_fn.rs:19:23
|
LL | let _: fn() = std::mem::transmute(ZPTR);
| ^^^^^^^^^^^^^^^^^^^^^^^^^ this transmute results in undefined behavior
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `None` as a null pointer value
error: aborting due to 3 previous errors