From f6d2bf63d3ce225a79dfdb9bab0f93568ad6e137 Mon Sep 17 00:00:00 2001 From: Urgau Date: Wed, 17 May 2023 11:22:26 +0200 Subject: [PATCH] Uplift `clippy::fn_null_check` to rustc --- compiler/rustc_lint/messages.ftl | 3 + compiler/rustc_lint/src/fn_null_check.rs | 112 +++++++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 6 ++ tests/ui/lint/fn_null_check.rs | 30 ++++++ tests/ui/lint/fn_null_check.stderr | 67 ++++++++++++++ 6 files changed, 221 insertions(+) create mode 100644 compiler/rustc_lint/src/fn_null_check.rs create mode 100644 tests/ui/lint/fn_null_check.rs create mode 100644 tests/ui/lint/fn_null_check.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 34b7e09576a..2c92277b50d 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -215,6 +215,9 @@ lint_expectation = this lint expectation is unfulfilled .note = the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message .rationale = {$rationale} +lint_fn_null_check = function pointers are not nullable, so checking them for null will always return false + .help = wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + lint_for_loops_over_fallibles = for loop over {$article} `{$ty}`. This is more readably written as an `if let` statement .suggestion = consider using `if let` to clear intent diff --git a/compiler/rustc_lint/src/fn_null_check.rs b/compiler/rustc_lint/src/fn_null_check.rs new file mode 100644 index 00000000000..e3b33463ccf --- /dev/null +++ b/compiler/rustc_lint/src/fn_null_check.rs @@ -0,0 +1,112 @@ +use crate::{lints::FnNullCheckDiag, LateContext, LateLintPass, LintContext}; +use rustc_ast::LitKind; +use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind}; +use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_span::sym; + +declare_lint! { + /// The `incorrect_fn_null_checks` lint checks for expression that checks if a + /// function pointer is null. + /// + /// ### Example + /// + /// ```rust + /// # fn test() {} + /// let fn_ptr: fn() = /* somehow obtained nullable function pointer */ + /// # test; + /// + /// if (fn_ptr as *const ()).is_null() { /* ... */ } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Function pointers are assumed to be non-null, checking them for null will always + /// return false. + INCORRECT_FN_NULL_CHECKS, + Warn, + "incorrect checking of null function pointer" +} + +declare_lint_pass!(IncorrectFnNullChecks => [INCORRECT_FN_NULL_CHECKS]); + +fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let mut expr = expr.peel_blocks(); + let mut had_at_least_one_cast = false; + while let ExprKind::Cast(cast_expr, cast_ty) = expr.kind + && let TyKind::Ptr(_) = cast_ty.kind { + expr = cast_expr.peel_blocks(); + had_at_least_one_cast = true; + } + had_at_least_one_cast && cx.typeck_results().expr_ty_adjusted(expr).is_fn() +} + +impl<'tcx> LateLintPass<'tcx> for IncorrectFnNullChecks { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + match expr.kind { + // Catching: + // <* >::is_null(fn_ptr as * ) + ExprKind::Call(path, [arg]) + if let ExprKind::Path(ref qpath) = path.kind + && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() + && matches!( + cx.tcx.get_diagnostic_name(def_id), + Some(sym::ptr_const_is_null | sym::ptr_is_null) + ) + && is_fn_ptr_cast(cx, arg) => + { + cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag) + } + + // Catching: + // (fn_ptr as * ).is_null() + ExprKind::MethodCall(_, receiver, _, _) + if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) + && matches!( + cx.tcx.get_diagnostic_name(def_id), + Some(sym::ptr_const_is_null | sym::ptr_is_null) + ) + && is_fn_ptr_cast(cx, receiver) => + { + cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag) + } + + 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 * ) == (0 as ) + ExprKind::Cast(cast_expr, _) + if let ExprKind::Lit(spanned) = cast_expr.kind + && let LitKind::Int(v, _) = spanned.node && v == 0 => + { + cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag) + }, + + // Catching: + // (fn_ptr as * ) == std::ptr::null() + ExprKind::Call(path, []) + if let ExprKind::Path(ref qpath) = path.kind + && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() + && let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id) + && (diag_item == sym::ptr_null || diag_item == sym::ptr_null_mut) => + { + cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag) + }, + + _ => {}, + } + } + _ => {} + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 5e3f057d428..beb38dbb94c 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -58,6 +58,7 @@ mod enum_intrinsics_non_enums; mod errors; mod expect; +mod fn_null_check; mod for_loops_over_fallibles; pub mod hidden_unicode_codepoints; mod internal; @@ -102,6 +103,7 @@ use deref_into_dyn_supertrait::*; use drop_forget_useless::*; use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; +use fn_null_check::*; use for_loops_over_fallibles::*; use hidden_unicode_codepoints::*; use internal::*; @@ -225,6 +227,7 @@ fn lint_mod(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { // Depends on types used in type definitions MissingCopyImplementations: MissingCopyImplementations, // Depends on referenced function signatures in expressions + IncorrectFnNullChecks: IncorrectFnNullChecks, MutableTransmutes: MutableTransmutes, TypeAliasBounds: TypeAliasBounds, TrivialConstraints: TrivialConstraints, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 0613ef2c5e9..1dea758bb29 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -613,6 +613,12 @@ pub struct ExpectationNote { pub rationale: Symbol, } +// fn_null_check.rs +#[derive(LintDiagnostic)] +#[diag(lint_fn_null_check)] +#[help] +pub struct FnNullCheckDiag; + // for_loops_over_fallibles.rs #[derive(LintDiagnostic)] #[diag(lint_for_loops_over_fallibles)] diff --git a/tests/ui/lint/fn_null_check.rs b/tests/ui/lint/fn_null_check.rs new file mode 100644 index 00000000000..7f01f2c4283 --- /dev/null +++ b/tests/ui/lint/fn_null_check.rs @@ -0,0 +1,30 @@ +// check-pass + +fn main() { + let fn_ptr = main; + + if (fn_ptr as *mut ()).is_null() {} + //~^ WARN function pointers are not nullable + if (fn_ptr as *const u8).is_null() {} + //~^ WARN function pointers are not nullable + if (fn_ptr as *const ()) == std::ptr::null() {} + //~^ WARN function pointers are not nullable + if (fn_ptr as *mut ()) == std::ptr::null_mut() {} + //~^ WARN function pointers are not nullable + if (fn_ptr as *const ()) == (0 as *const ()) {} + //~^ WARN function pointers are not nullable + if <*const _>::is_null(fn_ptr as *const ()) {} + //~^ WARN function pointers are not nullable + if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {} + //~^ WARN function pointers are not nullable + if (fn_ptr as fn() as *const ()).is_null() {} + //~^ WARN function pointers are not nullable + + const ZPTR: *const () = 0 as *const _; + const NOT_ZPTR: *const () = 1 as *const _; + + // unlike the uplifted clippy::fn_null_check lint we do + // not lint on them + if (fn_ptr as *const ()) == ZPTR {} + if (fn_ptr as *const ()) == NOT_ZPTR {} +} diff --git a/tests/ui/lint/fn_null_check.stderr b/tests/ui/lint/fn_null_check.stderr new file mode 100644 index 00000000000..0398c0da50f --- /dev/null +++ b/tests/ui/lint/fn_null_check.stderr @@ -0,0 +1,67 @@ +warning: function pointers are not nullable, so checking them for null will always return false + --> $DIR/fn_null_check.rs:6:8 + | +LL | if (fn_ptr as *mut ()).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + = note: `#[warn(incorrect_fn_null_checks)]` on by default + +warning: function pointers are not nullable, so checking them for null will always return false + --> $DIR/fn_null_check.rs:8:8 + | +LL | if (fn_ptr as *const u8).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + +warning: function pointers are not nullable, so checking them for null will always return false + --> $DIR/fn_null_check.rs:10:8 + | +LL | if (fn_ptr as *const ()) == std::ptr::null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + +warning: function pointers are not nullable, so checking them for null will always return false + --> $DIR/fn_null_check.rs:12:8 + | +LL | if (fn_ptr as *mut ()) == std::ptr::null_mut() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + +warning: function pointers are not nullable, so checking them for null will always return false + --> $DIR/fn_null_check.rs:14:8 + | +LL | if (fn_ptr as *const ()) == (0 as *const ()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + +warning: function pointers are not nullable, so checking them for null will always return false + --> $DIR/fn_null_check.rs:16:8 + | +LL | if <*const _>::is_null(fn_ptr as *const ()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + +warning: function pointers are not nullable, so checking them for null will always return false + --> $DIR/fn_null_check.rs:18:8 + | +LL | if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + +warning: function pointers are not nullable, so checking them for null will always return false + --> $DIR/fn_null_check.rs:20:8 + | +LL | if (fn_ptr as fn() as *const ()).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value + +warning: 8 warnings emitted +