-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Null fn lints #10099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Null fn lints #10099
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
6afe547
Add lint `transmute_null_to_fn`
Niki4tap 3cc67d0
Relax clippy_utils::consts::miri_to_const pointer type restrictiveness
Niki4tap 42106e0
Add lint `fn_null_check`
Niki4tap 54a9168
Remove useless pattern matching
Niki4tap dae54fa
Doc codeblock fixup
Niki4tap b1ca307
Address some of the code style issues
Niki4tap 9b2fc8e
Make clippy happy
Niki4tap 20f501a
Improve code style further
Niki4tap cc98574
Fix comments, use `constant` instead of raw `constant_context`
Niki4tap 691df70
Inline some `const`s
Niki4tap File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
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 if_chain::if_chain; | ||
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]); | ||
|
||
const LINT_MSG: &str = "function pointer assumed to be nullable, even though it isn't"; | ||
const HELP_MSG: &str = "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_chain! { | ||
if let ExprKind::Cast(cast_expr, cast_ty) = expr.kind; | ||
if let TyKind::Ptr(_) = cast_ty.kind; | ||
if cx.typeck_results().expr_ty_adjusted(cast_expr).is_fn(); | ||
then { | ||
true | ||
} else { | ||
false | ||
} | ||
Niki4tap marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
impl<'tcx> LateLintPass<'tcx> for FnNullCheck { | ||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | ||
// Catching: | ||
// (fn_ptr as *<const/mut> <ty>).is_null() | ||
if_chain! { | ||
if let ExprKind::MethodCall(method_name, receiver, _, _) = expr.kind; | ||
Niki4tap marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if method_name.ident.as_str() == "is_null"; | ||
if is_fn_ptr_cast(cx, receiver); | ||
then { | ||
span_lint_and_help( | ||
cx, | ||
FN_NULL_CHECK, | ||
expr.span, | ||
LINT_MSG, | ||
None, | ||
HELP_MSG | ||
); | ||
Niki4tap marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
if let ExprKind::Binary(op, left, right) = expr.kind | ||
&& let BinOpKind::Eq = op.node | ||
{ | ||
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; | ||
} | ||
|
||
// Catching: | ||
// (fn_ptr as *<const/mut> <ty>) == <const that evaluates to null_ptr> | ||
let c = constant(cx, cx.typeck_results(), to_check); | ||
if let Some((Constant::RawPtr(0), _)) = c { | ||
span_lint_and_help( | ||
cx, | ||
FN_NULL_CHECK, | ||
expr.span, | ||
LINT_MSG, | ||
None, | ||
HELP_MSG | ||
); | ||
return; | ||
} | ||
|
||
// Catching: | ||
// (fn_ptr as *<const/mut> <ty>) == (0 as <ty>) | ||
if let ExprKind::Cast(cast_expr, _) = to_check.kind && is_integer_literal(cast_expr, 0) { | ||
span_lint_and_help( | ||
cx, | ||
FN_NULL_CHECK, | ||
expr.span, | ||
LINT_MSG, | ||
None, | ||
HELP_MSG | ||
); | ||
return; | ||
} | ||
|
||
// Catching: | ||
// (fn_ptr as *<const/mut> <ty>) == std::ptr::null() | ||
if let ExprKind::Call(func, []) = to_check.kind && | ||
is_path_diagnostic_item(cx, func, sym::ptr_null) | ||
{ | ||
span_lint_and_help( | ||
cx, | ||
FN_NULL_CHECK, | ||
expr.span, | ||
LINT_MSG, | ||
None, | ||
HELP_MSG | ||
); | ||
} | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
use clippy_utils::consts::{constant_context, 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; | ||
|
||
const LINT_MSG: &str = "transmuting a known null pointer into a function pointer"; | ||
Niki4tap marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const NOTE_MSG: &str = "this transmute results in undefined behavior"; | ||
const HELP_MSG: &str = | ||
"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; | ||
} | ||
|
||
// 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 && | ||
Niki4tap marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let Some(Constant::RawPtr(0)) = const_eval_context.expr(arg) | ||
{ | ||
span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { | ||
diag.span_label(expr.span, NOTE_MSG); | ||
diag.help(HELP_MSG); | ||
}); | ||
return true; | ||
} | ||
|
||
// Catching: | ||
// `std::mem::transmute(0 as *const i32)` | ||
if let ExprKind::Cast(inner_expr, _cast_ty) = arg.kind && is_integer_literal(inner_expr, 0) { | ||
span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { | ||
diag.span_label(expr.span, NOTE_MSG); | ||
diag.help(HELP_MSG); | ||
}); | ||
return true; | ||
} | ||
|
||
// Catching: | ||
// `std::mem::transmute(std::ptr::null::<i32>())` | ||
if let ExprKind::Call(func1, []) = arg.kind && | ||
is_path_diagnostic_item(cx, func1, sym::ptr_null) | ||
{ | ||
span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { | ||
diag.span_label(expr.span, NOTE_MSG); | ||
diag.help(HELP_MSG); | ||
}); | ||
return 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 | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 {} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.