Uplift clippy::invalid_null_ptr_usage lint

compiler/rustc_lint/messages.ftl

@@ -313,6 +313,9 @@ lint_invalid_nan_comparisons_eq_ne = incorrect NaN comparison, NaN cannot be dir
 
 lint_invalid_nan_comparisons_lt_le_gt_ge = incorrect NaN comparison, NaN is not orderable
 
+lint_invalid_null_ptr_usages = calling this function with a null pointer is undefined behavior, even if the result of the function is unused, consider using a dangling pointer instead
+    .suggestion = use a dangling pointer instead
+
 lint_invalid_reference_casting_assign_to_ref = assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
     .label = casting happend here
 
@@ -453,15 +456,6 @@ lint_path_statement_drop = path statement drops value
 
 lint_path_statement_no_effect = path statement with no effect
 
-lint_ptr_null_checks_fn_ptr = 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
-    .label = expression has type `{$orig_ty}`
-
-lint_ptr_null_checks_fn_ret = returned pointer of `{$fn_name}` call is never null, so checking it for null will always return false
-
-lint_ptr_null_checks_ref = references are not nullable, so checking them for null will always return false
-    .label = expression has type `{$orig_ty}`
-
 lint_query_instability = using `{$query}` can result in unstable query results
     .note = if you believe this case to be fine, allow this lint and add a comment explaining your rationale
 
@@ -575,5 +569,14 @@ lint_unused_op = unused {$op} that must be used
 
 lint_unused_result = unused result of type `{$ty}`
 
+lint_useless_ptr_null_checks_fn_ptr = 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
+    .label = expression has type `{$orig_ty}`
+
+lint_useless_ptr_null_checks_fn_ret = returned pointer of `{$fn_name}` call is never null, so checking it for null will always return false
+
+lint_useless_ptr_null_checks_ref = references are not nullable, so checking them for null will always return false
+    .label = expression has type `{$orig_ty}`
+
 lint_variant_size_differences =
     enum variant is more than three times larger ({$largest} bytes) than the next largest

compiler/rustc_lint/src/lints.rs

@@ -596,24 +596,46 @@ pub struct ExpectationNote {
 
 // ptr_nulls.rs
 #[derive(LintDiagnostic)]
-pub enum PtrNullChecksDiag<'a> {
-    #[diag(lint_ptr_null_checks_fn_ptr)]
-    #[help(lint_help)]
+pub enum UselessPtrNullChecksDiag<'a> {
+    #[diag(lint_useless_ptr_null_checks_fn_ptr)]
+    #[help]
     FnPtr {
         orig_ty: Ty<'a>,
         #[label]
         label: Span,
     },
-    #[diag(lint_ptr_null_checks_ref)]
+    #[diag(lint_useless_ptr_null_checks_ref)]
     Ref {
         orig_ty: Ty<'a>,
         #[label]
         label: Span,
     },
-    #[diag(lint_ptr_null_checks_fn_ret)]
+    #[diag(lint_useless_ptr_null_checks_fn_ret)]
     FnRet { fn_name: Ident },
 }
 
+#[derive(LintDiagnostic)]
+#[diag(lint_invalid_null_ptr_usages)]
+pub struct InvalidNullPtrUsagesDiag<'a> {
+    #[subdiagnostic]
+    pub suggestion: InvalidNullPtrUsagesSuggestion<'a>,
+}
+
+#[derive(Subdiagnostic)]
+pub enum InvalidNullPtrUsagesSuggestion<'a> {
+    #[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")]
+    WithoutExplicitType {
+        #[suggestion_part(code = "core::ptr::NonNull::dangling().as_ptr()")]
+        arg_span: Span,
+    },
+    #[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")]
+    WithExplicitType {
+        ty: Ty<'a>,
+        #[suggestion_part(code = "core::ptr::NonNull::<{ty}>::dangling().as_ptr()")]
+        arg_span: Span,
+    },
+}
+
 // for_loops_over_fallibles.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_for_loops_over_fallibles)]

compiler/rustc_lint/src/ptr_nulls.rs

@@ -1,6 +1,11 @@
-use crate::{lints::PtrNullChecksDiag, LateContext, LateLintPass, LintContext};
+use crate::{
+    lints::{InvalidNullPtrUsagesDiag, InvalidNullPtrUsagesSuggestion, UselessPtrNullChecksDiag},
+    reference_casting::peel_casts,
+    LateContext, LateLintPass, LintContext,
+};
 use rustc_ast::LitKind;
 use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind};
+use rustc_middle::ty::{RawPtr, TypeAndMut};
 use rustc_session::{declare_lint, declare_lint_pass};
 use rustc_span::sym;
 
@@ -29,17 +34,40 @@ declare_lint! {
     "useless checking of non-null-typed pointer"
 }
 
-declare_lint_pass!(PtrNullChecks => [USELESS_PTR_NULL_CHECKS]);
+declare_lint! {
+    /// The `invalid_null_ptr_usages` lint checks for invalid usage of null pointers.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// # use std::{slice, ptr};
+    /// // Undefined behavior
+    /// # let _slice: &[u8] =
+    /// unsafe { slice::from_raw_parts(ptr::null(), 0) };
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Calling methods whos safety invariants requires non-null ptr with a null-ptr
+    /// is undefined behavior.
+    INVALID_NULL_PTR_USAGES,
+    Deny,
+    "invalid call with null ptr"
+}
+
+declare_lint_pass!(PtrNullChecks => [USELESS_PTR_NULL_CHECKS, INVALID_NULL_PTR_USAGES]);
 
 /// This function checks if the expression is from a series of consecutive casts,
 /// ie. `(my_fn as *const _ as *mut _).cast_mut()` and whether the original expression is either
 /// a fn ptr, a reference, or a function call whose definition is
 /// annotated with `#![rustc_never_returns_null_ptr]`.
 /// If this situation is present, the function returns the appropriate diagnostic.
-fn incorrect_check<'a, 'tcx: 'a>(
+fn useless_check<'a, 'tcx: 'a>(
     cx: &'a LateContext<'tcx>,
     mut e: &'a Expr<'a>,
-) -> Option<PtrNullChecksDiag<'tcx>> {
+) -> Option<UselessPtrNullChecksDiag<'tcx>> {
     let mut had_at_least_one_cast = false;
     loop {
         e = e.peel_blocks();
@@ -48,14 +76,14 @@ fn incorrect_check<'a, 'tcx: 'a>(
             && cx.tcx.has_attr(def_id, sym::rustc_never_returns_null_ptr)
             && let Some(fn_name) = cx.tcx.opt_item_ident(def_id)
         {
-            return Some(PtrNullChecksDiag::FnRet { fn_name });
+            return Some(UselessPtrNullChecksDiag::FnRet { fn_name });
         } else if let ExprKind::Call(path, _args) = e.kind
             && let ExprKind::Path(ref qpath) = path.kind
             && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
             && cx.tcx.has_attr(def_id, sym::rustc_never_returns_null_ptr)
             && let Some(fn_name) = cx.tcx.opt_item_ident(def_id)
         {
-            return Some(PtrNullChecksDiag::FnRet { fn_name });
+            return Some(UselessPtrNullChecksDiag::FnRet { fn_name });
         }
         e = if let ExprKind::Cast(expr, t) = e.kind
             && let TyKind::Ptr(_) = t.kind
@@ -71,9 +99,9 @@ fn incorrect_check<'a, 'tcx: 'a>(
         } else if had_at_least_one_cast {
             let orig_ty = cx.typeck_results().expr_ty(e);
             return if orig_ty.is_fn() {
-                Some(PtrNullChecksDiag::FnPtr { orig_ty, label: e.span })
+                Some(UselessPtrNullChecksDiag::FnPtr { orig_ty, label: e.span })
             } else if orig_ty.is_ref() {
-                Some(PtrNullChecksDiag::Ref { orig_ty, label: e.span })
+                Some(UselessPtrNullChecksDiag::Ref { orig_ty, label: e.span })
             } else {
                 None
             };
@@ -83,6 +111,24 @@ fn incorrect_check<'a, 'tcx: 'a>(
     }
 }
 
+fn is_null_ptr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
+    let (expr, _) = peel_casts(cx, expr);
+
+    if let ExprKind::Call(path, []) = expr.kind
+        && 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
+    } else if let ExprKind::Lit(spanned) = expr.kind
+        && let LitKind::Int(v, _) = spanned.node
+    {
+        v == 0
+    } else {
+        false
+    }
+}
+
 impl<'tcx> LateLintPass<'tcx> for PtrNullChecks {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         match expr.kind {
@@ -95,11 +141,59 @@ impl<'tcx> LateLintPass<'tcx> for PtrNullChecks {
                         cx.tcx.get_diagnostic_name(def_id),
                         Some(sym::ptr_const_is_null | sym::ptr_is_null)
                     )
-                    && let Some(diag) = incorrect_check(cx, arg) =>
+                    && let Some(diag) = useless_check(cx, arg) =>
             {
                 cx.emit_spanned_lint(USELESS_PTR_NULL_CHECKS, expr.span, diag)
             }
 
+            // Catching:
+            // <path>(arg...) where `arg` is null-ptr and `path` is a fn that expect non-null-ptr
+            ExprKind::Call(path, args)
+                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_name) = cx.tcx.get_diagnostic_name(def_id) =>
+            {
+                // `arg` positions where null would cause U.B.
+                let arg_indices: &[_] = match diag_name {
+                    sym::ptr_read
+                    | sym::ptr_read_unaligned
+                    | sym::ptr_read_volatile
+                    | sym::ptr_replace
+                    | sym::ptr_write
+                    | sym::ptr_write_bytes
+                    | sym::ptr_write_unaligned
+                    | sym::ptr_write_volatile
+                    | sym::slice_from_raw_parts
+                    | sym::slice_from_raw_parts_mut => &[0],
+                    sym::ptr_copy
+                    | sym::ptr_copy_nonoverlapping
+                    | sym::ptr_swap
+                    | sym::ptr_swap_nonoverlapping => &[0, 1],
+                    _ => return,
+                };
+
+                for &arg_idx in arg_indices {
+                    if let Some(arg) = args.get(arg_idx).filter(|arg| is_null_ptr(cx, arg)) {
+                        let arg_span = arg.span;
+
+                        let suggestion = if let ExprKind::Cast(..) = arg.peel_blocks().kind
+                            && let Some(ty) = cx.typeck_results().expr_ty_opt(arg)
+                            && let RawPtr(TypeAndMut { ty, .. }) = ty.kind()
+                        {
+                            InvalidNullPtrUsagesSuggestion::WithExplicitType { ty: *ty, arg_span }
+                        } else {
+                            InvalidNullPtrUsagesSuggestion::WithoutExplicitType { arg_span }
+                        };
+
+                        cx.emit_spanned_lint(
+                            INVALID_NULL_PTR_USAGES,
+                            expr.span,
+                            InvalidNullPtrUsagesDiag { suggestion },
+                        )
+                    }
+                }
+            }
+
             // Catching:
             // (fn_ptr as *<const/mut> <ty>).is_null()
             ExprKind::MethodCall(_, receiver, _, _)
@@ -108,18 +202,18 @@ impl<'tcx> LateLintPass<'tcx> for PtrNullChecks {
                         cx.tcx.get_diagnostic_name(def_id),
                         Some(sym::ptr_const_is_null | sym::ptr_is_null)
                     )
-                    && let Some(diag) = incorrect_check(cx, receiver) =>
+                    && let Some(diag) = useless_check(cx, receiver) =>
             {
                 cx.emit_spanned_lint(USELESS_PTR_NULL_CHECKS, expr.span, diag)
             }
 
             ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => {
                 let to_check: &Expr<'_>;
-                let diag: PtrNullChecksDiag<'_>;
-                if let Some(ddiag) = incorrect_check(cx, left) {
+                let diag: UselessPtrNullChecksDiag<'_>;
+                if let Some(ddiag) = useless_check(cx, left) {
                     to_check = right;
                     diag = ddiag;
-                } else if let Some(ddiag) = incorrect_check(cx, right) {
+                } else if let Some(ddiag) = useless_check(cx, right) {
                     to_check = left;
                     diag = ddiag;
                 } else {

compiler/rustc_lint/src/reference_casting.rs

@@ -151,7 +151,10 @@ fn is_cast_from_ref_to_mut_ptr<'tcx>(
     }
 }
 
-fn peel_casts<'tcx>(cx: &LateContext<'tcx>, mut e: &'tcx Expr<'tcx>) -> (&'tcx Expr<'tcx>, bool) {
+pub(super) fn peel_casts<'tcx>(
+    cx: &LateContext<'tcx>,
+    mut e: &'tcx Expr<'tcx>,
+) -> (&'tcx Expr<'tcx>, bool) {
     let mut gone_trough_unsafe_cell_raw_get = false;
 
     loop {

src/tools/clippy/clippy_lints/src/declared_lints.rs

@@ -569,7 +569,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO,
     crate::precedence::PRECEDENCE_INFO,
     crate::ptr::CMP_NULL_INFO,
-    crate::ptr::INVALID_NULL_PTR_USAGE_INFO,
     crate::ptr::MUT_FROM_REF_INFO,
     crate::ptr::PTR_ARG_INFO,
     crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,

src/tools/clippy/clippy_lints/src/ptr.rs

@@ -1,6 +1,6 @@
 //! Checks for usage of  `&Vec[_]` and `&String`.
 
-use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
+use clippy_utils::diagnostics::{span_lint, span_lint_and_then, span_lint_hir_and_then};
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::expr_sig;
 use clippy_utils::visitors::contains_unsafe_block;
@@ -129,30 +129,7 @@ declare_clippy_lint! {
     "fns that create mutable refs from immutable ref args"
 }
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// This lint checks for invalid usages of `ptr::null`.
-    ///
-    /// ### Why is this bad?
-    /// This causes undefined behavior.
-    ///
-    /// ### Example
-    /// ```ignore
-    /// // Undefined behavior
-    /// unsafe { std::slice::from_raw_parts(ptr::null(), 0); }
-    /// ```
-    ///
-    /// Use instead:
-    /// ```ignore
-    /// unsafe { std::slice::from_raw_parts(NonNull::dangling().as_ptr(), 0); }
-    /// ```
-    #[clippy::version = "1.53.0"]
-    pub INVALID_NULL_PTR_USAGE,
-    correctness,
-    "invalid usage of a null pointer, suggesting `NonNull::dangling()` instead"
-}
-
-declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE]);
+declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF]);
 
 impl<'tcx> LateLintPass<'tcx> for Ptr {
     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
@@ -264,48 +241,6 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
                     "comparing with null is better expressed by the `.is_null()` method",
                 );
             }
-        } else {
-            check_invalid_ptr_usage(cx, expr);
-        }
-    }
-}
-
-fn check_invalid_ptr_usage<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-    if let ExprKind::Call(fun, args) = expr.kind
-        && let ExprKind::Path(ref qpath) = fun.kind
-        && let Some(fun_def_id) = cx.qpath_res(qpath, fun.hir_id).opt_def_id()
-        && let Some(name) = cx.tcx.get_diagnostic_name(fun_def_id)
-    {
-        // `arg` positions where null would cause U.B.
-        let arg_indices: &[_] = match name {
-            sym::ptr_read
-            | sym::ptr_read_unaligned
-            | sym::ptr_read_volatile
-            | sym::ptr_replace
-            | sym::ptr_slice_from_raw_parts
-            | sym::ptr_slice_from_raw_parts_mut
-            | sym::ptr_write
-            | sym::ptr_write_bytes
-            | sym::ptr_write_unaligned
-            | sym::ptr_write_volatile
-            | sym::slice_from_raw_parts
-            | sym::slice_from_raw_parts_mut => &[0],
-            sym::ptr_copy | sym::ptr_copy_nonoverlapping | sym::ptr_swap | sym::ptr_swap_nonoverlapping => &[0, 1],
-            _ => return,
-        };
-
-        for &arg_idx in arg_indices {
-            if let Some(arg) = args.get(arg_idx).filter(|arg| is_null_path(cx, arg)) {
-                span_lint_and_sugg(
-                    cx,
-                    INVALID_NULL_PTR_USAGE,
-                    arg.span,
-                    "pointer must be non-null",
-                    "change this to",
-                    "core::ptr::NonNull::dangling().as_ptr()".to_string(),
-                    Applicability::MachineApplicable,
-                );
-            }
         }
     }
 }