Auto merge of rust-lang#119220 - Urgau:uplift-invalid_null_ptr_usage, r=fee1-dead

Uplift `clippy::invalid_null_ptr_usage` lint as `invalid_null_arguments`

This PR aims at uplifting the `clippy::invalid_null_ptr_usage` lint into rustc, this is similar to the [`clippy::invalid_utf8_in_unchecked` uplift](rust-lang#111543) a few months ago, in the sense that those two lints lint on invalid parameter(s), here a null pointer where it is unexpected and UB to pass one.

*For context: GitHub Search reveals that just for `slice::from_raw_parts{_mut}` [~20 invalid usages](hhttps://github.com/search?q=lang%3Arust+%2Fslice%3A%3Afrom_raw_parts%28_mut%29%3F%5C%28ptr%3A%3Anull%2F+NOT+path%3A%2F%5Eclippy_lints%5C%2Fsrc%5C%2F%2F+NOT+path%3A%2F%5Erust%5C%2Fsrc%5C%2Ftools%5C%2Fclippy%5C%2Fclippy_lints%5C%2Fsrc%5C%2F%2F+NOT+path%3A%2F%5Esrc%5C%2Ftools%5C%2Fclippy%5C%2Fclippy_lints%5C%2Fsrc%5C%2F%2F&type=code) with `ptr::null` and an additional [4 invalid usages](https://github.com/search?q=lang%3Arust+%2Fslice%3A%3Afrom_raw_parts%5C%280%28%5C%29%7C+as%29%2F+NOT+path%3A%2F%5Eclippy_lints%5C%2Fsrc%5C%2F%2F+NOT+path%3A%2F%5Erust%5C%2Fsrc%5C%2Ftools%5C%2Fclippy%5C%2Fclippy_lints%5C%2Fsrc%5C%2F%2F+NOT+path%3A%2F%5Esrc%5C%2Ftools%5C%2Fclippy%5C%2Fclippy_lints%5C%2Fsrc%5C%2F%2F+NOT+path%3A%2F%5Eutils%5C%2Ftinystr%5C%2Fsrc%5C%2F%2F+NOT+path%3A%2F%5Eutils%5C%2Fzerovec%5C%2Fsrc%5C%2F%2F+NOT+path%3A%2F%5Eprovider%5C%2Fcore%5C%2Fsrc%5C%2F%2F&type=code) with `0 as *const ...`-ish casts.*

-----

## `invalid_null_arguments`

(deny-by-default)

The `invalid_null_arguments` lint checks for invalid usage of null pointers.

### Example

```rust
// Undefined behavior
unsafe { std::slice::from_raw_parts(ptr::null(), 1); }
```

Produces:
```
error: calling this function with a null pointer is Undefined Behavior, even if the result of the function is unused
  --> $DIR/invalid_null_args.rs:21:23
   |
LL |     let _: &[usize] = std::slice::from_raw_parts(ptr::null_mut(), 0);
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^---------------^^^^
   |                                                  |
   |                                                  null pointer originates from here
   |
   = help: for more information, visit <https://doc.rust-lang.org/std/ptr/index.html> and <https://doc.rust-lang.org/reference/behavior-considered-undefined.html>
```

### Explanation

Calling methods whose safety invariants requires non-null pointer with a null pointer is undefined behavior.

-----

The lint use a list of functions to know which functions and arguments to checks, this could be improved in the future with a rustc attribute, or maybe even with a `#[diagnostic]` attribute.

This PR also includes some small refactoring to avoid some ambiguities in naming, those can be done in another PR is desired.

`@rustbot` label: +I-lang-nominated
r? compiler

Author: bors

compiler/rustc_lint/messages.ftl

@@ -456,6 +456,10 @@ 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_arguments = calling this function with a null pointer is undefined behavior, even if the result of the function is unused
+    .origin = null pointer originates from here
+    .doc = for more information, visit <https://doc.rust-lang.org/std/ptr/index.html> and <https://doc.rust-lang.org/reference/behavior-considered-undefined.html>
+
 lint_invalid_reference_casting_assign_to_ref = assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
     .label = casting happened here
 
@@ -680,15 +684,6 @@ lint_private_extern_crate_reexport = extern crate `{$ident}` is private and cann
 lint_proc_macro_derive_resolution_fallback = cannot find {$ns} `{$ident}` in this scope
     .label = names from parent modules are not accessible without an explicit import
 
-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
 
@@ -981,6 +976,15 @@ lint_unused_result = unused result of type `{$ty}`
 
 lint_use_let_underscore_ignore_suggestion = use `let _ = ...` to ignore the expression or result
 
+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_uses_power_alignment = repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type
 
 lint_variant_size_differences =

compiler/rustc_lint/src/lib.rs

@@ -80,6 +80,7 @@ mod types;
 mod unit_bindings;
 mod unqualified_local_imports;
 mod unused;
+mod utils;
 
 use async_closures::AsyncClosureUsage;
 use async_fn_in_trait::AsyncFnInTrait;

compiler/rustc_lint/src/lints.rs

@@ -591,24 +591,40 @@ pub(crate) struct ExpectationNote {
 
 // ptr_nulls.rs
 #[derive(LintDiagnostic)]
-pub(crate) enum PtrNullChecksDiag<'a> {
-    #[diag(lint_ptr_null_checks_fn_ptr)]
-    #[help(lint_help)]
+pub(crate) 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)]
+pub(crate) enum InvalidNullArgumentsDiag {
+    #[diag(lint_invalid_null_arguments)]
+    #[help(lint_doc)]
+    NullPtrInline {
+        #[label(lint_origin)]
+        null_span: Span,
+    },
+    #[diag(lint_invalid_null_arguments)]
+    #[help(lint_doc)]
+    NullPtrThroughBinding {
+        #[note(lint_origin)]
+        null_span: Span,
+    },
+}
+
 // for_loops_over_fallibles.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_for_loops_over_fallibles)]

compiler/rustc_lint/src/ptr_nulls.rs

@@ -1,9 +1,11 @@
 use rustc_ast::LitKind;
 use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind};
+use rustc_middle::ty::RawPtr;
 use rustc_session::{declare_lint, declare_lint_pass};
-use rustc_span::sym;
+use rustc_span::{Span, sym};
 
-use crate::lints::PtrNullChecksDiag;
+use crate::lints::{InvalidNullArgumentsDiag, UselessPtrNullChecksDiag};
+use crate::utils::peel_casts;
 use crate::{LateContext, LateLintPass, LintContext};
 
 declare_lint! {
@@ -31,17 +33,40 @@ declare_lint! {
     "useless checking of non-null-typed pointer"
 }
 
-declare_lint_pass!(PtrNullChecks => [USELESS_PTR_NULL_CHECKS]);
+declare_lint! {
+    /// The `invalid_null_arguments` lint checks for invalid usage of null pointers in arguments.
+    ///
+    /// ### 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 pointer
+    /// is [Undefined Behavior](https://doc.rust-lang.org/reference/behavior-considered-undefined.html)!
+    INVALID_NULL_ARGUMENTS,
+    Deny,
+    "invalid null pointer in arguments"
+}
+
+declare_lint_pass!(PtrNullChecks => [USELESS_PTR_NULL_CHECKS, INVALID_NULL_ARGUMENTS]);
 
 /// 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();
@@ -50,14 +75,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
@@ -73,9 +98,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
             };
@@ -85,6 +110,25 @@ fn incorrect_check<'a, 'tcx: 'a>(
     }
 }
 
+/// Checks if the given expression is a null pointer (modulo casting)
+fn is_null_ptr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Span> {
+    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).then_some(expr.span)
+    } else if let ExprKind::Lit(spanned) = expr.kind
+        && let LitKind::Int(v, _) = spanned.node
+    {
+        (v == 0).then_some(expr.span)
+    } else {
+        None
+    }
+}
+
 impl<'tcx> LateLintPass<'tcx> for PtrNullChecks {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         match expr.kind {
@@ -97,11 +141,67 @@ 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_span_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 and whenever ZST are allowed.
+                //
+                // We should probably have a `rustc` attribute, but checking them is costly,
+                // maybe if we checked for null ptr first, it would be acceptable?
+                let (arg_indices, are_zsts_allowed): (&[_], _) = 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 => (&[0], true),
+                    sym::slice_from_raw_parts | sym::slice_from_raw_parts_mut => (&[0], false),
+                    sym::ptr_copy
+                    | sym::ptr_copy_nonoverlapping
+                    | sym::ptr_swap
+                    | sym::ptr_swap_nonoverlapping => (&[0, 1], true),
+                    _ => return,
+                };
+
+                for &arg_idx in arg_indices {
+                    if let Some(arg) = args.get(arg_idx)
+                        && let Some(null_span) = is_null_ptr(cx, arg)
+                        && let Some(ty) = cx.typeck_results().expr_ty_opt(arg)
+                        && let RawPtr(ty, _mutbl) = ty.kind()
+                    {
+                        // If ZST are fine, don't lint on them
+                        let typing_env = cx.typing_env();
+                        if are_zsts_allowed
+                            && cx
+                                .tcx
+                                .layout_of(typing_env.as_query_input(*ty))
+                                .is_ok_and(|layout| layout.is_1zst())
+                        {
+                            break;
+                        }
+
+                        let diag = if arg.span.contains(null_span) {
+                            InvalidNullArgumentsDiag::NullPtrInline { null_span }
+                        } else {
+                            InvalidNullArgumentsDiag::NullPtrThroughBinding { null_span }
+                        };
+
+                        cx.emit_span_lint(INVALID_NULL_ARGUMENTS, expr.span, diag)
+                    }
+                }
+            }
+
             // Catching:
             // (fn_ptr as *<const/mut> <ty>).is_null()
             ExprKind::MethodCall(_, receiver, _, _)
@@ -110,18 +210,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_span_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

@@ -6,6 +6,7 @@ use rustc_session::{declare_lint, declare_lint_pass};
 use rustc_span::sym;
 
 use crate::lints::InvalidReferenceCastingDiag;
+use crate::utils::peel_casts;
 use crate::{LateContext, LateLintPass, LintContext};
 
 declare_lint! {
@@ -235,46 +236,3 @@ fn is_cast_to_bigger_memory_layout<'tcx>(
         None
     }
 }
-
-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 {
-        e = e.peel_blocks();
-        // <expr> as ...
-        e = if let ExprKind::Cast(expr, _) = e.kind {
-            expr
-        // <expr>.cast(), <expr>.cast_mut() or <expr>.cast_const()
-        } else if let ExprKind::MethodCall(_, expr, [], _) = e.kind
-            && let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
-            && matches!(
-                cx.tcx.get_diagnostic_name(def_id),
-                Some(sym::ptr_cast | sym::const_ptr_cast | sym::ptr_cast_mut | sym::ptr_cast_const)
-            )
-        {
-            expr
-        // ptr::from_ref(<expr>), UnsafeCell::raw_get(<expr>) or mem::transmute<_, _>(<expr>)
-        } else if let ExprKind::Call(path, [arg]) = e.kind
-            && 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_from_ref | sym::unsafe_cell_raw_get | sym::transmute)
-            )
-        {
-            if cx.tcx.is_diagnostic_item(sym::unsafe_cell_raw_get, def_id) {
-                gone_trough_unsafe_cell_raw_get = true;
-            }
-            arg
-        } else {
-            let init = cx.expr_or_init(e);
-            if init.hir_id != e.hir_id {
-                init
-            } else {
-                break;
-            }
-        };
-    }
-
-    (e, gone_trough_unsafe_cell_raw_get)
-}