diff options
| author | bors <bors@rust-lang.org> | 2025-03-31 04:17:14 +0000 | 
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2025-03-31 04:17:14 +0000 | 
| commit | 7bfd9529be7f4e10ca12f9eee1f442c12c6ea8ad (patch) | |
| tree | 7d68dbe33a369305bfe85c890e8539361c75f52a /compiler/rustc_lint/src | |
| parent | 3c0f72271b0fcc9ebfed79e1004ea4d5693f1a34 (diff) | |
| parent | aa8848040a160842d47a5a143e04da8ad9f27613 (diff) | |
| download | rust-7bfd9529be7f4e10ca12f9eee1f442c12c6ea8ad.tar.gz rust-7bfd9529be7f4e10ca12f9eee1f442c12c6ea8ad.zip | |
Auto merge of #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](https://github.com/rust-lang/rust/pull/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
Diffstat (limited to 'compiler/rustc_lint/src')
| -rw-r--r-- | compiler/rustc_lint/src/lib.rs | 1 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/lints.rs | 26 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/ptr_nulls.rs | 128 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/reference_casting.rs | 44 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/utils.rs | 55 | 
5 files changed, 192 insertions, 62 deletions
| diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index cd474f1b7db..25878c7ac81 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/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; diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 036d68d13fa..55d010e6d34 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/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)] diff --git a/compiler/rustc_lint/src/ptr_nulls.rs b/compiler/rustc_lint/src/ptr_nulls.rs index 1489f9de819..826bce2c315 100644 --- a/compiler/rustc_lint/src/ptr_nulls.rs +++ b/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,12 +141,68 @@ 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, _, _) if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) @@ -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 { diff --git a/compiler/rustc_lint/src/reference_casting.rs b/compiler/rustc_lint/src/reference_casting.rs index 7c6656f91c9..1d4d380cb68 100644 --- a/compiler/rustc_lint/src/reference_casting.rs +++ b/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) -} diff --git a/compiler/rustc_lint/src/utils.rs b/compiler/rustc_lint/src/utils.rs new file mode 100644 index 00000000000..a7295d9c532 --- /dev/null +++ b/compiler/rustc_lint/src/utils.rs @@ -0,0 +1,55 @@ +use rustc_hir::{Expr, ExprKind}; +use rustc_span::sym; + +use crate::LateContext; + +/// Given an expression, peel all of casts (`<expr> as ...`, `<expr>.cast{,_mut,_const}()`, +/// `ptr::from_ref(<expr>)`, ...) and init expressions. +/// +/// Returns the innermost expression and a boolean representing if one of the casts was +/// `UnsafeCell::raw_get(<expr>)` +pub(crate) 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) +} | 
