Improve diagnostics and add tests for function calls

This commit is contained in:
est31 2023-08-06 01:25:24 +02:00
parent 8faac74e54
commit 4b1bc27010
5 changed files with 64 additions and 44 deletions

View File

@ -457,6 +457,8 @@ lint_ptr_null_checks_fn_ptr = function pointers are not nullable, so checking th
.help = wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value .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}` .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 lint_ptr_null_checks_ref = references are not nullable, so checking them for null will always return false
.label = expression has type `{$orig_ty}` .label = expression has type `{$orig_ty}`

View File

@ -634,6 +634,8 @@ pub enum PtrNullChecksDiag<'a> {
#[label] #[label]
label: Span, label: Span,
}, },
#[diag(lint_ptr_null_checks_fn_ret)]
FnRet { fn_name: Ident },
} }
// for_loops_over_fallibles.rs // for_loops_over_fallibles.rs

View File

@ -31,12 +31,30 @@ declare_lint! {
declare_lint_pass!(PtrNullChecks => [USELESS_PTR_NULL_CHECKS]); declare_lint_pass!(PtrNullChecks => [USELESS_PTR_NULL_CHECKS]);
/// This function detects and returns the original expression from a series of consecutive casts, /// This function checks if the expression is from a series of consecutive casts,
/// ie. `(my_fn as *const _ as *mut _).cast_mut()` would return the expression for `my_fn`. /// ie. `(my_fn as *const _ as *mut _).cast_mut()` and whether the original expression is either
fn ptr_cast_chain<'a>(cx: &'a LateContext<'_>, mut e: &'a Expr<'a>) -> Option<&'a Expr<'a>> { /// 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>(
cx: &'a LateContext<'tcx>,
mut e: &'a Expr<'a>,
) -> Option<PtrNullChecksDiag<'tcx>> {
let mut had_at_least_one_cast = false; let mut had_at_least_one_cast = false;
loop { loop {
e = e.peel_blocks(); e = e.peel_blocks();
if let ExprKind::MethodCall(_, _expr, [], _) = e.kind
&& let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_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 });
} 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 });
}
e = if let ExprKind::Cast(expr, t) = e.kind e = if let ExprKind::Cast(expr, t) = e.kind
&& let TyKind::Ptr(_) = t.kind { && let TyKind::Ptr(_) = t.kind {
had_at_least_one_cast = true; had_at_least_one_cast = true;
@ -46,33 +64,21 @@ fn ptr_cast_chain<'a>(cx: &'a LateContext<'_>, mut e: &'a Expr<'a>) -> Option<&'
&& matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::ptr_cast | sym::ptr_cast_mut)) { && matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::ptr_cast | sym::ptr_cast_mut)) {
had_at_least_one_cast = true; had_at_least_one_cast = true;
expr 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()
&& cx.tcx.has_attr(def_id, sym::rustc_never_returns_null_ptr) {
had_at_least_one_cast = true;
arg
} else if had_at_least_one_cast { } else if had_at_least_one_cast {
return Some(e); let orig_ty = cx.typeck_results().expr_ty(e);
return if orig_ty.is_fn() {
Some(PtrNullChecksDiag::FnPtr { orig_ty, label: e.span })
} else if orig_ty.is_ref() {
Some(PtrNullChecksDiag::Ref { orig_ty, label: e.span })
} else {
None
};
} else { } else {
return None; return None;
}; };
} }
} }
fn incorrect_check<'a>(cx: &LateContext<'a>, expr: &Expr<'_>) -> Option<PtrNullChecksDiag<'a>> {
let expr = ptr_cast_chain(cx, expr)?;
let orig_ty = cx.typeck_results().expr_ty(expr);
if orig_ty.is_fn() {
Some(PtrNullChecksDiag::FnPtr { orig_ty, label: expr.span })
} else if orig_ty.is_ref() {
Some(PtrNullChecksDiag::Ref { orig_ty, label: expr.span })
} else {
None
}
}
impl<'tcx> LateLintPass<'tcx> for PtrNullChecks { impl<'tcx> LateLintPass<'tcx> for PtrNullChecks {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
match expr.kind { match expr.kind {

View File

@ -38,15 +38,15 @@ fn main() {
if (&mut 8 as *mut i32).is_null() {} if (&mut 8 as *mut i32).is_null() {}
//~^ WARN references are not nullable //~^ WARN references are not nullable
if ptr::from_mut(&mut 8).is_null() {} if ptr::from_mut(&mut 8).is_null() {}
//~^ WARN references are not nullable //~^ WARN call is never null
if (&8 as *const i32).is_null() {} if (&8 as *const i32).is_null() {}
//~^ WARN references are not nullable //~^ WARN references are not nullable
if ptr::from_ref(&8).is_null() {} if ptr::from_ref(&8).is_null() {}
//~^ WARN references are not nullable //~^ WARN call is never null
if ptr::from_ref(&8).cast_mut().is_null() {} if ptr::from_ref(&8).cast_mut().is_null() {}
//~^ WARN references are not nullable //~^ WARN call is never null
if (ptr::from_ref(&8).cast_mut() as *mut i32).is_null() {} if (ptr::from_ref(&8).cast_mut() as *mut i32).is_null() {}
//~^ WARN references are not nullable //~^ WARN call is never null
if (&8 as *const i32) == std::ptr::null() {} if (&8 as *const i32) == std::ptr::null() {}
//~^ WARN references are not nullable //~^ WARN references are not nullable
let ref_num = &8; let ref_num = &8;
@ -65,6 +65,12 @@ fn main() {
if (&*{ static_i32() } as *const i32).is_null() {} if (&*{ static_i32() } as *const i32).is_null() {}
//~^ WARN references are not nullable //~^ WARN references are not nullable
// ---------------- Functions -------------------
if ptr::NonNull::new(&mut 8).unwrap().as_ptr().is_null() {}
//~^ WARN call is never null
if ptr::NonNull::<u8>::dangling().as_ptr().is_null() {}
//~^ WARN call is never null
// ---------------------------------------------- // ----------------------------------------------
const ZPTR: *const () = 0 as *const _; const ZPTR: *const () = 0 as *const _;
const NOT_ZPTR: *const () = 1 as *const _; const NOT_ZPTR: *const () = 1 as *const _;

View File

@ -117,13 +117,11 @@ LL | if (&mut 8 as *mut i32).is_null() {}
| | | |
| expression has type `&mut i32` | expression has type `&mut i32`
warning: references are not nullable, so checking them for null will always return false warning: returned pointer of `from_mut` call is never null, so checking it for null will always return false
--> $DIR/ptr_null_checks.rs:40:8 --> $DIR/ptr_null_checks.rs:40:8
| |
LL | if ptr::from_mut(&mut 8).is_null() {} LL | if ptr::from_mut(&mut 8).is_null() {}
| ^^^^^^^^^^^^^^------^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| expression has type `&mut i32`
warning: references are not nullable, so checking them for null will always return false warning: references are not nullable, so checking them for null will always return false
--> $DIR/ptr_null_checks.rs:42:8 --> $DIR/ptr_null_checks.rs:42:8
@ -133,29 +131,23 @@ LL | if (&8 as *const i32).is_null() {}
| | | |
| expression has type `&i32` | expression has type `&i32`
warning: references are not nullable, so checking them for null will always return false warning: returned pointer of `from_ref` call is never null, so checking it for null will always return false
--> $DIR/ptr_null_checks.rs:44:8 --> $DIR/ptr_null_checks.rs:44:8
| |
LL | if ptr::from_ref(&8).is_null() {} LL | if ptr::from_ref(&8).is_null() {}
| ^^^^^^^^^^^^^^--^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| expression has type `&i32`
warning: references are not nullable, so checking them for null will always return false warning: returned pointer of `from_ref` call is never null, so checking it for null will always return false
--> $DIR/ptr_null_checks.rs:46:8 --> $DIR/ptr_null_checks.rs:46:8
| |
LL | if ptr::from_ref(&8).cast_mut().is_null() {} LL | if ptr::from_ref(&8).cast_mut().is_null() {}
| ^^^^^^^^^^^^^^--^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| expression has type `&i32`
warning: references are not nullable, so checking them for null will always return false warning: returned pointer of `from_ref` call is never null, so checking it for null will always return false
--> $DIR/ptr_null_checks.rs:48:8 --> $DIR/ptr_null_checks.rs:48:8
| |
LL | if (ptr::from_ref(&8).cast_mut() as *mut i32).is_null() {} LL | if (ptr::from_ref(&8).cast_mut() as *mut i32).is_null() {}
| ^^^^^^^^^^^^^^^--^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| expression has type `&i32`
warning: references are not nullable, so checking them for null will always return false warning: references are not nullable, so checking them for null will always return false
--> $DIR/ptr_null_checks.rs:50:8 --> $DIR/ptr_null_checks.rs:50:8
@ -221,5 +213,17 @@ LL | if (&*{ static_i32() } as *const i32).is_null() {}
| | | |
| expression has type `&i32` | expression has type `&i32`
warning: 25 warnings emitted warning: returned pointer of `as_ptr` call is never null, so checking it for null will always return false
--> $DIR/ptr_null_checks.rs:69:8
|
LL | if ptr::NonNull::new(&mut 8).unwrap().as_ptr().is_null() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: returned pointer of `as_ptr` call is never null, so checking it for null will always return false
--> $DIR/ptr_null_checks.rs:71:8
|
LL | if ptr::NonNull::<u8>::dangling().as_ptr().is_null() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: 27 warnings emitted