bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

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

Browse source 
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
This commit is contained in:
bors 2025-03-31 04:17:14 +00:00
commit 7bfd9529be
30 changed files with 713 additions and 719 deletions
Download patch file Download diff file Expand all files Collapse all files

22
compiler/rustc_lint/messages.ftl
View file

@ -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 =

1
compiler/rustc_lint/src/lib.rs
View file

@ -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;

26
compiler/rustc_lint/src/lints.rs
View file

@ -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)]

128
compiler/rustc_lint/src/ptr_nulls.rs
View file

@ -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 {

44
compiler/rustc_lint/src/reference_casting.rs
View file

@ -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)
}

55
compiler/rustc_lint/src/utils.rs Normal file
View file

@ -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)
}