Auto merge of rust-lang#114494 - est31:extend_useless_ptr_null_checks…

…, r=jackh726

Make useless_ptr_null_checks smarter about some std functions

This teaches the `useless_ptr_null_checks` lint that some std functions can't ever return null pointers, because they need to point to valid data, get references as input, etc.

This is achieved by introducing an `#[rustc_never_returns_null_ptr]` attribute and adding it to these std functions (gated behind bootstrap `cfg_attr`).

Later on, the attribute could maybe be used to tell LLVM that the returned pointer is never null. I don't expect much impact of that though, as the functions are pretty shallow and usually the input data is already never null.

Follow-up of PR rust-lang#113657

Fixes rust-lang#114442

compiler/rustc_feature/src/builtin_attrs.rs

@@ -699,6 +699,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         rustc_pass_by_value, Normal, template!(Word), ErrorFollowing,
         "#[rustc_pass_by_value] is used to mark types that must be passed by value instead of reference."
     ),
+    rustc_attr!(
+        rustc_never_returns_null_ptr, Normal, template!(Word), ErrorFollowing,
+        "#[rustc_never_returns_null_ptr] is used to mark functions returning non-null pointers."
+    ),
     rustc_attr!(
         rustc_coherence_is_core, AttributeType::CrateLevel, template!(Word), ErrorFollowing, @only_local: true,
         "#![rustc_coherence_is_core] allows inherent methods on builtin types, only intended to be used in `core`."

compiler/rustc_lint/messages.ftl

@@ -453,6 +453,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
     .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}`
 

compiler/rustc_lint/src/lints.rs

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

compiler/rustc_lint/src/ptr_nulls.rs

@@ -31,12 +31,30 @@ declare_lint! {
 
 declare_lint_pass!(PtrNullChecks => [USELESS_PTR_NULL_CHECKS]);
 
-/// This function detects and returns the original expression from a series of consecutive casts,
-/// ie. `(my_fn as *const _ as *mut _).cast_mut()` would return the expression for `my_fn`.
-fn ptr_cast_chain<'a>(cx: &'a LateContext<'_>, mut e: &'a Expr<'a>) -> Option<&'a Expr<'a>> {
+/// 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>(
+    cx: &'a LateContext<'tcx>,
+    mut e: &'a Expr<'a>,
+) -> Option<PtrNullChecksDiag<'tcx>> {
     let mut had_at_least_one_cast = false;
     loop {
         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
             && let TyKind::Ptr(_) = t.kind {
             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)) {
             had_at_least_one_cast = true;
             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::ptr_from_mut)) {
-            had_at_least_one_cast = true;
-            arg
         } 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 {
             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 {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         match expr.kind {

compiler/rustc_passes/src/check_attr.rs

@@ -139,6 +139,9 @@ impl CheckAttrVisitor<'_> {
                     self.check_rustc_std_internal_symbol(&attr, span, target)
                 }
                 sym::naked => self.check_naked(hir_id, attr, span, target),
+                sym::rustc_never_returns_null_ptr => {
+                    self.check_applied_to_fn_or_method(hir_id, attr, span, target)
+                }
                 sym::rustc_legacy_const_generics => {
                     self.check_rustc_legacy_const_generics(hir_id, &attr, span, target, item)
                 }

compiler/rustc_span/src/symbol.rs

@@ -1178,7 +1178,6 @@ symbols! {
         ptr_cast_const,
         ptr_cast_mut,
         ptr_const_is_null,
-        ptr_from_mut,
         ptr_from_ref,
         ptr_guaranteed_cmp,
         ptr_is_null,
@@ -1337,6 +1336,7 @@ symbols! {
         rustc_main,
         rustc_mir,
         rustc_must_implement_one_of,
+        rustc_never_returns_null_ptr,
         rustc_nonnull_optimization_guaranteed,
         rustc_nounwind,
         rustc_object_lifetime_default,

library/alloc/src/rc.rs

@@ -1304,6 +1304,7 @@ impl<T: ?Sized, A: Allocator> Rc<T, A> {
     /// assert_eq!(unsafe { &*x_ptr }, "hello");
     /// ```
     #[stable(feature = "rc_raw", since = "1.17.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub fn into_raw(this: Self) -> *const T {
         let ptr = Self::as_ptr(&this);
         mem::forget(this);
@@ -1327,6 +1328,7 @@ impl<T: ?Sized, A: Allocator> Rc<T, A> {
     /// assert_eq!(unsafe { &*x_ptr }, "hello");
     /// ```
     #[stable(feature = "weak_into_raw", since = "1.45.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub fn as_ptr(this: &Self) -> *const T {
         let ptr: *mut RcBox<T> = NonNull::as_ptr(this.ptr);
 

library/alloc/src/sync.rs

@@ -1454,6 +1454,7 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
     /// ```
     #[must_use = "losing the pointer will leak memory"]
     #[stable(feature = "rc_raw", since = "1.17.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub fn into_raw(this: Self) -> *const T {
         let ptr = Self::as_ptr(&this);
         mem::forget(this);
@@ -1478,6 +1479,7 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
     /// ```
     #[must_use]
     #[stable(feature = "rc_as_ptr", since = "1.45.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub fn as_ptr(this: &Self) -> *const T {
         let ptr: *mut ArcInner<T> = NonNull::as_ptr(this.ptr);
 

library/alloc/src/vec/mod.rs

@@ -1258,6 +1258,7 @@ impl<T, A: Allocator> Vec<T, A> {
     /// [`as_mut_ptr`]: Vec::as_mut_ptr
     /// [`as_ptr`]: Vec::as_ptr
     #[stable(feature = "vec_as_ptr", since = "1.37.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     #[inline]
     pub fn as_ptr(&self) -> *const T {
         // We shadow the slice method of the same name to avoid going through
@@ -1317,6 +1318,7 @@ impl<T, A: Allocator> Vec<T, A> {
     /// [`as_mut_ptr`]: Vec::as_mut_ptr
     /// [`as_ptr`]: Vec::as_ptr
     #[stable(feature = "vec_as_ptr", since = "1.37.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     #[inline]
     pub fn as_mut_ptr(&mut self) -> *mut T {
         // We shadow the slice method of the same name to avoid going through

library/core/src/cell.rs

@@ -556,6 +556,7 @@ impl<T: ?Sized> Cell<T> {
     #[inline]
     #[stable(feature = "cell_as_ptr", since = "1.12.0")]
     #[rustc_const_stable(feature = "const_cell_as_ptr", since = "1.32.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub const fn as_ptr(&self) -> *mut T {
         self.value.get()
     }
@@ -1111,6 +1112,7 @@ impl<T: ?Sized> RefCell<T> {
     /// ```
     #[inline]
     #[stable(feature = "cell_as_ptr", since = "1.12.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub fn as_ptr(&self) -> *mut T {
         self.value.get()
     }
@@ -2105,6 +2107,7 @@ impl<T: ?Sized> UnsafeCell<T> {
     #[inline(always)]
     #[stable(feature = "rust1", since = "1.0.0")]
     #[rustc_const_stable(feature = "const_unsafecell_get", since = "1.32.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub const fn get(&self) -> *mut T {
         // We can just cast the pointer from `UnsafeCell<T>` to `T` because of
         // #[repr(transparent)]. This exploits std's special status, there is
@@ -2248,6 +2251,7 @@ impl<T: ?Sized> SyncUnsafeCell<T> {
     /// when casting to `&mut T`, and ensure that there are no mutations
     /// or mutable aliases going on when casting to `&T`
     #[inline]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub const fn get(&self) -> *mut T {
         self.value.get()
     }

library/core/src/ffi/c_str.rs

@@ -511,6 +511,7 @@ impl CStr {
     #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     #[rustc_const_stable(feature = "const_str_as_ptr", since = "1.32.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub const fn as_ptr(&self) -> *const c_char {
         self.inner.as_ptr()
     }

library/core/src/ptr/mod.rs

@@ -698,6 +698,7 @@ where
 #[inline(always)]
 #[must_use]
 #[unstable(feature = "ptr_from_ref", issue = "106116")]
+#[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
 #[rustc_diagnostic_item = "ptr_from_ref"]
 pub const fn from_ref<T: ?Sized>(r: &T) -> *const T {
     r
@@ -710,7 +711,7 @@ pub const fn from_ref<T: ?Sized>(r: &T) -> *const T {
 #[inline(always)]
 #[must_use]
 #[unstable(feature = "ptr_from_ref", issue = "106116")]
-#[rustc_diagnostic_item = "ptr_from_mut"]
+#[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
 pub const fn from_mut<T: ?Sized>(r: &mut T) -> *mut T {
     r
 }

library/core/src/ptr/non_null.rs

@@ -338,6 +338,7 @@ impl<T: ?Sized> NonNull<T> {
     /// ```
     #[stable(feature = "nonnull", since = "1.25.0")]
     #[rustc_const_stable(feature = "const_nonnull_as_ptr", since = "1.32.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     #[must_use]
     #[inline(always)]
     pub const fn as_ptr(self) -> *mut T {
@@ -597,6 +598,7 @@ impl<T> NonNull<[T]> {
     #[must_use]
     #[unstable(feature = "slice_ptr_get", issue = "74265")]
     #[rustc_const_unstable(feature = "slice_ptr_get", issue = "74265")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub const fn as_mut_ptr(self) -> *mut T {
         self.as_non_null_ptr().as_ptr()
     }

library/core/src/slice/mod.rs

@@ -730,6 +730,7 @@ impl<T> [T] {
     /// [`as_mut_ptr`]: slice::as_mut_ptr
     #[stable(feature = "rust1", since = "1.0.0")]
     #[rustc_const_stable(feature = "const_slice_as_ptr", since = "1.32.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     #[inline(always)]
     #[must_use]
     pub const fn as_ptr(&self) -> *const T {
@@ -760,6 +761,7 @@ impl<T> [T] {
     #[stable(feature = "rust1", since = "1.0.0")]
     #[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
     #[rustc_allow_const_fn_unstable(const_mut_refs)]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     #[inline(always)]
     #[must_use]
     pub const fn as_mut_ptr(&mut self) -> *mut T {

library/core/src/str/mod.rs

@@ -386,6 +386,7 @@ impl str {
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     #[rustc_const_stable(feature = "rustc_str_as_ptr", since = "1.32.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     #[must_use]
     #[inline(always)]
     pub const fn as_ptr(&self) -> *const u8 {
@@ -401,6 +402,7 @@ impl str {
     /// It is your responsibility to make sure that the string slice only gets
     /// modified in a way that it remains valid UTF-8.
     #[stable(feature = "str_as_mut_ptr", since = "1.36.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     #[must_use]
     #[inline(always)]
     pub fn as_mut_ptr(&mut self) -> *mut u8 {

library/core/src/sync/atomic.rs

@@ -1018,6 +1018,7 @@ impl AtomicBool {
     #[inline]
     #[stable(feature = "atomic_as_ptr", since = "1.70.0")]
     #[rustc_const_stable(feature = "atomic_as_ptr", since = "1.70.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub const fn as_ptr(&self) -> *mut bool {
         self.v.get().cast()
     }
@@ -1953,6 +1954,7 @@ impl<T> AtomicPtr<T> {
     #[inline]
     #[stable(feature = "atomic_as_ptr", since = "1.70.0")]
     #[rustc_const_stable(feature = "atomic_as_ptr", since = "1.70.0")]
+    #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
     pub const fn as_ptr(&self) -> *mut *mut T {
         self.p.get()
     }
@@ -2891,6 +2893,7 @@ macro_rules! atomic_int {
             #[inline]
             #[stable(feature = "atomic_as_ptr", since = "1.70.0")]
             #[rustc_const_stable(feature = "atomic_as_ptr", since = "1.70.0")]
+            #[cfg_attr(not(bootstrap), rustc_never_returns_null_ptr)]
             pub const fn as_ptr(&self) -> *mut $int_type {
                 self.v.get()
             }

tests/ui/lint/ptr_null_checks.rs

@@ -38,15 +38,15 @@ fn main() {
     if (&mut 8 as *mut i32).is_null() {}
     //~^ WARN references are not nullable
     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() {}
     //~^ WARN references are not nullable
     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() {}
-    //~^ WARN references are not nullable
+    //~^ WARN call is never 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() {}
     //~^ WARN references are not nullable
     let ref_num = &8;
@@ -65,6 +65,12 @@ fn main() {
     if (&*{ static_i32() } as *const i32).is_null() {}
     //~^ 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 NOT_ZPTR: *const () = 1 as *const _;